From ca2471e78d77c2af0dee33c65d8a464340503d8f Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Tue, 6 Mar 2018 18:20:37 +0700
Subject: [PATCH] remove explicit error for duplicate or out of order ACKs

---
 internal/ackhandler/sent_packet_handler.go    |  9 ++----
 .../ackhandler/sent_packet_handler_test.go    | 32 +++++++++----------
 session.go                                    |  7 +---
 3 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go
index b5f9b6f6..53399442 100644
--- a/internal/ackhandler/sent_packet_handler.go
+++ b/internal/ackhandler/sent_packet_handler.go
@@ -1,7 +1,6 @@
 package ackhandler
 
 import (
-	"errors"
 	"fmt"
 	"math"
 	"time"
@@ -30,9 +29,6 @@ const (
 	maxRTOTimeout = 60 * time.Second
 )
 
-// ErrDuplicateOrOutOfOrderAck occurs when a duplicate or an out-of-order ACK is received
-var ErrDuplicateOrOutOfOrderAck = errors.New("SentPacketHandler: Duplicate or out-of-order ACK")
-
 type sentPacketHandler struct {
 	lastSentPacketNumber protocol.PacketNumber
 	nextPacketSendTime   time.Time
@@ -156,9 +152,10 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *wire.AckFrame, withPacketNumbe
 		return qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package")
 	}
 
-	// duplicate or out-of-order ACK
+	// duplicate or out of order ACK
 	if withPacketNumber != 0 && withPacketNumber <= h.largestReceivedPacketWithAck {
-		return ErrDuplicateOrOutOfOrderAck
+		utils.Debugf("Ignoring ACK frame (duplicate or out of order).")
+		return nil
 	}
 	h.largestReceivedPacketWithAck = withPacketNumber
 
diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go
index f2113fff..98818c3f 100644
--- a/internal/ackhandler/sent_packet_handler_test.go
+++ b/internal/ackhandler/sent_packet_handler_test.go
@@ -218,29 +218,29 @@ var _ = Describe("SentPacketHandler", func() {
 			})
 
 			It("rejects duplicate ACKs", func() {
-				largestAcked := 3
-				ack := wire.AckFrame{
-					LargestAcked: protocol.PacketNumber(largestAcked),
-					LowestAcked:  1,
-				}
-				err := handler.ReceivedAck(&ack, 1337, protocol.EncryptionUnencrypted, time.Now())
+				ack1 := wire.AckFrame{LargestAcked: 3}
+				ack2 := wire.AckFrame{LargestAcked: 4}
+				err := handler.ReceivedAck(&ack1, 1337, protocol.EncryptionUnencrypted, time.Now())
 				Expect(err).ToNot(HaveOccurred())
-				Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3)))
-				err = handler.ReceivedAck(&ack, 1337, protocol.EncryptionUnencrypted, time.Now())
-				Expect(err).To(MatchError(ErrDuplicateOrOutOfOrderAck))
-				Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3)))
+				Expect(handler.largestAcked).To(Equal(protocol.PacketNumber(3)))
+				// this wouldn't happen in practice
+				// for testing purposes, we pretend send a different ACK frame in a duplicated packet, to be able to verify that it actually doesn't get processed
+				err = handler.ReceivedAck(&ack2, 1337, protocol.EncryptionUnencrypted, time.Now())
+				Expect(err).ToNot(HaveOccurred())
+				Expect(handler.largestAcked).To(Equal(protocol.PacketNumber(3)))
 			})
 
 			It("rejects out of order ACKs", func() {
 				// acks packets 0, 1, 2, 3
-				ack := wire.AckFrame{LargestAcked: 3}
-				err := handler.ReceivedAck(&ack, 1337, protocol.EncryptionUnencrypted, time.Now())
+				ack1 := wire.AckFrame{LargestAcked: 3}
+				ack2 := wire.AckFrame{LargestAcked: 4}
+				err := handler.ReceivedAck(&ack1, 1337, protocol.EncryptionUnencrypted, time.Now())
+				Expect(err).ToNot(HaveOccurred())
+				// this wouldn't happen in practive
+				// a receiver wouldn't send an ACK for a lower largest acked in a packet sent later
+				err = handler.ReceivedAck(&ack2, 1337-1, protocol.EncryptionUnencrypted, time.Now())
 				Expect(err).ToNot(HaveOccurred())
-				Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 4)))
-				err = handler.ReceivedAck(&ack, 1337-1, protocol.EncryptionUnencrypted, time.Now())
-				Expect(err).To(MatchError(ErrDuplicateOrOutOfOrderAck))
 				Expect(handler.largestAcked).To(Equal(protocol.PacketNumber(3)))
-				Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 4)))
 			})
 
 			It("rejects ACKs with a too high LargestAcked packet number", func() {
diff --git a/session.go b/session.go
index 0cd76182..5c966830 100644
--- a/session.go
+++ b/session.go
@@ -595,12 +595,7 @@ func (s *session) handleFrames(fs []wire.Frame, encLevel protocol.EncryptionLeve
 		}
 
 		if err != nil {
-			switch err {
-			case ackhandler.ErrDuplicateOrOutOfOrderAck:
-				// Can happen e.g. when packets thought missing arrive late
-			default:
-				return err
-			}
+			return err
 		}
 	}
 	return nil