Issue #62 fix. Sync the first client CID length with the DeterministicConnectionIdGenerator::expected_connection_id_length_

[aDanPin]

Problem

MockableQuicClient::UseClientConnectionIdLength() updated override_client_connection_id_length_, which was used to create the initial CID in MockableQuicClient::GetClientConnectionId(), but it did not touch connection_id_generator_, which is constructed with expected_connection_id_length_ == kQuicDefaultConnectionIdLength.

Setting the initial CID also sets the framer's expected client CID length for whole connection in QuicConnection::set_client_connection_id.

As a result, the first CID had the length configured through MockableQuicClient::UseClientConnectionIdLength(), while every subsequent CID had DeterministicConnectionIdGenerator::expected_connection_id_length_, which is different to QuicFramer::expected_client_connection_id_length_.

Any packet carrying a CID of a different length than the one established by QuicConnection::set_client_connection_id is dropped rather than processed.

Solution

RFC 9000, Section 10.3.2 states that
An endpoint that uses this design MUST either use the same connection ID length for all connections or encode the length of the connection ID such that it can be recovered without state. In addition, it cannot provide a zero-length connection ID.

In the current QUIC implementation, the CID length is set once in QuicClientBase::StartConnect and stays constant for the entire lifetime of the connection.

Since DeterministicConnectionIdGenerator has the same lifetime as QuicClientBase, expected_connection_id_length_ is no longer const and a setter for it was added. QuicClientBase::set_client_connection_id_length() now keeps the generator in sync.

The CID length must be changed before the first handshake to avoid CIDs of different lengths within one connection, so QuicClientBase::set_client_connection_id_length() carries a QUICHE_DCHECK(!connected()).

The following overrides were also removed from quic_test_client.h:

  // Client connection ID to use, if client_connection_id_overridden_.
  // TODO(wub): Move client_connection_id_(length_) overrides to QuicClientBase.
  QuicConnectionId override_client_connection_id_;
  bool client_connection_id_overridden_;
  int override_client_connection_id_length_ = -1;

as this commit implements exactly that logic in QuicClientBase.

Testing

The multi-migration end-to-end test is now parameterized by the client connection ID length and additionally runs with kQuicMaxConnectionIdWithLengthPrefixLength, which fails without this fix.

bazel-bin/quiche/end_to_end_test --gtest_filter='*ConnectionMigrationWithMaxConnectionIDClientIPChangedMultipleTimes*:*ConnectionMigrationWithNonZeroConnectionIDClientIPChangedMultipleTimes*'

Issue #62