diff --git a/ackhandler/incoming_packet_ack_handler.go b/ackhandler/incoming_packet_ack_handler.go index 794db2e76..234b1912f 100644 --- a/ackhandler/incoming_packet_ack_handler.go +++ b/ackhandler/incoming_packet_ack_handler.go @@ -1,10 +1,14 @@ package ackhandler import ( + "errors" + "github.com/lucas-clemente/quic-go/frames" "github.com/lucas-clemente/quic-go/protocol" ) +var ErrDuplicatePacket = errors.New("Duplicate Packet") + // The AckHandler handles ACKs type incomingPacketAckHandler struct { largestObserved protocol.PacketNumber @@ -18,11 +22,19 @@ func NewIncomingPacketAckHandler() IncomingPacketAckHandler { } } -func (h *incomingPacketAckHandler) ReceivedPacket(packetNumber protocol.PacketNumber, entropyBit bool) { +func (h *incomingPacketAckHandler) ReceivedPacket(packetNumber protocol.PacketNumber, entropyBit bool) error { + if packetNumber == 0 { + return errors.New("Invalid packet number") + } + if h.observed[packetNumber] { + return ErrDuplicatePacket + } + if packetNumber > h.largestObserved { h.largestObserved = packetNumber } h.observed[packetNumber] = true + return nil } // GetNackRanges gets all the NACK ranges @@ -30,15 +42,13 @@ func (h *incomingPacketAckHandler) GetNackRanges() []*frames.NackRange { // ToDo: improve performance var ranges []*frames.NackRange inRange := false - // ToDo: fix types - for i := 0; i < int(h.largestObserved); i++ { - packetNumber := protocol.PacketNumber(i) - _, ok := h.observed[packetNumber] + for i := protocol.PacketNumber(1); i < h.largestObserved; i++ { + _, ok := h.observed[i] if !ok { if !inRange { r := &frames.NackRange{ - FirstPacketNumber: packetNumber, - LastPacketNumber: packetNumber, + FirstPacketNumber: i, + LastPacketNumber: i, } ranges = append(ranges, r) inRange = true diff --git a/ackhandler/incoming_packet_ack_handler_test.go b/ackhandler/incoming_packet_ack_handler_test.go index 60fe624c2..b92615240 100644 --- a/ackhandler/incoming_packet_ack_handler_test.go +++ b/ackhandler/incoming_packet_ack_handler_test.go @@ -15,15 +15,16 @@ var _ = Describe("incomingPacketAckHandler", func() { }) It("Returns no NACK ranges for continously received packets", func() { - for i := 0; i < 100; i++ { - handler.ReceivedPacket(protocol.PacketNumber(i), false) + for i := 1; i < 100; i++ { + err := handler.ReceivedPacket(protocol.PacketNumber(i), false) + Expect(err).ToNot(HaveOccurred()) } Expect(handler.largestObserved).To(Equal(protocol.PacketNumber(99))) Expect(len(handler.GetNackRanges())).To(Equal(0)) }) It("handles a single lost package", func() { - for i := 0; i < 10; i++ { + for i := 1; i < 10; i++ { if i == 5 { continue } @@ -37,7 +38,7 @@ var _ = Describe("incomingPacketAckHandler", func() { }) It("handles two consecutive lost packages", func() { - for i := 0; i < 10; i++ { + for i := 1; i < 10; i++ { if i == 5 || i == 6 { continue } @@ -51,7 +52,7 @@ var _ = Describe("incomingPacketAckHandler", func() { }) It("handles two non-consecutively lost packages", func() { - for i := 0; i < 10; i++ { + for i := 1; i < 10; i++ { if i == 3 || i == 7 { continue } @@ -67,7 +68,7 @@ var _ = Describe("incomingPacketAckHandler", func() { }) It("handles two sequences of lost packages", func() { - for i := 0; i < 10; i++ { + for i := 1; i < 10; i++ { if i == 2 || i == 3 || i == 4 || i == 7 || i == 8 { continue } @@ -82,4 +83,35 @@ var _ = Describe("incomingPacketAckHandler", func() { Expect(nackRanges[1].LastPacketNumber).To(Equal(protocol.PacketNumber(8))) }) + It("handles a packet that arrives late", func() { + err := handler.ReceivedPacket(protocol.PacketNumber(1), false) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedPacket(protocol.PacketNumber(3), false) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedPacket(protocol.PacketNumber(2), false) + Expect(err).ToNot(HaveOccurred()) + nackRanges := handler.GetNackRanges() + Expect(len(nackRanges)).To(Equal(0)) + }) + + It("rejects a duplicate package with PacketNumber equal to LargestObserved", func() { + for i := 1; i < 5; i++ { + err := handler.ReceivedPacket(protocol.PacketNumber(i), false) + Expect(err).ToNot(HaveOccurred()) + } + err := handler.ReceivedPacket(4, false) + Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(ErrDuplicatePacket)) + }) + + It("rejects a duplicate package with PacketNumber less than the LargestObserved", func() { + for i := 1; i < 5; i++ { + err := handler.ReceivedPacket(protocol.PacketNumber(i), false) + Expect(err).ToNot(HaveOccurred()) + } + err := handler.ReceivedPacket(2, false) + Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(ErrDuplicatePacket)) + }) + }) diff --git a/ackhandler/interfaces.go b/ackhandler/interfaces.go index 849d43687..df36add0d 100644 --- a/ackhandler/interfaces.go +++ b/ackhandler/interfaces.go @@ -5,6 +5,7 @@ import ( "github.com/lucas-clemente/quic-go/protocol" ) +// OutgoingPacketAckHandler handles ACKs received for outgoing packets type OutgoingPacketAckHandler interface { SentPacket(packet *Packet) error ReceivedAck(ackFrame *frames.AckFrame) error @@ -12,8 +13,9 @@ type OutgoingPacketAckHandler interface { DequeuePacketForRetransmission() (packet *Packet) } +// IncomingPacketAckHandler handles ACKs needed to send for incoming packets type IncomingPacketAckHandler interface { - ReceivedPacket(packetNumber protocol.PacketNumber, entropyBit bool) + ReceivedPacket(packetNumber protocol.PacketNumber, entropyBit bool) error DequeueAckFrame() *frames.AckFrame }