Merge pull request #2206 from lucas-clemente/ignore-reordered-ncid-frames

retire reordered NEW_CONNECTION_IDs that were already retired

conn_id_manager.go

@@ -15,6 +15,7 @@ type connIDManager struct {
 	queue utils.NewConnectionIDList
 
 	activeSequenceNumber      uint64
+	retiredPriorTo            uint64
 	activeConnectionID        protocol.ConnectionID
 	activeStatelessResetToken *[16]byte
 
@@ -62,18 +63,30 @@ func (h *connIDManager) Add(f *wire.NewConnectionIDFrame) error {
 }
 
 func (h *connIDManager) add(f *wire.NewConnectionIDFrame) error {
+	// If the NEW_CONNECTION_ID frame is reordered, such that its sequenece number
+	// was already retired, send the RETIRE_CONNECTION_ID frame immediately.
+	if f.SequenceNumber < h.retiredPriorTo {
+		h.queueControlFrame(&wire.RetireConnectionIDFrame{
+			SequenceNumber: f.SequenceNumber,
+		})
+		return nil
+	}
+
 	// Retire elements in the queue.
 	// Doesn't retire the active connection ID.
-	var next *utils.NewConnectionIDElement
+	if f.RetirePriorTo > h.retiredPriorTo {
-	for el := h.queue.Front(); el != nil; el = next {
+		var next *utils.NewConnectionIDElement
-		if el.Value.SequenceNumber >= f.RetirePriorTo {
+		for el := h.queue.Front(); el != nil; el = next {
-			break
+			if el.Value.SequenceNumber >= f.RetirePriorTo {
+				break
+			}
+			next = el.Next()
+			h.queueControlFrame(&wire.RetireConnectionIDFrame{
+				SequenceNumber: el.Value.SequenceNumber,
+			})
+			h.queue.Remove(el)
 		}
-		next = el.Next()
+		h.retiredPriorTo = f.RetirePriorTo
-		h.queueControlFrame(&wire.RetireConnectionIDFrame{
-			SequenceNumber: el.Value.SequenceNumber,
-		})
-		h.queue.Remove(el)
 	}
 
 	// insert a new element at the end

conn_id_manager_test.go

@@ -141,6 +141,25 @@ var _ = Describe("Connection ID Manager", func() {
 		Expect(m.Get()).To(Equal(protocol.ConnectionID{3, 4, 5, 6}))
 	})
 
+	It("ignores reordered connection IDs, if their sequence number was already retired", func() {
+		Expect(m.Add(&wire.NewConnectionIDFrame{
+			SequenceNumber: 10,
+			ConnectionID:   protocol.ConnectionID{1, 2, 3, 4},
+			RetirePriorTo:  5,
+		})).To(Succeed())
+		Expect(frameQueue).To(HaveLen(1))
+		Expect(frameQueue[0].(*wire.RetireConnectionIDFrame).SequenceNumber).To(BeZero())
+		frameQueue = nil
+		// If this NEW_CONNECTION_ID frame hadn't been reordered, we would have retired it before.
+		// Make sure it gets retired immediately now.
+		Expect(m.Add(&wire.NewConnectionIDFrame{
+			SequenceNumber: 4,
+			ConnectionID:   protocol.ConnectionID{4, 3, 2, 1},
+		})).To(Succeed())
+		Expect(frameQueue).To(HaveLen(1))
+		Expect(frameQueue[0].(*wire.RetireConnectionIDFrame).SequenceNumber).To(BeEquivalentTo(4))
+	})
+
 	It("retires old connection IDs when the peer sends too many new ones", func() {
 		for i := uint8(1); i <= protocol.MaxActiveConnectionIDs; i++ {
 			Expect(m.Add(&wire.NewConnectionIDFrame{