diff --git a/internal/wire/stream_frame.go b/internal/wire/stream_frame.go index dfc1d1ad..65269cf0 100644 --- a/internal/wire/stream_frame.go +++ b/internal/wire/stream_frame.go @@ -143,15 +143,18 @@ func (f *StreamFrame) MaxDataLen(maxSize protocol.ByteCount, version protocol.Ve } // MaybeSplitOffFrame splits a frame such that it is not bigger than n bytes. -// If n >= len(frame), nil is returned and nothing is modified. -func (f *StreamFrame) MaybeSplitOffFrame(maxSize protocol.ByteCount, version protocol.VersionNumber) (*StreamFrame, error) { +// It returns if the frame was actually split. +// The frame might not be split if: +// * the size is large enough to fit the whole frame +// * the size is too small to fit even a 1-byte frame. In that case, the frame returned is nil. +func (f *StreamFrame) MaybeSplitOffFrame(maxSize protocol.ByteCount, version protocol.VersionNumber) (*StreamFrame, bool /* was splitting required */) { if maxSize >= f.Length(version) { - return nil, nil + return nil, false } n := f.MaxDataLen(maxSize, version) if n == 0 { - return nil, errors.New("too small") + return nil, true } newFrame := &StreamFrame{ FinBit: false, @@ -164,5 +167,5 @@ func (f *StreamFrame) MaybeSplitOffFrame(maxSize protocol.ByteCount, version pro f.Data = f.Data[n:] f.Offset += n - return newFrame, nil + return newFrame, true } diff --git a/internal/wire/stream_frame_test.go b/internal/wire/stream_frame_test.go index 4675f71f..3b115203 100644 --- a/internal/wire/stream_frame_test.go +++ b/internal/wire/stream_frame_test.go @@ -287,12 +287,13 @@ var _ = Describe("STREAM frame", func() { Offset: 0xdeadbeef, Data: make([]byte, 100), } - newFrame, err := f.MaybeSplitOffFrame(f.Length(versionIETFFrames), versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) - Expect(newFrame).To(BeNil()) - newFrame, err = f.MaybeSplitOffFrame(f.Length(versionIETFFrames)-1, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) - Expect(newFrame).ToNot(BeNil()) + frame, needsSplit := f.MaybeSplitOffFrame(f.Length(versionIETFFrames), versionIETFFrames) + Expect(needsSplit).To(BeFalse()) + Expect(frame).To(BeNil()) + Expect(f.DataLen()).To(BeEquivalentTo(100)) + frame, needsSplit = f.MaybeSplitOffFrame(f.Length(versionIETFFrames)-1, versionIETFFrames) + Expect(needsSplit).To(BeTrue()) + Expect(frame.DataLen()).To(BeEquivalentTo(99)) }) It("keeps the data len", func() { @@ -301,11 +302,11 @@ var _ = Describe("STREAM frame", func() { DataLenPresent: true, Data: make([]byte, 100), } - newFrame, err := f.MaybeSplitOffFrame(66, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) - Expect(newFrame).ToNot(BeNil()) + frame, needsSplit := f.MaybeSplitOffFrame(66, versionIETFFrames) + Expect(needsSplit).To(BeTrue()) + Expect(frame).ToNot(BeNil()) Expect(f.DataLenPresent).To(BeTrue()) - Expect(newFrame.DataLenPresent).To(BeTrue()) + Expect(frame.DataLenPresent).To(BeTrue()) }) It("adjusts the offset", func() { @@ -314,11 +315,11 @@ var _ = Describe("STREAM frame", func() { Offset: 0x100, Data: []byte("foobar"), } - newFrame, err := f.MaybeSplitOffFrame(f.Length(versionIETFFrames)-3, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) - Expect(newFrame).ToNot(BeNil()) - Expect(newFrame.Offset).To(Equal(protocol.ByteCount(0x100))) - Expect(newFrame.Data).To(Equal([]byte("foo"))) + frame, needsSplit := f.MaybeSplitOffFrame(f.Length(versionIETFFrames)-3, versionIETFFrames) + Expect(needsSplit).To(BeTrue()) + Expect(frame).ToNot(BeNil()) + Expect(frame.Offset).To(Equal(protocol.ByteCount(0x100))) + Expect(frame.Data).To(Equal([]byte("foo"))) Expect(f.Offset).To(Equal(protocol.ByteCount(0x100 + 3))) Expect(f.Data).To(Equal([]byte("bar"))) }) @@ -330,12 +331,12 @@ var _ = Describe("STREAM frame", func() { Offset: 0xdeadbeef, Data: make([]byte, 100), } - newFrame, err := f.MaybeSplitOffFrame(50, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) - Expect(newFrame).ToNot(BeNil()) - Expect(newFrame.Offset).To(BeNumerically("<", f.Offset)) + frame, needsSplit := f.MaybeSplitOffFrame(50, versionIETFFrames) + Expect(needsSplit).To(BeTrue()) + Expect(frame).ToNot(BeNil()) + Expect(frame.Offset).To(BeNumerically("<", f.Offset)) Expect(f.FinBit).To(BeTrue()) - Expect(newFrame.FinBit).To(BeFalse()) + Expect(frame.FinBit).To(BeFalse()) }) It("produces frames of the correct length, without data len", func() { @@ -347,14 +348,15 @@ var _ = Describe("STREAM frame", func() { } minFrameSize := f.Length(versionIETFFrames) for i := protocol.ByteCount(0); i < minFrameSize; i++ { - _, err := f.MaybeSplitOffFrame(i, versionIETFFrames) - Expect(err).To(HaveOccurred()) + f, needsSplit := f.MaybeSplitOffFrame(i, versionIETFFrames) + Expect(needsSplit).To(BeTrue()) + Expect(f).To(BeNil()) } for i := minFrameSize; i < size; i++ { f.Data = make([]byte, size) - newFrame, err := f.MaybeSplitOffFrame(i, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) - Expect(newFrame.Length(versionIETFFrames)).To(Equal(i)) + f, needsSplit := f.MaybeSplitOffFrame(i, versionIETFFrames) + Expect(needsSplit).To(BeTrue()) + Expect(f.Length(versionIETFFrames)).To(Equal(i)) } }) @@ -368,14 +370,15 @@ var _ = Describe("STREAM frame", func() { } minFrameSize := f.Length(versionIETFFrames) for i := protocol.ByteCount(0); i < minFrameSize; i++ { - _, err := f.MaybeSplitOffFrame(i, versionIETFFrames) - Expect(err).To(HaveOccurred()) + f, needsSplit := f.MaybeSplitOffFrame(i, versionIETFFrames) + Expect(needsSplit).To(BeTrue()) + Expect(f).To(BeNil()) } var frameOneByteTooSmallCounter int for i := minFrameSize; i < size; i++ { f.Data = make([]byte, size) - newFrame, err := f.MaybeSplitOffFrame(i, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) + newFrame, needsSplit := f.MaybeSplitOffFrame(i, versionIETFFrames) + Expect(needsSplit).To(BeTrue()) // There's *one* pathological case, where a data length of x can be encoded into 1 byte // but a data lengths of x+1 needs 2 bytes // In that case, it's impossible to create a STREAM frame of the desired size diff --git a/packet_packer.go b/packet_packer.go index 8a4548a2..e2585d9c 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -284,11 +284,11 @@ func (p *packetPacker) PackRetransmission(packet *ackhandler.Packet) ([]*packedP frame.DataLenPresent = false frameToAdd := frame - sf, err := frame.MaybeSplitOffFrame(maxSize-length, p.version) - if err != nil { - return nil, err - } - if sf != nil { + sf, needsSplit := frame.MaybeSplitOffFrame(maxSize-length, p.version) + if needsSplit { + if sf == nil { // size too small to create a new STREAM frame + continue + } frameToAdd = sf } else { streamFrames = streamFrames[1:]