diff --git a/packet_packer.go b/packet_packer.go index efd93475..03bc28ba 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -43,12 +43,8 @@ func (p *packetPacker) AddHighPrioStreamFrame(f frames.StreamFrame) { func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, includeStreamFrames bool) (*packedPacket, error) { // TODO: save controlFrames as a member variable, makes it easier to handle in the unlikely event that there are more controlFrames than you can put into on packet - payloadFrames, err := p.composeNextPacket(stopWaitingFrame, controlFrames, includeStreamFrames) - if err != nil { - return nil, err - } - - if len(payloadFrames) == 0 { + // don't send out packets that only contain a StopWaitingFrame + if len(controlFrames) == 0 && (p.streamFrameQueue.Len() == 0 || !includeStreamFrames) { return nil, nil } @@ -57,6 +53,23 @@ func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, con 1, )) + responsePublicHeader := &PublicHeader{ + ConnectionID: p.connectionID, + PacketNumber: currentPacketNumber, + PacketNumberLen: getPacketNumberLength(currentPacketNumber, p.sentPacketHandler.GetLargestObserved()), + TruncateConnectionID: p.connectionParametersManager.TruncateConnectionID(), + } + + publicHeaderLength, err := responsePublicHeader.GetLength() + if err != nil { + return nil, err + } + + payloadFrames, err := p.composeNextPacket(stopWaitingFrame, controlFrames, publicHeaderLength, includeStreamFrames) + if err != nil { + return nil, err + } + payload, err := p.getPayload(payloadFrames, currentPacketNumber) if err != nil { return nil, err @@ -71,12 +84,6 @@ func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, con } var raw bytes.Buffer - responsePublicHeader := PublicHeader{ - ConnectionID: p.connectionID, - PacketNumber: currentPacketNumber, - PacketNumberLen: getPacketNumberLength(currentPacketNumber, p.sentPacketHandler.GetLargestObserved()), - TruncateConnectionID: p.connectionParametersManager.TruncateConnectionID(), - } if err := responsePublicHeader.WritePublicHeader(&raw); err != nil { return nil, err } @@ -105,15 +112,18 @@ func (p *packetPacker) getPayload(frames []frames.Frame, currentPacketNumber pro return payload.Bytes(), nil } -func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, includeStreamFrames bool) ([]frames.Frame, error) { +// func (p *packetPacker) composeNextPacketControlFrames(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame) (uint8, []frames.Frame, error) { +// +// } +// +// func (p *packetPacker) composeNextPacketStreamFrames() ([]frames.Frame, error) { +// +// } + +func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, publicHeaderLength uint8, includeStreamFrames bool) ([]frames.Frame, error) { payloadLength := 0 var payloadFrames []frames.Frame - // don't send out packets that only contain a StopWaitingFrame - if len(controlFrames) == 0 && p.streamFrameQueue.Len() == 0 { - return nil, nil - } - // TODO: handle the case where there are more controlFrames than we can put into one packet if stopWaitingFrame != nil { payloadFrames = append(payloadFrames, stopWaitingFrame) @@ -127,7 +137,9 @@ func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFra controlFrames = controlFrames[1:] } - if payloadLength > protocol.MaxFrameSize { + maxFrameSize := protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength) + + if payloadLength > maxFrameSize { panic("internal inconsistency: packet payload too large") } @@ -138,17 +150,17 @@ func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFra for p.streamFrameQueue.Len() > 0 { frame := p.streamFrameQueue.Front() - if payloadLength > protocol.MaxFrameSize { + if payloadLength > maxFrameSize { panic("internal inconsistency: packet payload too large") } // Does the frame fit into the remaining space? - if payloadLength+frame.MinLength() > protocol.MaxFrameSize { + if payloadLength+frame.MinLength() > maxFrameSize { break } // Split stream frames if necessary - previousFrame := frame.MaybeSplitOffFrame(protocol.MaxFrameSize - payloadLength) + previousFrame := frame.MaybeSplitOffFrame(maxFrameSize - payloadLength) if previousFrame != nil { // Don't pop the queue, leave the modified frame in frame = previousFrame diff --git a/packet_packer_test.go b/packet_packer_test.go index 36423eeb..25c53116 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -110,18 +110,19 @@ var _ = Describe("Packet packer", func() { }) It("packs many control frames into 1 packets", func() { + publicHeaderLength := uint8(10) f := &frames.AckFrame{LargestObserved: 1} b := &bytes.Buffer{} f.Write(b, 3, protocol.PacketNumberLen6, 32) - maxFramesPerPacket := protocol.MaxFrameSize / b.Len() + maxFramesPerPacket := (protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength)) / b.Len() var controlFrames []frames.Frame for i := 0; i < maxFramesPerPacket; i++ { controlFrames = append(controlFrames, f) } - payloadFrames, err := packer.composeNextPacket(nil, controlFrames, true) + payloadFrames, err := packer.composeNextPacket(nil, controlFrames, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(maxFramesPerPacket)) - payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true) + payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(BeZero()) }) @@ -149,16 +150,17 @@ var _ = Describe("Packet packer", func() { Context("Stream Frame handling", func() { It("does not splits a stream frame with maximum size", func() { - maxStreamFrameDataLen := protocol.MaxFrameSize - (1 + 4 + 8 + 2) + publicHeaderLength := uint8(12) + maxStreamFrameDataLen := protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength) - (1 + 4 + 8 + 2) f := frames.StreamFrame{ Data: bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen), Offset: 1, } packer.AddStreamFrame(f) - payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, true) + payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(1)) - payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true) + payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(0)) }) @@ -185,28 +187,29 @@ var _ = Describe("Packet packer", func() { }) It("splits one stream frame larger than maximum size", func() { - maxStreamFrameDataLen := protocol.MaxFrameSize - (1 + 4 + 8 + 2) + publicHeaderLength := uint8(5) + maxStreamFrameDataLen := protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength) - (1 + 4 + 8 + 2) f := frames.StreamFrame{ Data: bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen+200), Offset: 1, } packer.AddStreamFrame(f) - payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, true) + payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(1)) Expect(len(payloadFrames[0].(*frames.StreamFrame).Data)).To(Equal(maxStreamFrameDataLen)) - payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true) + payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(1)) Expect(len(payloadFrames[0].(*frames.StreamFrame).Data)).To(Equal(200)) - payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true) + payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(0)) }) - // set pending until https://github.com/lucas-clemente/quic-go/issues/67 is fixed - PIt("packs 2 stream frames that are too big for one packet correctly", func() { - maxStreamFrameDataLen := protocol.MaxFrameSize - (1 + 4 + 8 + 2) + It("packs 2 stream frames that are too big for one packet correctly", func() { + publicHeaderLength := uint8(5) + maxStreamFrameDataLen := protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength) - (1 + 4 + 8 + 2) f1 := frames.StreamFrame{ Data: bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen+100), Offset: 1, @@ -231,10 +234,10 @@ var _ = Describe("Packet packer", func() { Expect(p).To(BeNil()) }) - // set pending until https://github.com/lucas-clemente/quic-go/issues/67 is fixed - PIt("packs a packet that has the maximum packet size when given a large enough stream frame", func() { + It("packs a packet that has the maximum packet size when given a large enough stream frame", func() { + publicHeaderLength := uint8(3) f := frames.StreamFrame{ - Data: bytes.Repeat([]byte{'f'}, protocol.MaxFrameSize-(1+4+8+2)), + Data: bytes.Repeat([]byte{'f'}, protocol.MaxFrameAndPublicHeaderSize-int(publicHeaderLength)-(1+4+8+2)), Offset: 1, } packer.AddStreamFrame(f) @@ -245,15 +248,16 @@ var _ = Describe("Packet packer", func() { }) It("splits a stream frame larger than the maximum size", func() { + publicHeaderLength := uint8(13) f := frames.StreamFrame{ - Data: bytes.Repeat([]byte{'f'}, protocol.MaxFrameSize-(1+4+8+2)+1), + Data: bytes.Repeat([]byte{'f'}, protocol.MaxFrameAndPublicHeaderSize-int(publicHeaderLength)-(1+4+8+2)+1), Offset: 1, } packer.AddStreamFrame(f) - payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, true) + payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(1)) - payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true) + payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(1)) }) diff --git a/protocol/protocol.go b/protocol/protocol.go index 40428eb3..205a94fd 100644 --- a/protocol/protocol.go +++ b/protocol/protocol.go @@ -32,8 +32,8 @@ type ErrorCode uint32 // MaxPacketSize is the maximum packet size, including the public header const MaxPacketSize = 1452 -// MaxFrameSize is the maximum size of a QUIC frame -const MaxFrameSize = MaxPacketSize - (1 + 8 + 6) /*public header*/ - 1 /*private header*/ - 12 /*crypto signature*/ +// MaxFrameAndPublicHeaderSize is the maximum size of a QUIC frame plus PublicHeader +const MaxFrameAndPublicHeaderSize = MaxPacketSize - 1 /*private header*/ - 12 /*crypto signature*/ // DefaultTCPMSS is the default maximum packet size used in the Linux TCP implementation. // Used in QUIC for congestion window computations in bytes.