From 05265bd3c50d89c84ca3ba715b5f295fc29f081e Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Fri, 2 Sep 2016 11:47:20 +0700
Subject: [PATCH] fix AckFrame writing with gap lengths which are a multiple of
 255

fixes #306
---
 frames/ack_frame.go      | 10 +++----
 frames/ack_frame_test.go | 57 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/frames/ack_frame.go b/frames/ack_frame.go
index 2690770fa..b68b4482d 100644
--- a/frames/ack_frame.go
+++ b/frames/ack_frame.go
@@ -287,7 +287,7 @@ func (f *AckFrame) Write(b *bytes.Buffer, version protocol.VersionNumber) error
 
 				if i == int(num)-1 { // last block
 					lengthWritten = uint64(length)
-					gapWritten = uint8(gap % 0xFF)
+					gapWritten = uint8(1 + ((gap - 1) % 255))
 				} else {
 					lengthWritten = 0
 					gapWritten = 0xFF
@@ -403,10 +403,10 @@ func (f *AckFrame) numWritableNackRanges() uint64 {
 		}
 
 		lastAckRange := f.AckRanges[i-1]
-		gap := lastAckRange.FirstPacketNumber - ackRange.LastPacketNumber
-		rangeLength := uint64(gap) / (0xFF + 1)
-		if uint64(gap)%(0xFF+1) != 0 {
-			rangeLength++
+		gap := lastAckRange.FirstPacketNumber - ackRange.LastPacketNumber - 1
+		rangeLength := 1 + uint64(gap)/0xFF
+		if uint64(gap)%0xFF == 0 {
+			rangeLength--
 		}
 
 		if numRanges+rangeLength < 0xFF {
diff --git a/frames/ack_frame_test.go b/frames/ack_frame_test.go
index e6c207728..1c86ff818 100644
--- a/frames/ack_frame_test.go
+++ b/frames/ack_frame_test.go
@@ -447,6 +447,63 @@ var _ = Describe("AckFrame", func() {
 					Expect(frame.AckRanges).To(Equal(frameOrig.AckRanges))
 				})
 
+				It("writes two blocks for 510 lost packets", func() {
+					frameOrig := &AckFrame{
+						LargestAcked: 600,
+						LowestAcked:  1,
+						AckRanges: []AckRange{
+							{FirstPacketNumber: 20 + 510, LastPacketNumber: 600},
+							{FirstPacketNumber: 1, LastPacketNumber: 19},
+						},
+					}
+					Expect(frameOrig.numWritableNackRanges()).To(Equal(uint64(3)))
+					err := frameOrig.Write(b, 0)
+					Expect(err).ToNot(HaveOccurred())
+					r := bytes.NewReader(b.Bytes())
+					frame, err := ParseAckFrame(r, 0)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(frame.LargestAcked).To(Equal(frameOrig.LargestAcked))
+					Expect(frame.AckRanges).To(Equal(frameOrig.AckRanges))
+				})
+
+				It("writes three blocks for 511 lost packets", func() {
+					frameOrig := &AckFrame{
+						LargestAcked: 600,
+						LowestAcked:  1,
+						AckRanges: []AckRange{
+							{FirstPacketNumber: 20 + 511, LastPacketNumber: 600},
+							{FirstPacketNumber: 1, LastPacketNumber: 19},
+						},
+					}
+					Expect(frameOrig.numWritableNackRanges()).To(Equal(uint64(4)))
+					err := frameOrig.Write(b, 0)
+					Expect(err).ToNot(HaveOccurred())
+					r := bytes.NewReader(b.Bytes())
+					frame, err := ParseAckFrame(r, 0)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(frame.LargestAcked).To(Equal(frameOrig.LargestAcked))
+					Expect(frame.AckRanges).To(Equal(frameOrig.AckRanges))
+				})
+
+				It("writes three blocks for 512 lost packets", func() {
+					frameOrig := &AckFrame{
+						LargestAcked: 600,
+						LowestAcked:  1,
+						AckRanges: []AckRange{
+							{FirstPacketNumber: 20 + 512, LastPacketNumber: 600},
+							{FirstPacketNumber: 1, LastPacketNumber: 19},
+						},
+					}
+					Expect(frameOrig.numWritableNackRanges()).To(Equal(uint64(4)))
+					err := frameOrig.Write(b, 0)
+					Expect(err).ToNot(HaveOccurred())
+					r := bytes.NewReader(b.Bytes())
+					frame, err := ParseAckFrame(r, 0)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(frame.LargestAcked).To(Equal(frameOrig.LargestAcked))
+					Expect(frame.AckRanges).To(Equal(frameOrig.AckRanges))
+				})
+
 				It("writes multiple blocks for a lot of lost packets", func() {
 					frameOrig := &AckFrame{
 						LargestAcked: 3000,