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) /
