diff --git a/quic/QuicConstants.h b/quic/QuicConstants.h index 6e6a56381..0c9a16c31 100644 --- a/quic/QuicConstants.h +++ b/quic/QuicConstants.h @@ -589,7 +589,7 @@ constexpr std::chrono::seconds kTimeToRetainLastCongestionAndRttState = 60s; constexpr uint16_t kMaxNumMigrationsAllowed = 6; -constexpr auto kExpectedNumOfParamsInTheTicket = 8; +constexpr auto kMinimumNumOfParamsInTheTicket = 8; constexpr auto kStatelessResetTokenSecretLength = 32; diff --git a/quic/server/handshake/DefaultAppTokenValidator.cpp b/quic/server/handshake/DefaultAppTokenValidator.cpp index 617a39994..8a27ff3c0 100644 --- a/quic/server/handshake/DefaultAppTokenValidator.cpp +++ b/quic/server/handshake/DefaultAppTokenValidator.cpp @@ -58,14 +58,12 @@ bool DefaultAppTokenValidator::validate( auto& params = appToken->transportParams.parameters; - // TODO T33454954 Simplify ticket transport params. see comments in D9324131 - // Currenly only initialMaxData, initialMaxStreamData, ackDelayExponent, and - // maxRecvPacketSize are written into the ticket. In case new parameters - // are added for making early data decision (although not likely), this - // validator fails the check if number of parameters is not - // kExpectedNumOfParamsInTheTicket. - if (params.size() != kExpectedNumOfParamsInTheTicket) { - VLOG(10) << "Unexpected number of parameters in the ticket"; + // Reject tickets that do not have the minimum number of params in the ticket. + // This is a minimum to allow sending additional optional params + // that can be ignored by servers that don't support them. + if (params.size() < kMinimumNumOfParamsInTheTicket) { + VLOG(10) + << "Number of parameters in the ticket is less than the minimum expected"; return validated = false; } diff --git a/quic/server/handshake/test/AppTokenTest.cpp b/quic/server/handshake/test/AppTokenTest.cpp index 313eb0142..0a717edc8 100644 --- a/quic/server/handshake/test/AppTokenTest.cpp +++ b/quic/server/handshake/test/AppTokenTest.cpp @@ -29,9 +29,9 @@ void expectAppTokenEqual( decodedAppToken->transportParams.parameters.size(), appToken.transportParams.parameters.size()); - EXPECT_EQ( + EXPECT_GE( decodedAppToken->transportParams.parameters.size(), - kExpectedNumOfParamsInTheTicket); + kMinimumNumOfParamsInTheTicket); // TODO Split out into individual flow control parameters. auto maxStreamData = getIntegerParameter( TransportParameterId::initial_max_stream_data_bidi_local, diff --git a/quic/server/handshake/test/DefaultAppTokenValidatorTest.cpp b/quic/server/handshake/test/DefaultAppTokenValidatorTest.cpp index 1286b1662..d71da32e5 100644 --- a/quic/server/handshake/test/DefaultAppTokenValidatorTest.cpp +++ b/quic/server/handshake/test/DefaultAppTokenValidatorTest.cpp @@ -57,6 +57,39 @@ TEST(DefaultAppTokenValidatorTest, TestValidParams) { EXPECT_TRUE(validator.validate(resState)); } +TEST(DefaultAppTokenValidatorTest, TestValidOptionalParameter) { + QuicServerConnectionState conn( + FizzServerQuicHandshakeContext::Builder().build()); + conn.peerAddress = folly::SocketAddress("1.2.3.4", 443); + conn.version = QuicVersion::MVFST; + conn.transportSettings.zeroRttSourceTokenMatchingPolicy = + ZeroRttSourceTokenMatchingPolicy::LIMIT_IF_NO_EXACT_MATCH; + auto quicStats = std::make_shared(); + conn.statsCallback = quicStats.get(); + + AppToken appToken; + appToken.transportParams = createTicketTransportParameters( + conn.transportSettings.idleTimeout.count(), + conn.transportSettings.maxRecvPacketSize, + conn.transportSettings.advertisedInitialConnectionWindowSize, + conn.transportSettings.advertisedInitialBidiLocalStreamWindowSize, + conn.transportSettings.advertisedInitialBidiRemoteStreamWindowSize, + conn.transportSettings.advertisedInitialUniStreamWindowSize, + conn.transportSettings.advertisedInitialMaxStreamsBidi, + conn.transportSettings.advertisedInitialMaxStreamsUni); + appToken.transportParams.parameters.push_back( + encodeIntegerParameter(TransportParameterId::disable_migration, 1)); + ResumptionState resState; + resState.appToken = encodeAppToken(appToken); + + conn.earlyDataAppParamsValidator = [](const folly::Optional&, + const Buf&) { return true; }; + DefaultAppTokenValidator validator(&conn); + EXPECT_CALL(*quicStats, onZeroRttAccepted()).Times(1); + EXPECT_CALL(*quicStats, onZeroRttRejected()).Times(0); + EXPECT_TRUE(validator.validate(resState)); +} + TEST( DefaultAppTokenValidatorTest, TestValidUnequalParamsUpdateTransportSettings) {