Fix the default active connection id limit when the peer doesn't send the transport parameter

Summary:
When the client doesn't send the active_connection_id_limit transport parameter, the server was wrongly defaulting to 5 instead of 2.
(https://datatracker.ietf.org/doc/html/rfc9000#section-18.2-6.2.1)

This was uncovered by https://github.com/curl/curl/issues/14043.

In that case, ngtcp2 does not send the transport parameter when its value is set to 2.

Reviewed By: hanidamlaj, meleshuk

Differential Revision: D59165736

fbshipit-source-id: e44507550b2d0eb4c97f1f7f9879bfdca4746526

quic/QuicConstants.h

@@ -601,9 +601,14 @@ constexpr uint64_t kMaxRetryTokenValidMs = 1000 * 60 * 5;
 constexpr uint64_t kMaxNewTokenValidMs = 1000 * 24 * 60 * 60;
 
 // Default limit on active connection ids that a peer generates.
-constexpr uint64_t kDefaultActiveConnectionIdLimit = 5;
+// If the client doesn't send an active_connection_id_limit transport parameter,
+// this value will be used.
+// The value is defined in
+// https://datatracker.ietf.org/doc/html/rfc9000#section-18.2-6.2.1
+constexpr uint64_t kDefaultActiveConnectionIdLimit = 2;
 
 // Maximum number of active connection ids to generate for the peer.
+// This is also the default value Mvfst will advertise to the peer.
 constexpr uint64_t kMaxActiveConnectionIdLimit = 5;
 
 constexpr uint64_t kMaxPacketNumber = (1ull << 62) - 1;

quic/server/test/ServerStateMachineTest.cpp

@@ -249,7 +249,7 @@ TEST(ServerStateMachineTest, TestProcessMinAckDelayNotSet) {
   ClientTransportParameters clientTransportParams = {
       std::move(transportParams)};
   processClientInitialParams(serverConn, clientTransportParams);
-  EXPECT_FALSE(serverConn.peerAdvertisedKnobFrameSupport);
+  EXPECT_FALSE(serverConn.peerMinAckDelay.has_value());
 }
 
 TEST(ServerStateMachineTest, TestProcessMinAckDelaySet) {
@@ -336,6 +336,35 @@ TEST(ServerStateMachineTest, TestEncodeKnobFrameSupportedParamDisabled) {
           testing::Eq(TransportParameterId::knob_frames_supported)))));
 }
 
+TEST(ServerStateMachineTest, TestProcessActiveConnectionIdLimitNotSet) {
+  QuicServerConnectionState serverConn(
+      FizzServerQuicHandshakeContext::Builder().build());
+  std::vector<TransportParameter> transportParams;
+  ClientTransportParameters clientTransportParams = {
+      std::move(transportParams)};
+  processClientInitialParams(serverConn, clientTransportParams);
+  // Check that the value is the default specified in RFC9000 when the transport
+  // parameter is absent
+  // https://datatracker.ietf.org/doc/html/rfc9000#section-18.2-6.2.1
+  EXPECT_EQ(serverConn.peerActiveConnectionIdLimit, 2);
+}
+
+TEST(ServerStateMachineTest, TestProcessActiveConnectionIdLimitSet) {
+  QuicServerConnectionState serverConn(
+      FizzServerQuicHandshakeContext::Builder().build());
+  std::vector<TransportParameter> transportParams;
+  transportParams.push_back(encodeIntegerParameter(
+      TransportParameterId::active_connection_id_limit,
+      kMaxActiveConnectionIdLimit + 1));
+  ClientTransportParameters clientTransportParams = {
+      std::move(transportParams)};
+  processClientInitialParams(serverConn, clientTransportParams);
+  // This tests for max + 1. The maximum value will be enforced on sending the
+  // new connection id frames, not in parsing the parameters.
+  EXPECT_EQ(
+      serverConn.peerActiveConnectionIdLimit, kMaxActiveConnectionIdLimit + 1);
+}
+
 struct advertisedMaxStreamGroupstestStruct {
   uint64_t peerMaxGroupsIn;
   OptionalIntegral<uint64_t> expectedTransportSettingVal;

quic/state/TransportSettings.h

@@ -243,7 +243,7 @@ struct TransportSettings {
   // Default initial RTT
   std::chrono::microseconds initialRtt{kDefaultInitialRtt};
   // The active_connection_id_limit that is sent to the peer.
-  uint64_t selfActiveConnectionIdLimit{kDefaultActiveConnectionIdLimit};
+  uint64_t selfActiveConnectionIdLimit{kMaxActiveConnectionIdLimit};
   // Maximum size of the batch that should be used when receiving packets from
   // the kernel in one event loop.
   uint16_t maxRecvBatchSize{5};