diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index ceec1bd80..f53a6f037 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -163,7 +163,6 @@ class TestQuicTransport while (!cursor.isAtEnd()) { // create server chosen connId with processId = 0 and workerId = 0 ServerConnectionIdParams params(0, 0, 0); - params.clientConnId = *conn_->clientConnectionId; conn_->serverConnectionId = connIdAlgo_->encodeConnectionId(params); auto type = static_cast(cursor.readBE()); if (type == TestFrameType::CRYPTO) { @@ -312,7 +311,6 @@ class TestQuicTransport void setServerConnectionId() { // create server chosen connId with processId = 0 and workerId = 0 ServerConnectionIdParams params(0, 0, 0); - params.clientConnId = *conn_->clientConnectionId; conn_->serverConnectionId = connIdAlgo_->encodeConnectionId(params); } diff --git a/quic/client/test/QuicClientTransportTest.cpp b/quic/client/test/QuicClientTransportTest.cpp index 839733b96..a157328e6 100644 --- a/quic/client/test/QuicClientTransportTest.cpp +++ b/quic/client/test/QuicClientTransportTest.cpp @@ -1298,7 +1298,6 @@ class QuicClientTransportTest : public Test { void setConnectionIds() { originalConnId = client->getConn().clientConnectionId; ServerConnectionIdParams params(0, 0, 0); - params.clientConnId = *client->getConn().clientConnectionId; serverChosenConnId = connIdAlgo_->encodeConnectionId(params); } @@ -2471,7 +2470,6 @@ class QuicClientTransportVersionAndRetryTest originalConnId = client->getConn().clientConnectionId; // create server chosen connId with processId = 0 and workerId = 0 ServerConnectionIdParams params(0, 0, 0); - params.clientConnId = *client->getConn().clientConnectionId; serverChosenConnId = connIdAlgo_->encodeConnectionId(params); // The tests that we do here create streams before crypto is finished, // so we initialize the peer streams, to allow for this behavior. TODO: when diff --git a/quic/codec/DefaultConnectionIdAlgo.cpp b/quic/codec/DefaultConnectionIdAlgo.cpp index 498f48a36..8fb48a205 100644 --- a/quic/codec/DefaultConnectionIdAlgo.cpp +++ b/quic/codec/DefaultConnectionIdAlgo.cpp @@ -180,7 +180,6 @@ ServerConnectionIdParams DefaultConnectionIdAlgo::parseConnectionId( getHostIdBitsInConnId(id), getProcessIdBitsFromConnId(id), getWorkerIdFromConnId(id)); - serverConnIdParams.clientConnId.assign(id); return serverConnIdParams; } diff --git a/quic/codec/QuicConnectionId.h b/quic/codec/QuicConnectionId.h index 9bc975efc..3255e2cfc 100644 --- a/quic/codec/QuicConnectionId.h +++ b/quic/codec/QuicConnectionId.h @@ -110,7 +110,6 @@ struct ServerConnectionIdParams { */ void setWorkerId(uint8_t workerIdIn); - folly::Optional clientConnId; // Quic connection-id short version uint8_t version{0}; // Quic Host id diff --git a/quic/codec/test/TypesTest.cpp b/quic/codec/test/TypesTest.cpp index d44bc53be..ad7eebc87 100644 --- a/quic/codec/test/TypesTest.cpp +++ b/quic/codec/test/TypesTest.cpp @@ -199,13 +199,11 @@ TEST_F(TypesTest, KeyPhase) { TEST_F(TypesTest, TestConnIdWorkerId) { std::vector connIdData(kDefaultConnectionIdSize); folly::Random::secureRandom(connIdData.data(), connIdData.size()); - ConnectionId randConnId(connIdData); auto connIdAlgo = std::make_unique(); for (uint8_t i = 0; i <= 254; i++) { uint8_t processId = i % 2; uint16_t hostId = folly::Random::rand32() % 4095; ServerConnectionIdParams params(hostId, processId, i); - params.clientConnId = randConnId; auto paramsAfterEncode = connIdAlgo->parseConnectionId(connIdAlgo->encodeConnectionId(params)); EXPECT_TRUE(connIdAlgo->canParse(connIdAlgo->encodeConnectionId(params))); diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index 24482d9bc..a92147297 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -80,7 +80,6 @@ class QuicLossFunctionsTest : public TestWithParam { // create a serverConnectionId that is different from the client connId // with bits for processId and workerId set to 0 ServerConnectionIdParams params(0, 0, 0); - params.clientConnId = *conn->clientConnectionId; conn->connIdAlgo = connIdAlgo_.get(); conn->serverConnectionId = connIdAlgo_->encodeConnectionId(params); return conn; @@ -105,7 +104,6 @@ class QuicLossFunctionsTest : public TestWithParam { // create a serverConnectionId that is different from the client connId // with bits for processId and workerId set to 0 ServerConnectionIdParams params(0, 0, 0); - params.clientConnId = *conn->clientConnectionId; conn->connIdAlgo = connIdAlgo_.get(); conn->serverConnectionId = connIdAlgo_->encodeConnectionId(params); return conn; diff --git a/quic/server/state/ServerStateMachine.cpp b/quic/server/state/ServerStateMachine.cpp index 634460c27..d6ee6c731 100644 --- a/quic/server/state/ServerStateMachine.cpp +++ b/quic/server/state/ServerStateMachine.cpp @@ -501,6 +501,7 @@ void onServerReadDataFromOpen( FizzCryptoFactory(&fizzFactory) .getClientInitialCipher(initialDestinationConnectionId, version)); conn.readCodec->setClientConnectionId(clientConnectionId); + conn.readCodec->setServerConnectionId(*conn.serverConnectionId); if (conn.qLogger) { conn.qLogger->scid = conn.serverConnectionId; conn.qLogger->dcid = clientConnectionId; @@ -686,31 +687,11 @@ void onServerReadDataFromOpen( } } - // TODO: remove this when we actually negotiate connid and version - if (!conn.clientConnectionId) { - conn.clientConnectionId = folly::variant_match( - regularPacket.header, - [](const LongHeader& longHeader) { - return longHeader.getSourceConnId(); - }, - [](const ShortHeader& shortHeader) { - return shortHeader.getConnectionId(); - }); - // change the connection id when we switch - CHECK(conn.clientConnectionId); - // TODO: if conn.serverConnIdParams->clientConnId != conn.clientConnId, - // we need to update sourceAddressMap_. - // TODO: need to remove ServerConnectionIdParams::clientConnId, it is no - // longer needed. - conn.serverConnIdParams->clientConnId = *conn.clientConnectionId; - conn.readCodec->setServerConnectionId(*conn.serverConnectionId); - if (conn.qLogger) { - conn.qLogger->dcid = conn.clientConnectionId; - conn.qLogger->scid = conn.serverConnectionId; - } - } + CHECK(conn.clientConnectionId); if (conn.qLogger) { conn.qLogger->addPacket(regularPacket, packetSize); + conn.qLogger->dcid = conn.clientConnectionId; + conn.qLogger->scid = conn.serverConnectionId; } QUIC_TRACE(packet_recvd, conn, packetNum, packetSize); // We assume that the higher layer takes care of validating that the version diff --git a/quic/state/stream/test/StreamStateMachineTest.cpp b/quic/state/stream/test/StreamStateMachineTest.cpp index 399e03e9d..884b710ae 100644 --- a/quic/state/stream/test/StreamStateMachineTest.cpp +++ b/quic/state/stream/test/StreamStateMachineTest.cpp @@ -430,7 +430,6 @@ TEST_F(QuicHalfClosedRemoteStateTest, AckStream) { auto conn = createConn(); // create server chosen connId with processId = 0 and workerId = 0 ServerConnectionIdParams params(0, 0, 0); - params.clientConnId = *conn->clientConnectionId; auto connIdAlgo = std::make_unique(); folly::Optional serverChosenConnId = connIdAlgo->encodeConnectionId(params); @@ -470,7 +469,6 @@ TEST_F(QuicHalfClosedRemoteStateTest, AckStreamAfterSkip) { auto conn = createConn(); // create server chosen connId with processId = 0 and workerId = 0 ServerConnectionIdParams params(0, 0, 0); - params.clientConnId = *conn->clientConnectionId; auto connIdAlgo = std::make_unique(); folly::Optional serverChosenConnId = connIdAlgo->encodeConnectionId(params);