From eada642cc1c13c33ae86431db9e820961c16c855 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 26 Apr 2016 20:18:50 +0700 Subject: [PATCH] make PacketPacker assemble frames to pack into a Packet --- packet_packer.go | 36 +++++++++++----- packet_packer_test.go | 96 ++++++++++++++++++++++++++++++++----------- 2 files changed, 98 insertions(+), 34 deletions(-) diff --git a/packet_packer.go b/packet_packer.go index 51337253..203e5e99 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -47,7 +47,12 @@ func (p *packetPacker) PackPacket() (*packedPacket, error) { 1, )) - payload, err := p.composeNextPayload(currentPacketNumber) + frames, err := p.composeNextPacket() + if err != nil { + return nil, err + } + + payload, err := p.getPayload(frames, currentPacketNumber) if err != nil { return nil, err } @@ -84,38 +89,49 @@ func (p *packetPacker) PackPacket() (*packedPacket, error) { }, nil } -func (p *packetPacker) composeNextPayload(currentPacketNumber protocol.PacketNumber) ([]byte, error) { +func (p *packetPacker) getPayload(frames []frames.Frame, currentPacketNumber protocol.PacketNumber) ([]byte, error) { var payload bytes.Buffer payload.WriteByte(0) // The entropy bit is set in sendPayload + for _, frame := range frames { + frame.Write(&payload, currentPacketNumber, 6) + } + return payload.Bytes(), nil +} + +func (p *packetPacker) composeNextPacket() ([]frames.Frame, error) { + payloadLength := 0 + var payloadFrames []frames.Frame for len(p.queuedFrames) > 0 { frame := p.queuedFrames[0] - if payload.Len()-1 > protocol.MaxFrameSize { + if payloadLength > protocol.MaxFrameSize { panic("internal inconsistency: packet payload too large") } // Does the frame fit into the remaining space? - if payload.Len()-1+frame.MinLength() > protocol.MaxFrameSize { - return payload.Bytes(), nil + if payloadLength+frame.MinLength() > protocol.MaxFrameSize { + break } if streamframe, isStreamFrame := frame.(*frames.StreamFrame); isStreamFrame { // Split stream frames if necessary - previousFrame := streamframe.MaybeSplitOffFrame(protocol.MaxFrameSize - (payload.Len() - 1)) + previousFrame := streamframe.MaybeSplitOffFrame(protocol.MaxFrameSize - payloadLength) if previousFrame != nil { // Don't pop the queue, leave the modified frame in frame = previousFrame + payloadLength += len(previousFrame.Data) - 1 } else { p.queuedFrames = p.queuedFrames[1:] + payloadLength += len(streamframe.Data) - 1 } } else { p.queuedFrames = p.queuedFrames[1:] } - if err := frame.Write(&payload, currentPacketNumber, 6); err != nil { - return nil, err - } + payloadLength += frame.MinLength() + payloadFrames = append(payloadFrames, frame) } - return payload.Bytes(), nil + + return payloadFrames, nil } diff --git a/packet_packer_test.go b/packet_packer_test.go index d540fce1..c13ec3f3 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -57,35 +57,83 @@ var _ = Describe("Packet packer", func() { f := &frames.AckFrame{LargestObserved: 1} b := &bytes.Buffer{} f.Write(b, 3, 6) - for i := 0; i <= (protocol.MaxFrameSize-1)/b.Len()+1; i++ { + maxFramesPerPacket := protocol.MaxFrameSize / b.Len() + counter := 0 + for i := 0; i < maxFramesPerPacket+1; i++ { packer.AddFrame(f) + counter++ } - p, err := packer.PackPacket() - Expect(p).ToNot(BeNil()) + payloadFrames, err := packer.composeNextPacket() Expect(err).ToNot(HaveOccurred()) - Expect(len(p.payload) % b.Len()).To(BeZero()) - Expect(p.raw).To(ContainSubstring(string(b.Bytes()))) - p, err = packer.PackPacket() - Expect(p).ToNot(BeNil()) + Expect(len(payloadFrames)).To(Equal(maxFramesPerPacket)) + payloadFrames, err = packer.composeNextPacket() Expect(err).ToNot(HaveOccurred()) - Expect(p.payload).To(Equal(b.Bytes())) - Expect(p.raw).To(ContainSubstring(string(b.Bytes()))) + Expect(len(payloadFrames)).To(Equal(counter - maxFramesPerPacket)) }) - It("splits stream frames", func() { - f := &frames.StreamFrame{ - Data: bytes.Repeat([]byte{'f'}, protocol.MaxFrameSize), - Offset: 1, - } - b := &bytes.Buffer{} - f.Write(b, 4, 6) - packer.AddFrame(f) - p, err := packer.PackPacket() - Expect(p).ToNot(BeNil()) - Expect(len(p.raw)).To(Equal(protocol.MaxPacketSize)) - Expect(err).ToNot(HaveOccurred()) - p, err = packer.PackPacket() - Expect(p).ToNot(BeNil()) - Expect(err).ToNot(HaveOccurred()) + Context("Stream Frame handling", func() { + It("does not splits a stream frame with maximum size", func() { + maxStreamFrameDataLen := protocol.MaxFrameSize - (1 + 4 + 8 + 2) + f := &frames.StreamFrame{ + Data: bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen), + Offset: 1, + } + packer.AddFrame(f) + payloadFrames, err := packer.composeNextPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(len(payloadFrames)).To(Equal(1)) + payloadFrames, err = packer.composeNextPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(len(payloadFrames)).To(Equal(0)) + }) + + It("packs 2 stream frames that are too big for one packet correctly", func() { + maxStreamFrameDataLen := protocol.MaxFrameSize - (1 + 4 + 8 + 2) + f1 := &frames.StreamFrame{ + Data: bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen+100), + Offset: 1, + } + f2 := &frames.StreamFrame{ + Data: bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen+100), + Offset: 1, + } + packer.AddFrame(f1) + packer.AddFrame(f2) + p, err := packer.PackPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(len(p.raw)).To(Equal(protocol.MaxPacketSize)) + p, err = packer.PackPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(len(p.raw)).To(Equal(protocol.MaxPacketSize)) + p, err = packer.PackPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(p).ToNot(BeNil()) + }) + + It("packs a packet that has the maximum packet size when given a large enough stream frame", func() { + f := &frames.StreamFrame{ + Data: bytes.Repeat([]byte{'f'}, protocol.MaxFrameSize-(1+4+8+2)), + Offset: 1, + } + packer.AddFrame(f) + p, err := packer.PackPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(p).ToNot(BeNil()) + Expect(len(p.raw)).To(Equal(protocol.MaxPacketSize)) + }) + + It("splits a stream frame larger than the maximum size", func() { + f := &frames.StreamFrame{ + Data: bytes.Repeat([]byte{'f'}, protocol.MaxFrameSize-(1+4+8+2)+1), + Offset: 1, + } + packer.AddFrame(f) + payloadFrames, err := packer.composeNextPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(len(payloadFrames)).To(Equal(1)) + payloadFrames, err = packer.composeNextPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(len(payloadFrames)).To(Equal(1)) + }) }) })