diff --git a/frames/ack_frame_new.go b/frames/ack_frame_new.go index c8c55b4e..fa084bad 100644 --- a/frames/ack_frame_new.go +++ b/frames/ack_frame_new.go @@ -9,7 +9,13 @@ import ( "github.com/lucas-clemente/quic-go/utils" ) -var errInvalidAckRanges = errors.New("AckFrame: ACK frame contains invalid ACK ranges") +// ErrInvalidAckRanges occurs when a client sends inconsistent ACK ranges +var ErrInvalidAckRanges = errors.New("AckFrame: ACK frame contains invalid ACK ranges") + +var ( + errInconsistentAckLargestObserved = errors.New("internal inconsistency: LargestObserved does not match ACK ranges") + errInconsistentAckLowestAcked = errors.New("internal inconsistency: LowestAcked does not match ACK ranges") +) // An AckFrameNew is a ACK frame in QUIC c34 type AckFrameNew struct { @@ -73,7 +79,7 @@ func ParseAckFrameNew(r *bytes.Reader, version protocol.VersionNumber) (*AckFram } if ackBlockLength > largestObserved { - return nil, errInvalidAckRanges + return nil, ErrInvalidAckRanges } if hasMissingRanges { @@ -117,7 +123,7 @@ func ParseAckFrameNew(r *bytes.Reader, version protocol.VersionNumber) (*AckFram } if !frame.validateAckRanges() { - return nil, errInvalidAckRanges + return nil, ErrInvalidAckRanges } var numTimestampByte byte @@ -207,10 +213,10 @@ func (f *AckFrameNew) Write(b *bytes.Buffer, version protocol.VersionNumber) err utils.WriteUint48(b, uint64(f.LargestObserved-f.LowestAcked+1)) } else { if f.LargestObserved != f.AckRanges[0].LastPacketNumber { - return errors.New("internal inconsistency: LastPacketNumber does not match ACK ranges") + return errInconsistentAckLargestObserved } if f.LowestAcked != f.AckRanges[len(f.AckRanges)-1].FirstPacketNumber { - return errors.New("internal inconsistency: FirstPacketNumber does not match ACK ranges") + return errInconsistentAckLowestAcked } length := f.LargestObserved - f.AckRanges[0].FirstPacketNumber + 1 utils.WriteUint48(b, uint64(length)) diff --git a/frames/ack_frame_new_test.go b/frames/ack_frame_new_test.go index 5744f51a..baab25e0 100644 --- a/frames/ack_frame_new_test.go +++ b/frames/ack_frame_new_test.go @@ -59,7 +59,7 @@ var _ = Describe("AckFrame", func() { // Length: 0x1d => LowestAcked would be -1 b := bytes.NewReader([]byte{0x40, 0x1c, 0x8e, 0x0, 0x1d, 0x1, 0x1, 0x6b, 0x26, 0x3, 0x0}) _, err := ParseAckFrameNew(b, 0) - Expect(err).To(MatchError(errInvalidAckRanges)) + Expect(err).To(MatchError(ErrInvalidAckRanges)) }) Context("ACK blocks", func() { @@ -80,7 +80,7 @@ var _ = Describe("AckFrame", func() { // like the test before, but increased the last ACK range, such that the FirstPacketNumber would be negative b := bytes.NewReader([]byte{0x60, 0x18, 0x94, 0x1, 0x1, 0x3, 0x2, 0x15, 0x2, 0x1, 0x5c, 0xd5, 0x0, 0x0, 0x0, 0x95, 0x0}) _, err := ParseAckFrameNew(b, 0) - Expect(err).To(MatchError(errInvalidAckRanges)) + Expect(err).To(MatchError(ErrInvalidAckRanges)) }) It("parses a frame with multiple single packets missing", func() { @@ -284,6 +284,32 @@ var _ = Describe("AckFrame", func() { Expect(r.Len()).To(BeZero()) }) + It("rejects a frame with incorrect LargestObserved value", func() { + frame := &AckFrameNew{ + LargestObserved: 26, + LowestAcked: 1, + AckRanges: []AckRange{ + AckRange{FirstPacketNumber: 12, LastPacketNumber: 25}, + AckRange{FirstPacketNumber: 1, LastPacketNumber: 10}, + }, + } + err := frame.Write(b, 0) + Expect(err).To(MatchError(errInconsistentAckLargestObserved)) + }) + + It("rejects a frame with incorrect LargestObserved value", func() { + frame := &AckFrameNew{ + LargestObserved: 25, + LowestAcked: 2, + AckRanges: []AckRange{ + AckRange{FirstPacketNumber: 12, LastPacketNumber: 25}, + AckRange{FirstPacketNumber: 1, LastPacketNumber: 10}, + }, + } + err := frame.Write(b, 0) + Expect(err).To(MatchError(errInconsistentAckLowestAcked)) + }) + Context("longer ACK blocks", func() { It("only writes one block for 254 lost packets", func() { frameOrig := &AckFrameNew{