ackhandler: reject duplicate packets in ReceivedPacket (#3568)

Before we process a packet, IsPotentiallyDuplicate is called.
This should filter out all duplicates before the reach ReceivedPacket.

internal/ackhandler/received_packet_handler.go

@@ -46,24 +46,26 @@ func (h *receivedPacketHandler) ReceivedPacket(
 	h.sentPackets.ReceivedPacket(encLevel)
 	switch encLevel {
 	case protocol.EncryptionInitial:
-		h.initialPackets.ReceivedPacket(pn, ecn, rcvTime, shouldInstigateAck)
+		return h.initialPackets.ReceivedPacket(pn, ecn, rcvTime, shouldInstigateAck)
 	case protocol.EncryptionHandshake:
-		h.handshakePackets.ReceivedPacket(pn, ecn, rcvTime, shouldInstigateAck)
+		return h.handshakePackets.ReceivedPacket(pn, ecn, rcvTime, shouldInstigateAck)
 	case protocol.Encryption0RTT:
 		if h.lowest1RTTPacket != protocol.InvalidPacketNumber && pn > h.lowest1RTTPacket {
 			return fmt.Errorf("received packet number %d on a 0-RTT packet after receiving %d on a 1-RTT packet", pn, h.lowest1RTTPacket)
 		}
-		h.appDataPackets.ReceivedPacket(pn, ecn, rcvTime, shouldInstigateAck)
+		return h.appDataPackets.ReceivedPacket(pn, ecn, rcvTime, shouldInstigateAck)
 	case protocol.Encryption1RTT:
 		if h.lowest1RTTPacket == protocol.InvalidPacketNumber || pn < h.lowest1RTTPacket {
 			h.lowest1RTTPacket = pn
 		}
+		if err := h.appDataPackets.ReceivedPacket(pn, ecn, rcvTime, shouldInstigateAck); err != nil {
+			return err
+		}
 		h.appDataPackets.IgnoreBelow(h.sentPackets.GetLowestPacketNotConfirmedAcked())
-		h.appDataPackets.ReceivedPacket(pn, ecn, rcvTime, shouldInstigateAck)
+		return nil
 	default:
 		panic(fmt.Sprintf("received packet with unknown encryption level: %s", encLevel))
 	}
-	return nil
 }
 
 func (h *receivedPacketHandler) DropPackets(encLevel protocol.EncryptionLevel) {

internal/ackhandler/received_packet_tracker.go

@@ -1,6 +1,7 @@
 package ackhandler
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/lucas-clemente/quic-go/internal/protocol"
@@ -48,9 +49,9 @@ func newReceivedPacketTracker(
 	}
 }
 
-func (h *receivedPacketTracker) ReceivedPacket(packetNumber protocol.PacketNumber, ecn protocol.ECN, rcvTime time.Time, shouldInstigateAck bool) {
-	if packetNumber < h.ignoreBelow {
-		return
+func (h *receivedPacketTracker) ReceivedPacket(packetNumber protocol.PacketNumber, ecn protocol.ECN, rcvTime time.Time, shouldInstigateAck bool) error {
+	if isNew := h.packetHistory.ReceivedPacket(packetNumber); !isNew {
+		return fmt.Errorf("recevedPacketTracker BUG: ReceivedPacket called for old / duplicate packet %d", packetNumber)
 	}
 
 	isMissing := h.isMissing(packetNumber)
@@ -59,7 +60,7 @@ func (h *receivedPacketTracker) ReceivedPacket(packetNumber protocol.PacketNumbe
 		h.largestObservedReceivedTime = rcvTime
 	}
 
-	if isNew := h.packetHistory.ReceivedPacket(packetNumber); isNew && shouldInstigateAck {
+	if shouldInstigateAck {
 		h.hasNewAck = true
 	}
 	if shouldInstigateAck {
@@ -74,6 +75,7 @@ func (h *receivedPacketTracker) ReceivedPacket(packetNumber protocol.PacketNumbe
 	case protocol.ECNCE:
 		h.ecnce++
 	}
+	return nil
 }
 
 // IgnoreBelow sets a lower limit for acknowledging packets.