detect packets smaller than LeastUnacked in ReceivedPacketHandler

ref #196

ackhandlerlegacy/received_packet_handler.go

@@ -9,8 +9,12 @@ import (
 	"github.com/lucas-clemente/quic-go/qerr"
 )
 
-// ErrDuplicatePacket occurres when a duplicate packet is received
-var ErrDuplicatePacket = errors.New("ReceivedPacketHandler: Duplicate Packet")
+var (
+	// ErrDuplicatePacket occurres when a duplicate packet is received
+	ErrDuplicatePacket = errors.New("ReceivedPacketHandler: Duplicate Packet")
+	// ErrPacketSmallerThanLastStopWaiting occurs when a packet arrives with a packet number smaller than the largest LeastUnacked of a StopWaitingFrame. If this error occurs, the packet should be ignored
+	ErrPacketSmallerThanLastStopWaiting = errors.New("ReceivedPacketHandler: Packet number smaller than highest StopWaiting")
+)
 
 var (
 	errInvalidPacketNumber               = errors.New("ReceivedPacketHandler: Invalid packet number")
@@ -26,6 +30,7 @@ type receivedPacketHandler struct {
 	highestInOrderObserved        protocol.PacketNumber
 	highestInOrderObservedEntropy EntropyAccumulator
 	largestObserved               protocol.PacketNumber
+	ignorePacketsBelow            protocol.PacketNumber
 	currentAckFrame               *frames.AckFrameLegacy
 	stateChanged                  bool // has an ACK for this state already been sent? Will be set to false every time a new packet arrives, and to false every time an ACK is sent
 
@@ -44,6 +49,13 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe
 	if packetNumber == 0 {
 		return errInvalidPacketNumber
 	}
+
+	// if the packet number is smaller than the largest LeastUnacked value of a StopWaiting we received, we cannot detect if this packet has a duplicate number
+	// the packet has to be ignored anyway
+	if packetNumber <= h.ignorePacketsBelow {
+		return ErrPacketSmallerThanLastStopWaiting
+	}
+
 	_, ok := h.packetHistory[packetNumber]
 	if packetNumber <= h.highestInOrderObserved || ok {
 		return ErrDuplicatePacket
@@ -76,7 +88,14 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe
 }
 
 func (h *receivedPacketHandler) ReceivedStopWaiting(f *frames.StopWaitingFrame) error {
-	// Ignore if STOP_WAITING is unneeded
+	// ignore if StopWaiting is unneeded, because we already received a StopWaiting with a higher LeastUnacked
+	if h.ignorePacketsBelow >= f.LeastUnacked {
+		return nil
+	}
+
+	h.ignorePacketsBelow = f.LeastUnacked - 1
+
+	// ignore if StopWaiting is unneeded, since all packets below have already been received
 	if h.highestInOrderObserved >= f.LeastUnacked {
 		return nil
 	}

ackhandlerlegacy/received_packet_handler_test.go

@@ -57,6 +57,26 @@ var _ = Describe("receivedPacketHandler", func() {
 			Expect(err).To(MatchError(ErrDuplicatePacket))
 		})
 
+		It("ignores a packet with PacketNumber less than the LeastUnacked of a previously received StopWaiting", func() {
+			err := handler.ReceivedPacket(5, false)
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10})
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedPacket(9, false)
+			Expect(err).To(MatchError(ErrPacketSmallerThanLastStopWaiting))
+			Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(9)))
+		})
+
+		It("does not ignore a packet with PacketNumber equal to LeastUnacked of a previously received StopWaiting", func() {
+			err := handler.ReceivedPacket(5, false)
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10})
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedPacket(10, false)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(10)))
+		})
+
 		It("saves the time when each packet arrived", func() {
 			err := handler.ReceivedPacket(protocol.PacketNumber(3), false)
 			Expect(err).ToNot(HaveOccurred())
@@ -245,6 +265,32 @@ var _ = Describe("receivedPacketHandler", func() {
 			ranges, _ = handler.getNackRanges()
 			Expect(ranges).To(BeEmpty())
 		})
+
+		It("increases the ignorePacketsBelow number", func() {
+			err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
+		})
+
+		It("increase the ignorePacketsBelow number, even if all packets below the LeastUnacked were already acked", func() {
+			for i := 1; i < 20; i++ {
+				err := handler.ReceivedPacket(protocol.PacketNumber(i), false)
+				Expect(err).ToNot(HaveOccurred())
+			}
+			Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(19)))
+			err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
+		})
+
+		It("does not decrease the ignorePacketsBelow number when an out-of-order StopWaiting arrives", func() {
+			err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
+			err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(6)})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
+		})
 	})
 
 	Context("ACK package generation", func() {

ackhandlernew/received_packet_handler.go

@@ -9,8 +9,12 @@ import (
 	"github.com/lucas-clemente/quic-go/qerr"
 )
 
-// ErrDuplicatePacket occurres when a duplicate packet is received
-var ErrDuplicatePacket = errors.New("ReceivedPacketHandler: Duplicate Packet")
+var (
+	// ErrDuplicatePacket occurres when a duplicate packet is received
+	ErrDuplicatePacket = errors.New("ReceivedPacketHandler: Duplicate Packet")
+	// ErrPacketSmallerThanLastStopWaiting occurs when a packet arrives with a packet number smaller than the largest LeastUnacked of a StopWaitingFrame. If this error occurs, the packet should be ignored
+	ErrPacketSmallerThanLastStopWaiting = errors.New("ReceivedPacketHandler: Packet number smaller than highest StopWaiting")
+)
 
 var (
 	errInvalidPacketNumber               = errors.New("ReceivedPacketHandler: Invalid packet number")
@@ -20,6 +24,7 @@ var (
 type receivedPacketHandler struct {
 	largestInOrderObserved protocol.PacketNumber
 	largestObserved        protocol.PacketNumber
+	ignorePacketsBelow     protocol.PacketNumber
 	currentAckFrame        *frames.AckFrameNew
 	stateChanged           bool // has an ACK for this state already been sent? Will be set to false every time a new packet arrives, and to false every time an ACK is sent
 
@@ -40,6 +45,13 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe
 	if packetNumber == 0 {
 		return errInvalidPacketNumber
 	}
+
+	// if the packet number is smaller than the largest LeastUnacked value of a StopWaiting we received, we cannot detect if this packet has a duplicate number
+	// the packet has to be ignored anyway
+	if packetNumber <= h.ignorePacketsBelow {
+		return ErrPacketSmallerThanLastStopWaiting
+	}
+
 	_, ok := h.receivedTimes[packetNumber]
 	if packetNumber <= h.largestInOrderObserved || ok {
 		return ErrDuplicatePacket
@@ -70,7 +82,14 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe
 }
 
 func (h *receivedPacketHandler) ReceivedStopWaiting(f *frames.StopWaitingFrame) error {
-	// Ignore if STOP_WAITING is unneeded
+	// ignore if StopWaiting is unneeded, because we already received a StopWaiting with a higher LeastUnacked
+	if h.ignorePacketsBelow >= f.LeastUnacked {
+		return nil
+	}
+
+	h.ignorePacketsBelow = f.LeastUnacked - 1
+
+	// ignore if StopWaiting is unneeded, since all packets below have already been received
 	if h.largestInOrderObserved >= f.LeastUnacked {
 		return nil
 	}

ackhandlernew/received_packet_handler_test.go

@@ -55,6 +55,26 @@ var _ = Describe("receivedPacketHandler", func() {
 			Expect(err).To(MatchError(ErrDuplicatePacket))
 		})
 
+		It("ignores a packet with PacketNumber less than the LeastUnacked of a previously received StopWaiting", func() {
+			err := handler.ReceivedPacket(5)
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10})
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedPacket(9)
+			Expect(err).To(MatchError(ErrPacketSmallerThanLastStopWaiting))
+			Expect(handler.largestInOrderObserved).To(Equal(protocol.PacketNumber(9)))
+		})
+
+		It("does not ignore a packet with PacketNumber equal to LeastUnacked of a previously received StopWaiting", func() {
+			err := handler.ReceivedPacket(5)
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10})
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedPacket(10)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.largestInOrderObserved).To(Equal(protocol.PacketNumber(10)))
+		})
+
 		It("saves the time when each packet arrived", func() {
 			err := handler.ReceivedPacket(protocol.PacketNumber(3))
 			Expect(err).ToNot(HaveOccurred())
@@ -87,6 +107,32 @@ var _ = Describe("receivedPacketHandler", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(handler.largestInOrderObserved).To(Equal(protocol.PacketNumber(11)))
 		})
+
+		It("increases the ignorePacketsBelow number", func() {
+			err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
+		})
+
+		It("increase the ignorePacketsBelow number, even if all packets below the LeastUnacked were already acked", func() {
+			for i := 1; i < 20; i++ {
+				err := handler.ReceivedPacket(protocol.PacketNumber(i))
+				Expect(err).ToNot(HaveOccurred())
+			}
+			Expect(handler.largestInOrderObserved).To(Equal(protocol.PacketNumber(19)))
+			err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
+		})
+
+		It("does not decrease the ignorePacketsBelow number when an out-of-order StopWaiting arrives", func() {
+			err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
+			err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(6)})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
+		})
 	})
 
 	Context("ACK package generation", func() {