From 936d34f875ba21730d9b9a46e8c970af22545fa7 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 27 Apr 2016 13:55:34 +0700 Subject: [PATCH] implement contiguous NACK frame parsing --- frames/ack_frame.go | 48 ++++++++++++++++++++++-------------- frames/ack_frame_test.go | 53 ++++++++++++++++++++++++++++++++-------- 2 files changed, 72 insertions(+), 29 deletions(-) diff --git a/frames/ack_frame.go b/frames/ack_frame.go index 3823a9c8..1235bb33 100644 --- a/frames/ack_frame.go +++ b/frames/ack_frame.go @@ -85,7 +85,7 @@ func (f *AckFrame) HasNACK() bool { return false } -// GetHighestInOrderPacket gets the highest in order packet number that is confirmed by this ACK +// GetHighestInOrderPacketNumber gets the highest in order packet number that is confirmed by this ACK func (f *AckFrame) GetHighestInOrderPacketNumber() protocol.PacketNumber { if f.HasNACK() { return (f.NackRanges[len(f.NackRanges)-1].FirstPacketNumber - 1) @@ -168,7 +168,7 @@ func ParseAckFrame(r *bytes.Reader) (*AckFrame, error) { // Invalid NACK Handling: // NACKs contain a lot of offsets that require substractions of PacketNumbers. If an ACK contains invalid data, it is possible to underflow the uint64 used to store the PacketNumber - // ToDo: handle uint64 overflows + // TODO: handle uint64 overflows if hasNACK { var numRanges uint8 numRanges, err = r.ReadByte() @@ -188,24 +188,34 @@ func ParseAckFrame(r *bytes.Reader) (*AckFrame, error) { } rangeLength := uint8(rangeLengthByte) - nackRange := NackRange{} - if i == 0 { - if uint64(frame.LargestObserved) < missingPacketSequenceNumberDelta+uint64(rangeLength) { - return nil, errInvalidNackRanges - } - nackRange.FirstPacketNumber = frame.LargestObserved - protocol.PacketNumber(missingPacketSequenceNumberDelta+uint64(rangeLength)) - } else { - if missingPacketSequenceNumberDelta == 0 { - return nil, errors.New("ACK frame: Continues NACK ranges not yet implemented") - } - lastNackRange := frame.NackRanges[len(frame.NackRanges)-1] - if uint64(lastNackRange.FirstPacketNumber) <= missingPacketSequenceNumberDelta+uint64(rangeLength) { - return nil, errInvalidNackRanges - } - nackRange.FirstPacketNumber = lastNackRange.FirstPacketNumber - protocol.PacketNumber(missingPacketSequenceNumberDelta+uint64(rangeLength)) - 1 + if i == 0 && missingPacketSequenceNumberDelta == 0 { + return nil, errors.New("ACK frame: largest observed missing not yet implemented") + } + + // contiguous NACK range + if missingPacketSequenceNumberDelta == 0 { + nackRange := &frame.NackRanges[len(frame.NackRanges)-1] + if uint64(nackRange.FirstPacketNumber) <= uint64(rangeLength)+1 { + return nil, errInvalidNackRanges + } + nackRange.FirstPacketNumber = protocol.PacketNumber(uint64(nackRange.FirstPacketNumber) - uint64(rangeLength) - 1) + } else { + nackRange := NackRange{} + if i == 0 { + if uint64(frame.LargestObserved) < missingPacketSequenceNumberDelta+uint64(rangeLength) { + return nil, errInvalidNackRanges + } + nackRange.FirstPacketNumber = frame.LargestObserved - protocol.PacketNumber(missingPacketSequenceNumberDelta+uint64(rangeLength)) + } else { + lastNackRange := frame.NackRanges[len(frame.NackRanges)-1] + if uint64(lastNackRange.FirstPacketNumber) <= missingPacketSequenceNumberDelta+uint64(rangeLength) { + return nil, errInvalidNackRanges + } + nackRange.FirstPacketNumber = lastNackRange.FirstPacketNumber - protocol.PacketNumber(missingPacketSequenceNumberDelta+uint64(rangeLength)) - 1 + } + nackRange.LastPacketNumber = protocol.PacketNumber(uint64(nackRange.FirstPacketNumber) + uint64(rangeLength)) + frame.NackRanges = append(frame.NackRanges, nackRange) } - nackRange.LastPacketNumber = protocol.PacketNumber(uint64(nackRange.FirstPacketNumber) + uint64(rangeLength)) - frame.NackRanges = append(frame.NackRanges, nackRange) } } diff --git a/frames/ack_frame_test.go b/frames/ack_frame_test.go index 8fa5ca4d..119cd31d 100644 --- a/frames/ack_frame_test.go +++ b/frames/ack_frame_test.go @@ -43,8 +43,7 @@ var _ = Describe("AckFrame", func() { Expect(frame.HasNACK()).To(Equal(true)) Expect(len(frame.NackRanges)).To(Equal(1)) Expect(frame.LargestObserved).To(Equal(protocol.PacketNumber(3))) - Expect(frame.NackRanges[0].FirstPacketNumber).To(Equal(protocol.PacketNumber(1))) - Expect(frame.NackRanges[0].LastPacketNumber).To(Equal(protocol.PacketNumber(2))) + Expect(frame.NackRanges[0]).To(Equal(NackRange{FirstPacketNumber: 1, LastPacketNumber: 2})) Expect(b.Len()).To(Equal(0)) }) @@ -56,8 +55,7 @@ var _ = Describe("AckFrame", func() { Expect(frame.LargestObserved).To(Equal(protocol.PacketNumber(0xDECAFBAD1337))) Expect(frame.HasNACK()).To(Equal(true)) Expect(len(frame.NackRanges)).To(Equal(1)) - Expect(frame.NackRanges[0].FirstPacketNumber).To(Equal(protocol.PacketNumber(0xDECAFBAD1337 - 0xDEADBEEFCAFE - rangeLength))) - Expect(frame.NackRanges[0].LastPacketNumber).To(Equal(protocol.PacketNumber(0xDECAFBAD1337 - 0xDEADBEEFCAFE))) + Expect(frame.NackRanges[0]).To(Equal(NackRange{FirstPacketNumber: protocol.PacketNumber(0xDECAFBAD1337 - 0xDEADBEEFCAFE - rangeLength), LastPacketNumber: 0xDECAFBAD1337 - 0xDEADBEEFCAFE})) Expect(b.Len()).To(Equal(0)) }) @@ -68,12 +66,9 @@ var _ = Describe("AckFrame", func() { Expect(err).ToNot(HaveOccurred()) Expect(frame.HasNACK()).To(Equal(true)) Expect(len(frame.NackRanges)).To(Equal(3)) - Expect(frame.NackRanges[0].FirstPacketNumber).To(Equal(protocol.PacketNumber(8))) - Expect(frame.NackRanges[0].LastPacketNumber).To(Equal(protocol.PacketNumber(14))) - Expect(frame.NackRanges[1].FirstPacketNumber).To(Equal(protocol.PacketNumber(4))) - Expect(frame.NackRanges[1].LastPacketNumber).To(Equal(protocol.PacketNumber(6))) - Expect(frame.NackRanges[2].FirstPacketNumber).To(Equal(protocol.PacketNumber(2))) - Expect(frame.NackRanges[2].LastPacketNumber).To(Equal(protocol.PacketNumber(2))) + Expect(frame.NackRanges[0]).To(Equal(NackRange{FirstPacketNumber: 8, LastPacketNumber: 14})) + Expect(frame.NackRanges[1]).To(Equal(NackRange{FirstPacketNumber: 4, LastPacketNumber: 6})) + Expect(frame.NackRanges[2]).To(Equal(NackRange{FirstPacketNumber: 2, LastPacketNumber: 2})) Expect(b.Len()).To(Equal(0)) }) @@ -84,6 +79,44 @@ var _ = Describe("AckFrame", func() { Expect(err).To(HaveOccurred()) Expect(err).To(Equal(errInvalidNackRanges)) }) + + Context("contiguous NACK ranges", func() { + It("parses a frame with a contiguous NACK range spanning two fields", func() { + b := bytes.NewReader([]byte{0x64, 0x8, 0x2E, 0x01, 0x72, 0x1, 0x1, 0x0, 0xc0, 0x15, 0x0, 0x0, 0x2, 0x1, 0x2b, 0x0, 0xff}) + frame, err := ParseAckFrame(b) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.LargestObserved).To(Equal(protocol.PacketNumber(302))) + Expect(len(frame.NackRanges)).To(Equal(1)) + Expect(frame.NackRanges[0]).To(Equal(NackRange{FirstPacketNumber: 2, LastPacketNumber: 301})) + }) + + It("parses a frame with a contiguous NACK range spanning more than two fields", func() { + b := bytes.NewReader([]byte{0x64, 0x8, 0x16, 0x05, 0x72, 0x1, 0x1, 0x0, 0xc0, 0x15, 0x0, 0x0, 0x6, 0x1, 0x13, 0x0, 0xff, 0x0, 0xff, 0x0, 0xff, 0x0, 0xff, 0x0, 0xff}) + frame, err := ParseAckFrame(b) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.LargestObserved).To(Equal(protocol.PacketNumber(1302))) + Expect(len(frame.NackRanges)).To(Equal(1)) + Expect(frame.NackRanges[0]).To(Equal(NackRange{FirstPacketNumber: 2, LastPacketNumber: 1301})) + }) + + It("parses a frame with two contiguous NACK ranges", func() { + b := bytes.NewReader([]byte{0x64, 0x8, 0x23, 0x03, 0x72, 0x1, 0x1, 0x0, 0xc0, 0x15, 0x0, 0x0, 0x4, 0x1, 0x8f, 0x0, 0xff, 0x1, 0x8f, 0x0, 0xff}) + frame, err := ParseAckFrame(b) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.LargestObserved).To(Equal(protocol.PacketNumber(803))) + Expect(len(frame.NackRanges)).To(Equal(2)) + Expect(frame.NackRanges[0]).To(Equal(NackRange{FirstPacketNumber: 403, LastPacketNumber: 802})) + Expect(frame.NackRanges[1]).To(Equal(NackRange{FirstPacketNumber: 2, LastPacketNumber: 401})) + }) + + It("rejects a frame with an invalid NACK range", func() { + // LargestObserved: 280, but NACK range is 301 packets long + b := bytes.NewReader([]byte{0x64, 0x8, 0x18, 0x01, 0x72, 0x1, 0x1, 0x0, 0xc0, 0x15, 0x0, 0x0, 0x2, 0x1, 0x2b, 0x0, 0xff}) + _, err := ParseAckFrame(b) + Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(errInvalidNackRanges)) + }) + }) }) Context("GetHighestInOrderPacket", func() {