From 3f6030fdb33b0f113f4c237c0720d6f2000d1b32 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 3 Dec 2019 18:25:15 +0700 Subject: [PATCH] count the connection ID used during the handshake towards the limit --- conn_id_generator.go | 8 ++++++-- conn_id_generator_test.go | 20 ++++++++++---------- session_test.go | 4 ++-- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/conn_id_generator.go b/conn_id_generator.go index 61b8ed8a6..071689552 100644 --- a/conn_id_generator.go +++ b/conn_id_generator.go @@ -51,8 +51,12 @@ func (m *connIDGenerator) SetMaxActiveConnIDs(limit uint64) error { return nil } // The active_connection_id_limit transport parameter is the number of - // connection IDs issued in NEW_CONNECTION_IDs frame that the peer will store. - for i := uint64(0); i < utils.MinUint64(limit, protocol.MaxIssuedConnectionIDs); i++ { + // connection IDs the peer will store. This limit includes the connection ID + // used during the handshake, and the one sent in the preferred_address + // transport parameter. + // We currently don't send the preferred_address transport parameter, + // so we can issue (limit - 1) connection IDs. + for i := uint64(1); i < utils.MinUint64(limit, protocol.MaxIssuedConnectionIDs); i++ { if err := m.issueNewConnID(); err != nil { return err } diff --git a/conn_id_generator_test.go b/conn_id_generator_test.go index 1532139eb..6ba7430c2 100644 --- a/conn_id_generator_test.go +++ b/conn_id_generator_test.go @@ -44,12 +44,12 @@ var _ = Describe("Connection ID Generator", func() { It("issues new connection IDs", func() { Expect(g.SetMaxActiveConnIDs(4)).To(Succeed()) Expect(retiredConnIDs).To(BeEmpty()) - Expect(addedConnIDs).To(HaveLen(4)) + Expect(addedConnIDs).To(HaveLen(3)) for i := 0; i < len(addedConnIDs)-1; i++ { Expect(addedConnIDs[i]).ToNot(Equal(addedConnIDs[i+1])) } - Expect(queuedFrames).To(HaveLen(4)) - for i := 0; i < 4; i++ { + Expect(queuedFrames).To(HaveLen(3)) + for i := 0; i < 3; i++ { f := queuedFrames[i] Expect(f).To(BeAssignableToTypeOf(&wire.NewConnectionIDFrame{})) nf := f.(*wire.NewConnectionIDFrame) @@ -63,8 +63,8 @@ var _ = Describe("Connection ID Generator", func() { It("limits the number of connection IDs that it issues", func() { Expect(g.SetMaxActiveConnIDs(9999999)).To(Succeed()) Expect(retiredConnIDs).To(BeEmpty()) - Expect(addedConnIDs).To(HaveLen(protocol.MaxIssuedConnectionIDs)) - Expect(queuedFrames).To(HaveLen(protocol.MaxIssuedConnectionIDs)) + Expect(addedConnIDs).To(HaveLen(protocol.MaxIssuedConnectionIDs - 1)) + Expect(queuedFrames).To(HaveLen(protocol.MaxIssuedConnectionIDs - 1)) }) It("errors if the peers tries to retire a connection ID that wasn't yet issued", func() { @@ -79,7 +79,7 @@ var _ = Describe("Connection ID Generator", func() { Expect(queuedFrames).To(HaveLen(1)) Expect(queuedFrames[0]).To(BeAssignableToTypeOf(&wire.NewConnectionIDFrame{})) nf := queuedFrames[0].(*wire.NewConnectionIDFrame) - Expect(nf.SequenceNumber).To(BeEquivalentTo(6)) + Expect(nf.SequenceNumber).To(BeEquivalentTo(5)) Expect(nf.ConnectionID.Len()).To(Equal(7)) }) @@ -111,9 +111,9 @@ var _ = Describe("Connection ID Generator", func() { It("removes all connection IDs", func() { Expect(g.SetMaxActiveConnIDs(5)).To(Succeed()) - Expect(queuedFrames).To(HaveLen(5)) + Expect(queuedFrames).To(HaveLen(4)) g.RemoveAll() - Expect(removedConnIDs).To(HaveLen(7)) // initial conn ID, initial client dest conn id, and newly issued ones + Expect(removedConnIDs).To(HaveLen(6)) // initial conn ID, initial client dest conn id, and newly issued ones Expect(removedConnIDs).To(ContainElement(initialConnID)) Expect(removedConnIDs).To(ContainElement(initialClientDestConnID)) for _, f := range queuedFrames { @@ -124,10 +124,10 @@ var _ = Describe("Connection ID Generator", func() { It("replaces with a closed session for all connection IDs", func() { Expect(g.SetMaxActiveConnIDs(5)).To(Succeed()) - Expect(queuedFrames).To(HaveLen(5)) + Expect(queuedFrames).To(HaveLen(4)) sess := NewMockPacketHandler(mockCtrl) g.ReplaceWithClosed(sess) - Expect(replacedWithClosed).To(HaveLen(7)) // initial conn ID, initial client dest conn id, and newly issued ones + Expect(replacedWithClosed).To(HaveLen(6)) // initial conn ID, initial client dest conn id, and newly issued ones Expect(replacedWithClosed).To(HaveKeyWithValue(string(initialClientDestConnID), sess)) Expect(replacedWithClosed).To(HaveKeyWithValue(string(initialConnID), sess)) for _, f := range queuedFrames { diff --git a/session_test.go b/session_test.go index b277b7326..53df75d43 100644 --- a/session_test.go +++ b/session_test.go @@ -1335,7 +1335,7 @@ var _ = Describe("Session", func() { packer.EXPECT().HandleTransportParameters(params) packer.EXPECT().PackPacket().MaxTimes(3) Expect(sess.earlySessionReady()).ToNot(BeClosed()) - sessionRunner.EXPECT().Add(gomock.Any(), sess).Times(3) + sessionRunner.EXPECT().Add(gomock.Any(), sess).Times(2) sess.processTransportParameters(params.Marshal()) Expect(sess.earlySessionReady()).To(BeClosed()) @@ -1345,7 +1345,7 @@ var _ = Describe("Session", func() { sessionRunner.EXPECT().ReplaceWithClosed(gomock.Any(), gomock.Any()).Do(func(_ protocol.ConnectionID, s packetHandler) { Expect(s).To(BeAssignableToTypeOf(&closedLocalSession{})) Expect(s.Close()).To(Succeed()) - }).Times(5) // initial connection ID + initial client dest conn ID + 3 newly issued conn IDs + }).Times(4) // initial connection ID + initial client dest conn ID + 2 newly issued conn IDs packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() sess.Close()