diff --git a/frames/ack_frame_new.go b/frames/ack_frame_new.go index b86407fd..ec8f7099 100644 --- a/frames/ack_frame_new.go +++ b/frames/ack_frame_new.go @@ -222,10 +222,10 @@ func (f *AckFrameNew) Write(b *bytes.Buffer, version protocol.VersionNumber) err utils.WriteUfloat16(b, uint64(f.DelayTime/time.Microsecond)) var numRanges uint64 - var numRangesWritten uint64 // to check for internal consistency + var numRangesWritten uint64 if f.HasMissingRanges() { - numRanges = f.numWrittenNackRanges() - if numRanges >= 0xFF { + numRanges = f.numWritableNackRanges() + if numRanges > 0xFF { panic("AckFrame: Too many ACK ranges") } b.WriteByte(uint8(numRanges - 1)) @@ -280,6 +280,11 @@ func (f *AckFrameNew) Write(b *bytes.Buffer, version protocol.VersionNumber) err numRangesWritten++ } } + + // this is needed if not all AckRanges can be written to the ACK frame (if there are more than 0xFF) + if numRangesWritten >= numRanges { + break + } } if numRanges != numRangesWritten { @@ -300,7 +305,7 @@ func (f *AckFrameNew) MinLength(version protocol.VersionNumber) (protocol.ByteCo missingSequenceNumberDeltaLen := protocol.ByteCount(protocol.PacketNumberLen6) if f.HasMissingRanges() { - length += (1 + missingSequenceNumberDeltaLen) * protocol.ByteCount(f.numWrittenNackRanges()) + length += (1 + missingSequenceNumberDeltaLen) * protocol.ByteCount(f.numWritableNackRanges()) } else { length += missingSequenceNumberDeltaLen } @@ -356,9 +361,9 @@ func (f *AckFrameNew) validateAckRanges() bool { return true } -// numWrittenNackRanges calculates the number of ACK blocks that are about to be written +// numWritableNackRanges calculates the number of ACK blocks that are about to be written // this number is different from len(f.AckRanges) for the case of long gaps (> 255 packets) -func (f *AckFrameNew) numWrittenNackRanges() uint64 { +func (f *AckFrameNew) numWritableNackRanges() uint64 { if len(f.AckRanges) == 0 { return 0 } @@ -371,9 +376,15 @@ func (f *AckFrameNew) numWrittenNackRanges() uint64 { lastAckRange := f.AckRanges[i-1] gap := lastAckRange.FirstPacketNumber - ackRange.LastPacketNumber - numRanges += 1 + uint64(gap)/(0xFF+1) - if uint64(gap)%(0xFF+1) == 0 { - numRanges-- + rangeLength := uint64(gap) / (0xFF + 1) + if uint64(gap)%(0xFF+1) != 0 { + rangeLength++ + } + + if numRanges+rangeLength < 0xFF { + numRanges += rangeLength + } else { + break } } diff --git a/frames/ack_frame_new_test.go b/frames/ack_frame_new_test.go index 0dd097d3..ac7482d6 100644 --- a/frames/ack_frame_new_test.go +++ b/frames/ack_frame_new_test.go @@ -387,7 +387,7 @@ var _ = Describe("AckFrame", func() { AckRange{FirstPacketNumber: 1, LastPacketNumber: 19}, }, } - Expect(frameOrig.numWrittenNackRanges()).To(Equal(uint64(2))) + Expect(frameOrig.numWritableNackRanges()).To(Equal(uint64(2))) err := frameOrig.Write(b, 0) Expect(err).ToNot(HaveOccurred()) r := bytes.NewReader(b.Bytes()) @@ -406,7 +406,7 @@ var _ = Describe("AckFrame", func() { AckRange{FirstPacketNumber: 1, LastPacketNumber: 19}, }, } - Expect(frameOrig.numWrittenNackRanges()).To(Equal(uint64(2))) + Expect(frameOrig.numWritableNackRanges()).To(Equal(uint64(2))) err := frameOrig.Write(b, 0) Expect(err).ToNot(HaveOccurred()) r := bytes.NewReader(b.Bytes()) @@ -425,7 +425,7 @@ var _ = Describe("AckFrame", func() { AckRange{FirstPacketNumber: 1, LastPacketNumber: 19}, }, } - Expect(frameOrig.numWrittenNackRanges()).To(Equal(uint64(3))) + Expect(frameOrig.numWritableNackRanges()).To(Equal(uint64(3))) err := frameOrig.Write(b, 0) Expect(err).ToNot(HaveOccurred()) // Expect(b.Bytes()[13+0*(1+6) : 13+1*(1+6)]).To(Equal([]byte{0xFF, 0, 0, 0, 0, 0, 0})) @@ -474,6 +474,72 @@ var _ = Describe("AckFrame", func() { Expect(frame.AckRanges).To(Equal(frameOrig.AckRanges)) }) }) + + Context("too many ACK blocks", func() { + It("skips the lowest ACK ranges, if there are more than 255 AckRanges", func() { + ackRanges := make([]AckRange, 300) + for i := 1; i <= 300; i++ { + ackRanges[300-i] = AckRange{FirstPacketNumber: protocol.PacketNumber(3 * i), LastPacketNumber: protocol.PacketNumber(3*i + 1)} + } + frameOrig := &AckFrameNew{ + LargestAcked: ackRanges[0].LastPacketNumber, + LowestAcked: ackRanges[len(ackRanges)-1].FirstPacketNumber, + AckRanges: ackRanges, + } + err := frameOrig.Write(b, 0) + Expect(err).ToNot(HaveOccurred()) + r := bytes.NewReader(b.Bytes()) + frame, err := ParseAckFrameNew(r, 0) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.LargestAcked).To(Equal(frameOrig.LargestAcked)) + Expect(frame.LowestAcked).To(Equal(ackRanges[254].FirstPacketNumber)) + Expect(frame.AckRanges).To(HaveLen(0xFF)) + Expect(frame.validateAckRanges()).To(BeTrue()) + }) + + It("skips the lowest ACK ranges, if the gaps are large", func() { + ackRanges := make([]AckRange, 100) + // every AckRange will take 4 written ACK ranges + for i := 1; i <= 100; i++ { + ackRanges[100-i] = AckRange{FirstPacketNumber: protocol.PacketNumber(1000 * i), LastPacketNumber: protocol.PacketNumber(1000*i + 1)} + } + frameOrig := &AckFrameNew{ + LargestAcked: ackRanges[0].LastPacketNumber, + LowestAcked: ackRanges[len(ackRanges)-1].FirstPacketNumber, + AckRanges: ackRanges, + } + err := frameOrig.Write(b, 0) + Expect(err).ToNot(HaveOccurred()) + r := bytes.NewReader(b.Bytes()) + frame, err := ParseAckFrameNew(r, 0) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.LargestAcked).To(Equal(frameOrig.LargestAcked)) + Expect(frame.LowestAcked).To(Equal(ackRanges[255/4].FirstPacketNumber)) + Expect(frame.validateAckRanges()).To(BeTrue()) + }) + + It("works with huge gaps", func() { + ackRanges := []AckRange{ + AckRange{FirstPacketNumber: 2 * 255 * 200, LastPacketNumber: 2*255*200 + 1}, + AckRange{FirstPacketNumber: 1 * 255 * 200, LastPacketNumber: 1*255*200 + 1}, + AckRange{FirstPacketNumber: 1, LastPacketNumber: 2}, + } + frameOrig := &AckFrameNew{ + LargestAcked: ackRanges[0].LastPacketNumber, + LowestAcked: ackRanges[len(ackRanges)-1].FirstPacketNumber, + AckRanges: ackRanges, + } + err := frameOrig.Write(b, 0) + Expect(err).ToNot(HaveOccurred()) + r := bytes.NewReader(b.Bytes()) + frame, err := ParseAckFrameNew(r, 0) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.LargestAcked).To(Equal(frameOrig.LargestAcked)) + Expect(frame.AckRanges).To(HaveLen(2)) + Expect(frame.LowestAcked).To(Equal(ackRanges[1].FirstPacketNumber)) + Expect(frame.validateAckRanges()).To(BeTrue()) + }) + }) }) Context("min length", func() {