Add version guard to QuicUtil methods which are IETF QUIC only. This includes some minor method parameter changes along the way. No behavior change. not protected. PiperOrigin-RevId: 317388565 Change-Id: Ia73f1028212a318d4640afa2fbdd0239e095e1f2
diff --git a/quic/core/http/quic_spdy_client_session.cc b/quic/core/http/quic_spdy_client_session.cc index 7440edc..78b6a56 100644 --- a/quic/core/http/quic_spdy_client_session.cc +++ b/quic/core/http/quic_spdy_client_session.cc
@@ -152,7 +152,7 @@ } if (VersionHasIetfQuicFrames(transport_version()) && - QuicUtils::IsBidirectionalStreamId(id)) { + QuicUtils::IsBidirectionalStreamId(id, version())) { connection()->CloseConnection( QUIC_HTTP_SERVER_INITIATED_BIDIRECTIONAL_STREAM, "Server created bidirectional stream.",
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc index 6408667..d5481b5 100644 --- a/quic/core/http/quic_spdy_client_session_test.cc +++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -98,6 +98,7 @@ client_session_cache_ = client_cache.get(); SetQuicReloadableFlag(quic_enable_tls_resumption, true); SetQuicReloadableFlag(quic_enable_zero_rtt_for_tls, true); + SetQuicReloadableFlag(quic_fix_gquic_stream_type, true); client_crypto_config_ = std::make_unique<QuicCryptoClientConfig>( crypto_test_utils::ProofVerifierForTesting(), std::move(client_cache)); server_crypto_config_ = crypto_test_utils::CryptoServerConfigForTesting(); @@ -1179,8 +1180,6 @@ return; } - SetQuicReloadableFlag(quic_fix_gquic_stream_type, true); - CompleteFirstConnection(); // Create a second connection, but disable 0-RTT on the server.
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index e69a051..08f82c8 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -558,7 +558,7 @@ bool QuicSpdySession::OnPriorityUpdateForRequestStream(QuicStreamId stream_id, int urgency) { if (perspective() == Perspective::IS_CLIENT || - !QuicUtils::IsBidirectionalStreamId(stream_id) || + !QuicUtils::IsBidirectionalStreamId(stream_id, version()) || !QuicUtils::IsClientInitiatedStreamId(transport_version(), stream_id)) { return true; } @@ -657,7 +657,7 @@ void QuicSpdySession::OnHttp3GoAway(QuicStreamId stream_id) { DCHECK_EQ(perspective(), Perspective::IS_CLIENT); - if (!QuicUtils::IsBidirectionalStreamId(stream_id) || + if (!QuicUtils::IsBidirectionalStreamId(stream_id, version()) || IsIncomingStream(stream_id)) { CloseConnectionWithDetails( QUIC_INVALID_STREAM_ID,
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index b391c06..b2bcaa3 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -251,9 +251,8 @@ } else { TestStream* stream = new TestStream( id, this, - DetermineStreamType(id, connection()->transport_version(), - perspective(), /*is_incoming=*/true, - BIDIRECTIONAL)); + DetermineStreamType(id, connection()->version(), perspective(), + /*is_incoming=*/true, BIDIRECTIONAL)); ActivateStream(QuicWrapUnique(stream)); return stream; } @@ -261,11 +260,10 @@ TestStream* CreateIncomingStream(PendingStream* pending) override { QuicStreamId id = pending->id(); - TestStream* stream = - new TestStream(pending, this, - DetermineStreamType( - id, connection()->transport_version(), perspective(), - /*is_incoming=*/true, BIDIRECTIONAL)); + TestStream* stream = new TestStream( + pending, this, + DetermineStreamType(id, connection()->version(), perspective(), + /*is_incoming=*/true, BIDIRECTIONAL)); ActivateStream(QuicWrapUnique(stream)); return stream; }
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index fec3fe5..59e6cc8 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -184,8 +184,8 @@ if (UsesPendingStreams() && QuicUtils::GetStreamType(stream_id, perspective(), - IsIncomingStream(stream_id)) == - READ_UNIDIRECTIONAL && + IsIncomingStream(stream_id), + version()) == READ_UNIDIRECTIONAL && stream_map_.find(stream_id) == stream_map_.end()) { PendingStreamOnStreamFrame(frame); return; @@ -232,8 +232,8 @@ // If stream_id is READ_UNIDIRECTIONAL, close the connection. if (QuicUtils::GetStreamType(stream_id, perspective(), - IsIncomingStream(stream_id)) == - READ_UNIDIRECTIONAL) { + IsIncomingStream(stream_id), + version()) == READ_UNIDIRECTIONAL) { QUIC_DVLOG(1) << ENDPOINT << "Received STOP_SENDING for a read-only stream_id: " << stream_id << "."; @@ -298,7 +298,7 @@ // Pending stream is currently read only. We can safely close the stream. DCHECK_EQ(READ_UNIDIRECTIONAL, QuicUtils::GetStreamType(pending->id(), perspective(), - /*peer_initiated = */ true)); + /*peer_initiated = */ true, version())); ClosePendingStream(stream_id); } @@ -313,8 +313,8 @@ if (VersionHasIetfQuicFrames(transport_version()) && QuicUtils::GetStreamType(stream_id, perspective(), - IsIncomingStream(stream_id)) == - WRITE_UNIDIRECTIONAL) { + IsIncomingStream(stream_id), + version()) == WRITE_UNIDIRECTIONAL) { connection()->CloseConnection( QUIC_INVALID_STREAM_ID, "Received RESET_STREAM for a write-only stream", ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); @@ -327,8 +327,8 @@ if (UsesPendingStreams() && QuicUtils::GetStreamType(stream_id, perspective(), - IsIncomingStream(stream_id)) == - READ_UNIDIRECTIONAL && + IsIncomingStream(stream_id), + version()) == READ_UNIDIRECTIONAL && stream_map_.find(stream_id) == stream_map_.end()) { PendingStreamOnRstStream(frame); return; @@ -467,8 +467,8 @@ if (VersionHasIetfQuicFrames(transport_version()) && QuicUtils::GetStreamType(stream_id, perspective(), - IsIncomingStream(stream_id)) == - READ_UNIDIRECTIONAL) { + IsIncomingStream(stream_id), + version()) == READ_UNIDIRECTIONAL) { connection()->CloseConnection( QUIC_WINDOW_UPDATE_RECEIVED_ON_READ_UNIDIRECTIONAL_STREAM, "WindowUpdateFrame received on READ_UNIDIRECTIONAL stream.", @@ -782,8 +782,8 @@ QuicStreamOffset bytes_written) { DCHECK(connection()->connected()); if (!VersionHasIetfQuicFrames(transport_version()) || - QuicUtils::GetStreamType(id, perspective(), IsIncomingStream(id)) != - READ_UNIDIRECTIONAL) { + QuicUtils::GetStreamType(id, perspective(), IsIncomingStream(id), + version()) != READ_UNIDIRECTIONAL) { control_frame_manager_.WriteOrBufferRstStream(id, error, bytes_written); } } @@ -792,8 +792,8 @@ QuicRstStreamErrorCode error) { DCHECK(connection()->connected()); if (VersionHasIetfQuicFrames(transport_version()) && - QuicUtils::GetStreamType(id, perspective(), IsIncomingStream(id)) != - WRITE_UNIDIRECTIONAL) { + QuicUtils::GetStreamType(id, perspective(), IsIncomingStream(id), + version()) != WRITE_UNIDIRECTIONAL) { control_frame_manager_.WriteOrBufferStopSending(error, id); } } @@ -1332,7 +1332,7 @@ continue; } } else { - if (QuicUtils::IsBidirectionalStreamId(id)) { + if (QuicUtils::IsBidirectionalStreamId(id, version())) { continue; } } @@ -1366,7 +1366,7 @@ continue; } } else { - if (!QuicUtils::IsBidirectionalStreamId(id)) { + if (!QuicUtils::IsBidirectionalStreamId(id, version())) { continue; } } @@ -1400,7 +1400,7 @@ continue; } } else { - if (!QuicUtils::IsBidirectionalStreamId(id)) { + if (!QuicUtils::IsBidirectionalStreamId(id, version())) { continue; } }
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index c64dca3..12a4830 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -243,11 +243,10 @@ return nullptr; } - TestStream* stream = - new TestStream(id, this, - DetermineStreamType( - id, connection()->transport_version(), perspective(), - /*is_incoming=*/true, BIDIRECTIONAL)); + TestStream* stream = new TestStream( + id, this, + DetermineStreamType(id, connection()->version(), perspective(), + /*is_incoming=*/true, BIDIRECTIONAL)); ActivateStream(QuicWrapUnique(stream)); ++num_incoming_streams_created_; return stream; @@ -256,8 +255,7 @@ TestStream* CreateIncomingStream(PendingStream* pending) override { QuicStreamId id = pending->id(); TestStream* stream = new TestStream( - pending, DetermineStreamType(id, connection()->transport_version(), - perspective(), + pending, DetermineStreamType(id, connection()->version(), perspective(), /*is_incoming=*/true, BIDIRECTIONAL)); ActivateStream(QuicWrapUnique(stream)); ++num_incoming_streams_created_; @@ -438,15 +436,15 @@ void CloseStream(QuicStreamId id) { if (VersionHasIetfQuicFrames(transport_version())) { - if (QuicUtils::GetStreamType(id, session_.perspective(), - session_.IsIncomingStream(id)) == - READ_UNIDIRECTIONAL) { + if (QuicUtils::GetStreamType( + id, session_.perspective(), session_.IsIncomingStream(id), + connection_->version()) == READ_UNIDIRECTIONAL) { // Verify reset is not sent for READ_UNIDIRECTIONAL streams. EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(0); - } else if (QuicUtils::GetStreamType(id, session_.perspective(), - session_.IsIncomingStream(id)) == - WRITE_UNIDIRECTIONAL) { + } else if (QuicUtils::GetStreamType( + id, session_.perspective(), session_.IsIncomingStream(id), + connection_->version()) == WRITE_UNIDIRECTIONAL) { // Verify RESET_STREAM but not STOP_SENDING is sent for write-only // stream. EXPECT_CALL(*connection_, SendControlFrame(_))
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc index 9903781..6318ac1 100644 --- a/quic/core/quic_stream.cc +++ b/quic/core/quic_stream.cc
@@ -46,7 +46,7 @@ // Unidirectional streams (v99 only). if (VersionHasIetfQuicFrames(version.transport_version) && - !QuicUtils::IsBidirectionalStreamId(stream_id)) { + !QuicUtils::IsBidirectionalStreamId(stream_id, version)) { return session->config() ->GetInitialMaxStreamDataBytesUnidirectionalToSend(); } @@ -74,7 +74,7 @@ // Unidirectional streams (v99 only). if (VersionHasIetfQuicFrames(version.transport_version) && - !QuicUtils::IsBidirectionalStreamId(stream_id)) { + !QuicUtils::IsBidirectionalStreamId(stream_id, version)) { if (session->config() ->HasReceivedInitialMaxStreamDataBytesUnidirectional()) { return session->config() @@ -359,7 +359,8 @@ type != CRYPTO ? QuicUtils::GetStreamType(id_, session->perspective(), - session->IsIncomingStream(id_)) + session->IsIncomingStream(id_), + session->version()) : type), perspective_(session->perspective()) { if (type_ == WRITE_UNIDIRECTIONAL) {
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc index 5cde890..528d955 100644 --- a/quic/core/quic_stream_id_manager.cc +++ b/quic/core/quic_stream_id_manager.cc
@@ -121,7 +121,8 @@ } void QuicStreamIdManager::OnStreamClosed(QuicStreamId stream_id) { - DCHECK_NE(QuicUtils::IsBidirectionalStreamId(stream_id), unidirectional_); + DCHECK_NE(QuicUtils::IsBidirectionalStreamId(stream_id, version_), + unidirectional_); if (QuicUtils::IsOutgoingStreamId(version_, stream_id, perspective_)) { // Nothing to do for outgoing streams. return; @@ -159,7 +160,8 @@ const QuicStreamId stream_id, std::string* error_details) { // |stream_id| must be an incoming stream of the right directionality. - DCHECK_NE(QuicUtils::IsBidirectionalStreamId(stream_id), unidirectional_); + DCHECK_NE(QuicUtils::IsBidirectionalStreamId(stream_id, version_), + unidirectional_); DCHECK_NE(QuicUtils::IsServerInitiatedStreamId(version_.transport_version, stream_id), perspective_ == Perspective::IS_SERVER); @@ -205,7 +207,7 @@ } bool QuicStreamIdManager::IsAvailableStream(QuicStreamId id) const { - DCHECK_NE(QuicUtils::IsBidirectionalStreamId(id), unidirectional_); + DCHECK_NE(QuicUtils::IsBidirectionalStreamId(id, version_), unidirectional_); if (QuicUtils::IsOutgoingStreamId(version_, id, perspective_)) { // Stream IDs under next_ougoing_stream_id_ are either open or previously // open but now closed.
diff --git a/quic/core/quic_utils.cc b/quic/core/quic_utils.cc index 72d422b..de43503 100644 --- a/quic/core/quic_utils.cc +++ b/quic/core/quic_utils.cc
@@ -428,15 +428,21 @@ } // static -bool QuicUtils::IsBidirectionalStreamId(QuicStreamId id) { +bool QuicUtils::IsBidirectionalStreamId(QuicStreamId id, + ParsedQuicVersion version) { + DCHECK(!GetQuicReloadableFlag(quic_fix_gquic_stream_type) || + version.HasIetfQuicFrames()); return id % 4 < 2; } // static StreamType QuicUtils::GetStreamType(QuicStreamId id, Perspective perspective, - bool peer_initiated) { - if (IsBidirectionalStreamId(id)) { + bool peer_initiated, + ParsedQuicVersion version) { + DCHECK(!GetQuicReloadableFlag(quic_fix_gquic_stream_type) || + version.HasIetfQuicFrames()); + if (IsBidirectionalStreamId(id, version)) { return BIDIRECTIONAL; }
diff --git a/quic/core/quic_utils.h b/quic/core/quic_utils.h index e470e6f..9e190e0 100644 --- a/quic/core/quic_utils.h +++ b/quic/core/quic_utils.h
@@ -146,14 +146,16 @@ // Returns true if |id| is considered as bidirectional stream ID. Only used in // v99. - static bool IsBidirectionalStreamId(QuicStreamId id); + static bool IsBidirectionalStreamId(QuicStreamId id, + ParsedQuicVersion version); // Returns stream type. Either |perspective| or |peer_initiated| would be // enough together with |id|. This method enforces that the three parameters // are consistent. Only used in v99. static StreamType GetStreamType(QuicStreamId id, Perspective perspective, - bool peer_initiated); + bool peer_initiated, + ParsedQuicVersion version); // Returns the delta between consecutive stream IDs of the same type. static QuicStreamId StreamIdDelta(QuicTransportVersion version);
diff --git a/quic/core/uber_quic_stream_id_manager.cc b/quic/core/uber_quic_stream_id_manager.cc index 64cbd58..2f33594 100644 --- a/quic/core/uber_quic_stream_id_manager.cc +++ b/quic/core/uber_quic_stream_id_manager.cc
@@ -17,7 +17,8 @@ QuicStreamCount max_open_outgoing_unidirectional_streams, QuicStreamCount max_open_incoming_bidirectional_streams, QuicStreamCount max_open_incoming_unidirectional_streams) - : bidirectional_stream_id_manager_(delegate, + : version_(version), + bidirectional_stream_id_manager_(delegate, /*unidirectional=*/false, perspective, version, @@ -69,7 +70,7 @@ bool UberQuicStreamIdManager::MaybeIncreaseLargestPeerStreamId( QuicStreamId id, std::string* error_details) { - if (QuicUtils::IsBidirectionalStreamId(id)) { + if (QuicUtils::IsBidirectionalStreamId(id, version_)) { return bidirectional_stream_id_manager_.MaybeIncreaseLargestPeerStreamId( id, error_details); } @@ -78,7 +79,7 @@ } void UberQuicStreamIdManager::OnStreamClosed(QuicStreamId id) { - if (QuicUtils::IsBidirectionalStreamId(id)) { + if (QuicUtils::IsBidirectionalStreamId(id, version_)) { bidirectional_stream_id_manager_.OnStreamClosed(id); return; } @@ -97,7 +98,7 @@ } bool UberQuicStreamIdManager::IsAvailableStream(QuicStreamId id) const { - if (QuicUtils::IsBidirectionalStreamId(id)) { + if (QuicUtils::IsBidirectionalStreamId(id, version_)) { return bidirectional_stream_id_manager_.IsAvailableStream(id); } return unidirectional_stream_id_manager_.IsAvailableStream(id);
diff --git a/quic/core/uber_quic_stream_id_manager.h b/quic/core/uber_quic_stream_id_manager.h index aafe0b6..0e03b42 100644 --- a/quic/core/uber_quic_stream_id_manager.h +++ b/quic/core/uber_quic_stream_id_manager.h
@@ -94,6 +94,7 @@ friend class test::QuicSessionPeer; friend class test::UberQuicStreamIdManagerPeer; + ParsedQuicVersion version_; // Manages stream IDs of bidirectional streams. QuicStreamIdManager bidirectional_stream_id_manager_;
diff --git a/quic/quic_transport/quic_transport_stream.cc b/quic/quic_transport/quic_transport_stream.cc index 401ae7e..0fb988a 100644 --- a/quic/quic_transport/quic_transport_stream.cc +++ b/quic/quic_transport/quic_transport_stream.cc
@@ -23,7 +23,8 @@ /*is_static=*/false, QuicUtils::GetStreamType(id, session->connection()->perspective(), - session->IsIncomingStream(id))), + session->IsIncomingStream(id), + session->version())), session_interface_(session_interface) {} size_t QuicTransportStream::Read(char* buffer, size_t buffer_size) {
diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc index 61d4ec6..ce2f7b8 100644 --- a/quic/test_tools/quic_test_utils.cc +++ b/quic/test_tools/quic_test_utils.cc
@@ -1256,12 +1256,12 @@ } StreamType DetermineStreamType(QuicStreamId id, - QuicTransportVersion version, + ParsedQuicVersion version, Perspective perspective, bool is_incoming, StreamType default_type) { - return VersionHasIetfQuicFrames(version) - ? QuicUtils::GetStreamType(id, perspective, is_incoming) + return version.HasIetfQuicFrames() + ? QuicUtils::GetStreamType(id, perspective, is_incoming, version) : default_type; }
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index c205b95..f4f2aeb 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -1620,7 +1620,7 @@ int n); StreamType DetermineStreamType(QuicStreamId id, - QuicTransportVersion version, + ParsedQuicVersion version, Perspective perspective, bool is_incoming, StreamType default_type);
diff --git a/quic/tools/quic_simple_server_session.cc b/quic/tools/quic_simple_server_session.cc index 595fda7..387b313 100644 --- a/quic/tools/quic_simple_server_session.cc +++ b/quic/tools/quic_simple_server_session.cc
@@ -158,7 +158,7 @@ // index for it in promised_streams_ can be calculated. QuicStreamId next_stream_id = next_outgoing_unidirectional_stream_id(); if (VersionHasIetfQuicFrames(transport_version())) { - DCHECK(!QuicUtils::IsBidirectionalStreamId(frame.stream_id)); + DCHECK(!QuicUtils::IsBidirectionalStreamId(frame.stream_id, version())); } DCHECK_GE(frame.stream_id, next_stream_id); size_t index = (frame.stream_id - next_stream_id) /