diff --git a/ackhandler/sent_packet_handler.go b/ackhandler/sent_packet_handler.go index 10b8c828f..d21bef449 100644 --- a/ackhandler/sent_packet_handler.go +++ b/ackhandler/sent_packet_handler.go @@ -17,7 +17,9 @@ var ( ErrDuplicateOrOutOfOrderAck = errors.New("SentPacketHandler: Duplicate or out-of-order ACK") // ErrTooManyTrackedSentPackets occurs when the sentPacketHandler has to keep track of too many packets ErrTooManyTrackedSentPackets = errors.New("Too many outstanding non-acked and non-retransmitted packets") - errAckForUnsentPacket = qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package") + // ErrAckForSkippedPacket occurs when the client sent an ACK for a packet number that we intentionally skipped + ErrAckForSkippedPacket = qerr.Error(qerr.InvalidAckData, "Received an ACK for a skipped packet number") + errAckForUnsentPacket = qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package") ) var errPacketNumberNotIncreasing = errors.New("Already sent a packet with a higher packet number.") @@ -166,6 +168,13 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *frames.AckFrame, withPacketNum return nil } + // check if it acks any packets that were skipped + for _, p := range h.skippedPackets { + if ackFrame.AcksPacket(p) { + return ErrAckForSkippedPacket + } + } + h.LargestAcked = ackFrame.LargestAcked var ackedPackets congestion.PacketVector diff --git a/ackhandler/sent_packet_handler_test.go b/ackhandler/sent_packet_handler_test.go index df3a7b278..8f9e2cbd7 100644 --- a/ackhandler/sent_packet_handler_test.go +++ b/ackhandler/sent_packet_handler_test.go @@ -207,6 +207,7 @@ var _ = Describe("SentPacketHandler", func() { {PacketNumber: 8, Frames: []frames.Frame{&streamFrame}, Length: 1}, {PacketNumber: 9, Frames: []frames.Frame{&streamFrame}, Length: 1}, {PacketNumber: 10, Frames: []frames.Frame{&streamFrame}, Length: 1}, + {PacketNumber: 12, Frames: []frames.Frame{&streamFrame}, Length: 1}, } for _, packet := range packets { handler.SentPacket(packet) @@ -264,6 +265,30 @@ var _ = Describe("SentPacketHandler", func() { Expect(handler.LargestAcked).To(Equal(protocol.PacketNumber(3))) Expect(handler.BytesInFlight()).To(Equal(protocol.ByteCount(len(packets) - 3))) }) + + It("rejects ACKs for skipped packets", func() { + ack := frames.AckFrame{ + LargestAcked: 12, + LowestAcked: 5, + } + err := handler.ReceivedAck(&ack, 1337) + Expect(err).To(MatchError(ErrAckForSkippedPacket)) + Expect(handler.LargestAcked).To(BeZero()) + }) + + It("accepts an ACK that correctly nacks a skipped packet", func() { + ack := frames.AckFrame{ + LargestAcked: 12, + LowestAcked: 5, + AckRanges: []frames.AckRange{ + {FirstPacketNumber: 12, LastPacketNumber: 12}, + {FirstPacketNumber: 5, LastPacketNumber: 10}, + }, + } + err := handler.ReceivedAck(&ack, 1337) + Expect(err).ToNot(HaveOccurred()) + Expect(handler.LargestAcked).ToNot(BeZero()) + }) }) Context("acks and nacks the right packets", func() { @@ -281,7 +306,7 @@ var _ = Describe("SentPacketHandler", func() { Expect(el.Value.MissingReports).To(BeZero()) el = el.Next() } - Expect(el).To(BeNil()) + Expect(el.Value.PacketNumber).To(Equal(protocol.PacketNumber(12))) }) It("ACKs all packets for an ACK frame with no missing packets", func() { @@ -300,7 +325,7 @@ var _ = Describe("SentPacketHandler", func() { el = el.Next() Expect(el.Value.PacketNumber).To(Equal(protocol.PacketNumber(10))) Expect(el.Value.MissingReports).To(BeZero()) - Expect(el.Next()).To(BeNil()) + Expect(el.Next().Value.PacketNumber).To(Equal(protocol.PacketNumber(12))) }) It("handles an ACK frame with one missing packet range", func() { @@ -327,7 +352,7 @@ var _ = Describe("SentPacketHandler", func() { el = el.Next() Expect(el.Value.PacketNumber).To(Equal(protocol.PacketNumber(10))) Expect(el.Value.MissingReports).To(BeZero()) - Expect(el.Next()).To(BeNil()) + Expect(el.Next().Value.PacketNumber).To(Equal(protocol.PacketNumber(12))) }) It("NACKs packets below the LowestAcked", func() {