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
