Automated g4 rollback of changelist 258400699. *** Reason for rollback *** Chromium does not support dynamic_cast, so this cannot be merged as is. It will take some time to figure out how to determine if session() is a QuicSpdyClientSession instance or not, so I need to revert it for the time being to unblock the QUICHE merge. *** Original change description *** gfe-relnote: Add support for sending MAX_PUSH_ID, defaulting to zero and close connection if we receive a push ID higher than the max. Protected by the existing disabled and blocked quic_enable_version_99 reloadable flag. *** PiperOrigin-RevId: 258464901 Change-Id: Ic738f37737850cff8f46a30491637b3314c99144
diff --git a/quic/core/http/quic_spdy_client_session_base.cc b/quic/core/http/quic_spdy_client_session_base.cc index f27411a..ba5a9cc 100644 --- a/quic/core/http/quic_spdy_client_session_base.cc +++ b/quic/core/http/quic_spdy_client_session_base.cc
@@ -24,8 +24,7 @@ : QuicSpdySession(connection, nullptr, config, supported_versions), push_promise_index_(push_promise_index), largest_promised_stream_id_( - QuicUtils::GetInvalidStreamId(connection->transport_version())), - max_allowed_push_id_(0) {} + QuicUtils::GetInvalidStreamId(connection->transport_version())) {} QuicSpdyClientSessionBase::~QuicSpdyClientSessionBase() { // all promised streams for this session @@ -89,14 +88,6 @@ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); return; } - - if (VersionHasIetfQuicFrames(connection()->transport_version()) && - promised_stream_id > max_allowed_push_id()) { - connection()->CloseConnection( - QUIC_INVALID_STREAM_ID, - "Received push stream id higher than MAX_PUSH_ID.", - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); - } largest_promised_stream_id_ = promised_stream_id; QuicSpdyStream* stream = GetSpdyDataStream(stream_id); @@ -218,9 +209,4 @@ return !HasActiveRequestStreams() && promised_by_id_.empty(); } -void QuicSpdyClientSessionBase::set_max_allowed_push_id( - QuicStreamId max_allowed_push_id) { - max_allowed_push_id_ = max_allowed_push_id; -} - } // namespace quic
diff --git a/quic/core/http/quic_spdy_client_session_base.h b/quic/core/http/quic_spdy_client_session_base.h index 98b8589..aec5e75 100644 --- a/quic/core/http/quic_spdy_client_session_base.h +++ b/quic/core/http/quic_spdy_client_session_base.h
@@ -111,10 +111,6 @@ // Returns true if there are no active requests and no promised streams. bool ShouldReleaseHeadersStreamSequencerBuffer() override; - void set_max_allowed_push_id(QuicStreamId max_allowed_push_id); - - QuicStreamId max_allowed_push_id() { return max_allowed_push_id_; } - size_t get_max_promises() const { return max_open_incoming_unidirectional_streams() * kMaxPromisedStreamsMultiplier; @@ -138,7 +134,6 @@ QuicClientPushPromiseIndex* push_promise_index_; QuicPromisedByIdMap promised_by_id_; QuicStreamId largest_promised_stream_id_; - QuicStreamId max_allowed_push_id_; }; } // namespace quic
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc index dfa3031..716ae42 100644 --- a/quic/core/http/quic_spdy_client_session_test.cc +++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -610,38 +610,6 @@ QuicHeaderList()); } -TEST_P(QuicSpdyClientSessionTest, PushPromiseStreamIdTooHigh) { - // Initialize crypto before the client session will create a stream. - CompleteCryptoHandshake(); - QuicStreamId stream_id = - QuicSessionPeer::GetNextOutgoingBidirectionalStreamId(session_.get()); - QuicSessionPeer::ActivateStream( - session_.get(), QuicMakeUnique<QuicSpdyClientStream>( - stream_id, session_.get(), BIDIRECTIONAL)); - - session_->set_max_allowed_push_id(GetNthServerInitiatedUnidirectionalStreamId( - connection_->transport_version(), 10)); - if (VersionHasIetfQuicFrames(connection_->transport_version())) { - // TODO(b/136295430) Use PushId to represent Push IDs instead of - // QuicStreamId. - EXPECT_CALL( - *connection_, - CloseConnection(QUIC_INVALID_STREAM_ID, - "Received push stream id higher than MAX_PUSH_ID.", _)); - } - auto promise_id = GetNthServerInitiatedUnidirectionalStreamId( - connection_->transport_version(), 11); - auto headers = QuicHeaderList(); - headers.OnHeaderBlockStart(); - headers.OnHeader(":path", "/bar"); - headers.OnHeader(":authority", "www.google.com"); - headers.OnHeader(":version", "HTTP/1.1"); - headers.OnHeader(":method", "GET"); - headers.OnHeader(":scheme", "https"); - headers.OnHeaderBlockEnd(0, 0); - session_->OnPromiseHeaderList(stream_id, promise_id, 0, headers); -} - TEST_P(QuicSpdyClientSessionTest, PushPromiseOnPromiseHeadersAlreadyClosed) { // Initialize crypto before the client session will create a stream. CompleteCryptoHandshake();
diff --git a/quic/tools/quic_client_base.cc b/quic/tools/quic_client_base.cc index e53911d..b03f7cc 100644 --- a/quic/tools/quic_client_base.cc +++ b/quic/tools/quic_client_base.cc
@@ -85,10 +85,6 @@ break; } } - if (max_allowed_push_id_ > 0 && - dynamic_cast<QuicSpdyClientSession*>(session())) - static_cast<QuicSpdyClientSession*>(session())->set_max_allowed_push_id( - max_allowed_push_id_); return session()->connection()->connected(); }
diff --git a/quic/tools/quic_client_base.h b/quic/tools/quic_client_base.h index a54c621..fb15b86 100644 --- a/quic/tools/quic_client_base.h +++ b/quic/tools/quic_client_base.h
@@ -211,9 +211,6 @@ crypto_config_.set_pre_shared_key(key); } - // Set the max promise id for the client session. - void set_max_allowed_push_id(QuicStreamId max) { max_allowed_push_id_ = max; } - protected: // TODO(rch): Move GetNumSentClientHellosFromSession and // GetNumReceivedServerConfigUpdatesFromSession into a new/better @@ -332,9 +329,6 @@ // The network helper used to create sockets and manage the event loop. // Not owned by this class. std::unique_ptr<NetworkHelper> network_helper_; - - // The max promise id to set on the client session when created. - QuicStreamId max_allowed_push_id_; }; } // namespace quic