reject duplicate packets and packets with packet number 0

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

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))
+	})
+
 })

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
 }