gfe-relnote: Implement sending the MAX_PUSH_ID frame when the client sets a non-zero maximum push id. PiperOrigin-RevId: 263372703 Change-Id: If68ed7819648fdd7bd5ec968592d04f88467f5d2
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc index 14dd32c..d6c8486 100644 --- a/quic/core/http/end_to_end_test.cc +++ b/quic/core/http/end_to_end_test.cc
@@ -280,7 +280,9 @@ } client->UseConnectionIdLength(override_server_connection_id_length_); client->UseClientConnectionIdLength(override_client_connection_id_length_); - client->client()->set_max_allowed_push_id(kMaxQuicStreamId); + if (support_server_push_) { + client->client()->set_max_allowed_push_id(kMaxQuicStreamId); + } client->Connect(); return client; } @@ -4114,6 +4116,29 @@ EXPECT_EQ(QUIC_INVALID_STREAM_ID, client_->connection_error()); } +TEST_P(EndToEndTest, TestMaxPushId) { + // Has to be before version test, see EndToEndTest::TearDown() + ASSERT_TRUE(Initialize()); + if (!VersionHasIetfQuicFrames(negotiated_version_.transport_version)) { + // Only runs for IETF QUIC. + return; + } + + EXPECT_TRUE(client_->client()->WaitForCryptoHandshakeConfirmed()); + static_cast<QuicSpdySession*>(client_->client()->session()) + ->set_max_allowed_push_id(kMaxQuicStreamId); + + client_->SendSynchronousRequest("/foo"); + + EXPECT_EQ(kMaxQuicStreamId, + static_cast<QuicSpdySession*>(client_->client()->session()) + ->max_allowed_push_id()); + + EXPECT_EQ( + kMaxQuicStreamId, + static_cast<QuicSpdySession*>(GetServerSession())->max_allowed_push_id()); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/http/quic_headers_stream_test.cc b/quic/core/http/quic_headers_stream_test.cc index e3e03dd..2035d47 100644 --- a/quic/core/http/quic_headers_stream_test.cc +++ b/quic/core/http/quic_headers_stream_test.cc
@@ -397,6 +397,8 @@ } TEST_P(QuicHeadersStreamTest, WritePushPromises) { + session_.set_max_allowed_push_id(kMaxQuicStreamId); + for (QuicStreamId stream_id = client_id_1_; stream_id < client_id_3_; stream_id += next_stream_id_) { QuicStreamId promised_stream_id = NextPromisedStreamId();
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc index f899e78..1808a92 100644 --- a/quic/core/http/quic_receive_control_stream.cc +++ b/quic/core/http/quic_receive_control_stream.cc
@@ -52,7 +52,13 @@ return false; } - bool OnMaxPushIdFrame(const MaxPushIdFrame& /*frame*/) override { + bool OnMaxPushIdFrame(const MaxPushIdFrame& frame) override { + if (stream_->session()->perspective() == Perspective::IS_SERVER) { + QuicSpdySession* spdy_session = + static_cast<QuicSpdySession*>(stream_->session()); + spdy_session->set_max_allowed_push_id(frame.push_id); + return true; + } CloseConnectionOnWrongFrame("Max Push Id"); return false; }
diff --git a/quic/core/http/quic_send_control_stream.cc b/quic/core/http/quic_send_control_stream.cc index 53ff3ef..e033b61 100644 --- a/quic/core/http/quic_send_control_stream.cc +++ b/quic/core/http/quic_send_control_stream.cc
@@ -65,4 +65,15 @@ nullptr); } +void QuicSendControlStream::SendMaxPushIdFrame(PushId max_push_id) { + QuicConnection::ScopedPacketFlusher flusher(session()->connection()); + + MaxPushIdFrame frame; + frame.push_id = max_push_id; + std::unique_ptr<char[]> buffer; + QuicByteCount frame_length = encoder_.SerializeMaxPushIdFrame(frame, &buffer); + WriteOrBufferData(QuicStringPiece(buffer.get(), frame_length), + /*fin = */ false, nullptr); +} + } // namespace quic
diff --git a/quic/core/http/quic_send_control_stream.h b/quic/core/http/quic_send_control_stream.h index 566bcc8..b38ae95 100644 --- a/quic/core/http/quic_send_control_stream.h +++ b/quic/core/http/quic_send_control_stream.h
@@ -34,6 +34,9 @@ // stream. Settings frame must be the first frame sent on this stream. void SendSettingsFrame(); + // Construct a MAX_PUSH_ID frame and send it on this stream. + void SendMaxPushIdFrame(PushId max_push_id); + // Send |Priority| on this stream. It must be sent after settings. void WritePriority(const PriorityFrame& priority);
diff --git a/quic/core/http/quic_spdy_client_session_base.cc b/quic/core/http/quic_spdy_client_session_base.cc index d5cdb28..a2a9aae 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 @@ -43,6 +42,10 @@ void QuicSpdyClientSessionBase::OnCryptoHandshakeEvent( CryptoHandshakeEvent event) { QuicSpdySession::OnCryptoHandshakeEvent(event); + if (event == HANDSHAKE_CONFIRMED && max_allowed_push_id() > 0 && + VersionHasIetfQuicFrames(connection()->transport_version())) { + SendMaxPushId(max_allowed_push_id()); + } } void QuicSpdyClientSessionBase::OnInitialHeadersComplete( @@ -193,6 +196,8 @@ push_promise_index_->promised_by_url()->erase(promised->url()); // Since promised_by_id_ contains the unique_ptr, this will destroy // promised. + // ToDo: Consider implementing logic to send a new MAX_PUSH_ID frame to allow + // another stream to be promised. promised_by_id_.erase(promised->id()); if (!VersionUsesQpack(connection()->transport_version())) { headers_stream()->MaybeReleaseSequencerBuffer(); @@ -223,9 +228,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_session.cc b/quic/core/http/quic_spdy_session.cc index 90560ad..e6ec954 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -320,7 +320,8 @@ frame_len_(0), supports_push_promise_(perspective() == Perspective::IS_CLIENT), spdy_framer_(SpdyFramer::ENABLE_COMPRESSION), - spdy_framer_visitor_(new SpdyFramerVisitor(this)) { + spdy_framer_visitor_(new SpdyFramerVisitor(this)), + max_allowed_push_id_(0) { h2_deframer_.set_visitor(spdy_framer_visitor_.get()); h2_deframer_.set_debug_visitor(spdy_framer_visitor_.get()); spdy_framer_.set_debug_visitor(spdy_framer_visitor_.get()); @@ -514,6 +515,13 @@ return; } + if (VersionHasIetfQuicFrames(connection()->transport_version()) && + promised_stream_id > max_allowed_push_id()) { + QUIC_BUG + << "Server shouldn't send push id higher than client's MAX_PUSH_ID."; + return; + } + if (!VersionHasStreamType(connection()->transport_version())) { SpdyPushPromiseIR push_promise(original_stream_id, promised_stream_id, std::move(headers)); @@ -916,4 +924,25 @@ } } +void QuicSpdySession::set_max_allowed_push_id( + QuicStreamId max_allowed_push_id) { + if (VersionHasIetfQuicFrames(connection()->transport_version()) && + perspective() == Perspective::IS_SERVER && + max_allowed_push_id > max_allowed_push_id_) { + OnCanCreateNewOutgoingStream(true); + } + + max_allowed_push_id_ = max_allowed_push_id; + + if (VersionHasIetfQuicFrames(connection()->transport_version()) && + perspective() == Perspective::IS_CLIENT && IsHandshakeConfirmed()) { + SendMaxPushId(max_allowed_push_id); + } +} + +void QuicSpdySession::SendMaxPushId(QuicStreamId max_allowed_push_id) { + DCHECK(VersionHasStreamType(connection()->transport_version())); + send_control_stream_->SendMaxPushIdFrame(max_allowed_push_id); +} + } // namespace quic
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h index 68becb6..ce8fff0 100644 --- a/quic/core/http/quic_spdy_session.h +++ b/quic/core/http/quic_spdy_session.h
@@ -186,6 +186,10 @@ // those streams are not initialized yet. void OnCanCreateNewOutgoingStream(bool unidirectional) override; + void set_max_allowed_push_id(QuicStreamId max_allowed_push_id); + + QuicStreamId max_allowed_push_id() { return max_allowed_push_id_; } + protected: // Override CreateIncomingStream(), CreateOutgoingBidirectionalStream() and // CreateOutgoingUnidirectionalStream() with QuicSpdyStream return type to @@ -252,6 +256,11 @@ // Initializes HTTP/3 unidirectional streams if not yet initialzed. virtual void MaybeInitializeHttp3UnidirectionalStreams(); + void set_max_uncompressed_header_bytes( + size_t set_max_uncompressed_header_bytes); + + void SendMaxPushId(QuicStreamId max_allowed_push_id); + private: friend class test::QuicSpdySessionPeer; @@ -308,6 +317,7 @@ // TODO(renjietang): Replace these two members with actual QPACK send streams. NoopQpackStreamSenderDelegate encoder_stream_sender_delegate_; NoopQpackStreamSenderDelegate decoder_stream_sender_delegate_; + QuicStreamId max_allowed_push_id_; }; } // namespace quic
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 57ced3a..cbfff08 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -575,6 +575,8 @@ return false; } + bool IsHandshakeConfirmed() { return is_handshake_confirmed_; } + private: friend class test::QuicSessionPeer;
diff --git a/quic/tools/quic_simple_server_session.cc b/quic/tools/quic_simple_server_session.cc index cf2c569..d06cd4b 100644 --- a/quic/tools/quic_simple_server_session.cc +++ b/quic/tools/quic_simple_server_session.cc
@@ -76,6 +76,10 @@ request_url, resource, original_request_headers); highest_promised_stream_id_ += QuicUtils::StreamIdDelta(connection()->transport_version()); + if (VersionHasIetfQuicFrames(connection()->transport_version()) && + highest_promised_stream_id_ > max_allowed_push_id()) { + return; + } SendPushPromise(original_stream_id, highest_promised_stream_id_, headers.Clone()); promised_streams_.push_back(PromisedStreamInfo(
diff --git a/quic/tools/quic_simple_server_session_test.cc b/quic/tools/quic_simple_server_session_test.cc index 7fbce71..10661ba 100644 --- a/quic/tools/quic_simple_server_session_test.cc +++ b/quic/tools/quic_simple_server_session_test.cc
@@ -711,6 +711,7 @@ // opened and send push response. TEST_P(QuicSimpleServerSessionServerPushTest, TestPromisePushResources) { MaybeConsumeHeadersStreamData(); + session_->set_max_allowed_push_id(kMaxQuicStreamId); size_t num_resources = kMaxStreamsForTest + 5; PromisePushResources(num_resources); EXPECT_EQ(kMaxStreamsForTest, session_->GetNumOpenOutgoingStreams()); @@ -721,6 +722,7 @@ TEST_P(QuicSimpleServerSessionServerPushTest, HandlePromisedPushRequestsAfterStreamDraining) { MaybeConsumeHeadersStreamData(); + session_->set_max_allowed_push_id(kMaxQuicStreamId); size_t num_resources = kMaxStreamsForTest + 1; QuicByteCount data_frame_header_length = PromisePushResources(num_resources); QuicStreamId next_out_going_stream_id; @@ -789,6 +791,7 @@ TEST_P(QuicSimpleServerSessionServerPushTest, ResetPromisedStreamToCancelServerPush) { MaybeConsumeHeadersStreamData(); + session_->set_max_allowed_push_id(kMaxQuicStreamId); // Having two extra resources to be send later. One of them will be reset, so // when opened stream become close, only one will become open. @@ -877,6 +880,7 @@ TEST_P(QuicSimpleServerSessionServerPushTest, CloseStreamToHandleMorePromisedStream) { MaybeConsumeHeadersStreamData(); + session_->set_max_allowed_push_id(kMaxQuicStreamId); size_t num_resources = kMaxStreamsForTest + 1; if (VersionHasIetfQuicFrames(transport_version())) { // V99 will send out a stream-id-blocked frame when the we desired to exceed