If WebTransport is on, buffer all incoming HTTP/3 requests until SETTINGS are received.
This fixes a bug where the server would reject WebTransport requests based on lack of WebTransport support if client settings arrive after the request.
PiperOrigin-RevId: 365819131
Change-Id: I8e9bc2069525b7b7115903cb5dc02e3a36cdd2e4
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 48ed82d..6138269 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -16,6 +16,7 @@
#include "quic/core/crypto/null_encrypter.h"
#include "quic/core/http/http_constants.h"
#include "quic/core/http/quic_spdy_client_stream.h"
+#include "quic/core/http/web_transport_http3.h"
#include "quic/core/quic_data_writer.h"
#include "quic/core/quic_epoll_connection_helper.h"
#include "quic/core/quic_error_codes.h"
@@ -677,6 +678,41 @@
return WaitForFooResponseAndCheckIt(client_.get());
}
+ WebTransportHttp3* CreateWebTransportSession(const std::string& path,
+ bool wait_for_server_response) {
+ // Wait until we receive the settings from the server indicating
+ // WebTransport support.
+ client_->WaitUntil(
+ 2000, [this]() { return GetClientSession()->SupportsWebTransport(); });
+ if (!GetClientSession()->SupportsWebTransport()) {
+ return nullptr;
+ }
+
+ spdy::SpdyHeaderBlock headers;
+ headers[":scheme"] = "https";
+ headers[":authority"] = "localhost";
+ headers[":path"] = path;
+ headers[":method"] = "CONNECT";
+ headers[":protocol"] = "webtransport";
+
+ client_->SendMessage(headers, "", /*fin=*/false);
+ QuicSpdyStream* stream = client_->latest_created_stream();
+ if (stream->web_transport() == nullptr) {
+ return nullptr;
+ }
+ WebTransportSessionId id = client_->latest_created_stream()->id();
+ QuicSpdySession* client_session = GetClientSession();
+ if (client_session->GetWebTransportSession(id) == nullptr) {
+ return nullptr;
+ }
+ WebTransportHttp3* session = client_session->GetWebTransportSession(id);
+ if (wait_for_server_response) {
+ client_->WaitUntil(-1,
+ [stream]() { return stream->headers_decompressed(); });
+ }
+ return session;
+ }
+
ScopedEnvironmentForThreads environment_;
bool initialized_;
// If true, the Initialize() function will create |client_| and starts to
@@ -5690,24 +5726,34 @@
return;
}
- spdy::SpdyHeaderBlock headers;
- headers[":scheme"] = "https";
- headers[":authority"] = "localhost";
- headers[":path"] = "/echo";
- headers[":method"] = "CONNECT";
- headers[":protocol"] = "webtransport";
-
- client_->SendMessage(headers, "", /*fin=*/false);
- QuicSpdyStream* stream = client_->latest_created_stream();
- EXPECT_TRUE(stream->web_transport() != nullptr);
- WebTransportSessionId id = client_->latest_created_stream()->id();
- QuicSpdySession* client_session = GetClientSession();
- EXPECT_TRUE(client_session->GetWebTransportSession(id) != nullptr);
- client_->WaitUntil(-1, [stream]() { return stream->headers_decompressed(); });
+ WebTransportHttp3* web_transport =
+ CreateWebTransportSession("/echo", /*wait_for_server_response=*/true);
server_thread_->Pause();
QuicSpdySession* server_session = GetServerSession();
- EXPECT_TRUE(server_session->GetWebTransportSession(id) != nullptr);
+ EXPECT_TRUE(server_session->GetWebTransportSession(web_transport->id()) !=
+ nullptr);
+ server_thread_->Resume();
+}
+
+TEST_P(EndToEndTest, WebTransportSessionWithLoss) {
+ enable_web_transport_ = true;
+ // Enable loss to verify all permutations of receiving SETTINGS and
+ // request/response data.
+ SetPacketLossPercentage(30);
+ ASSERT_TRUE(Initialize());
+
+ if (!version_.UsesHttp3()) {
+ return;
+ }
+
+ WebTransportHttp3* web_transport =
+ CreateWebTransportSession("/echo", /*wait_for_server_response=*/true);
+
+ server_thread_->Pause();
+ QuicSpdySession* server_session = GetServerSession();
+ EXPECT_TRUE(server_session->GetWebTransportSession(web_transport->id()) !=
+ nullptr);
server_thread_->Resume();
}
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 1d20a9c..442e1eb 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -1025,6 +1025,7 @@
if (debug_visitor_ != nullptr) {
debug_visitor_->OnSettingsFrameResumed(out);
}
+ QUICHE_DCHECK(streams_waiting_for_settings_.empty());
for (const auto& setting : out.values) {
OnSetting(setting.first, setting.second);
}
@@ -1069,6 +1070,17 @@
return false;
}
}
+ for (QuicStreamId stream_id : streams_waiting_for_settings_) {
+ QUICHE_DCHECK(ShouldBufferRequestsUntilSettings());
+ QuicSpdyStream* stream = GetOrCreateSpdyDataStream(stream_id);
+ if (stream == nullptr) {
+ // The stream may no longer exist, since it is possible for a stream to
+ // get reset while waiting for the SETTINGS frame.
+ continue;
+ }
+ stream->OnDataAvailable();
+ }
+ streams_waiting_for_settings_.clear();
return true;
}
@@ -1091,6 +1103,8 @@
}
bool QuicSpdySession::OnSetting(uint64_t id, uint64_t value) {
+ any_settings_received_ = true;
+
if (VersionUsesHttp3(transport_version())) {
// SETTINGS frame received on the control stream.
switch (id) {
@@ -1762,6 +1776,20 @@
return connect_stream->web_transport();
}
+bool QuicSpdySession::ShouldProcessIncomingRequests() {
+ if (!ShouldBufferRequestsUntilSettings()) {
+ return true;
+ }
+
+ return any_settings_received_;
+}
+
+void QuicSpdySession::OnStreamWaitingForClientSettings(QuicStreamId id) {
+ QUICHE_DCHECK(ShouldBufferRequestsUntilSettings());
+ QUICHE_DCHECK(QuicUtils::IsBidirectionalStreamId(id, version()));
+ streams_waiting_for_settings_.insert(id);
+}
+
#undef ENDPOINT // undef for jumbo builds
} // namespace quic
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index a3fda3f..3a9b60e 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h
@@ -10,6 +10,7 @@
#include <string>
#include "absl/container/flat_hash_map.h"
+#include "absl/container/flat_hash_set.h"
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "quic/core/http/http_frames.h"
@@ -446,6 +447,20 @@
// session is associated with the given ID.
WebTransportHttp3* GetWebTransportSession(WebTransportSessionId id);
+ // If true, no data on bidirectional streams will be processed by the server
+ // until the SETTINGS are received. Only works for HTTP/3.
+ bool ShouldBufferRequestsUntilSettings() {
+ return version().UsesHttp3() && perspective() == Perspective::IS_SERVER &&
+ WillNegotiateWebTransport();
+ }
+
+ // Returns if the incoming bidirectional streams should process data. This is
+ // usually true, but in certain cases we would want to wait until the settings
+ // are received.
+ bool ShouldProcessIncomingRequests();
+
+ void OnStreamWaitingForClientSettings(QuicStreamId id);
+
protected:
// Override CreateIncomingStream(), CreateOutgoingBidirectionalStream() and
// CreateOutgoingUnidirectionalStream() with QuicSpdyStream return type to
@@ -660,6 +675,14 @@
absl::flat_hash_map<QuicDatagramFlowId, Http3DatagramVisitor*>
h3_datagram_registrations_;
+
+ // Whether any settings have been received, either from the peer or from a
+ // session ticket.
+ bool any_settings_received_ = false;
+
+ // If ShouldBufferRequestsUntilSettings() is true, all streams that are
+ // blocked by that are tracked here.
+ absl::flat_hash_set<QuicStreamId> streams_waiting_for_settings_;
};
} // namespace quic
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 54747d3..d434931 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -3554,6 +3554,31 @@
EXPECT_TRUE(session_.SupportsWebTransport());
}
+TEST_P(QuicSpdySessionTestServer, WebTransportSetting) {
+ if (!version().UsesHttp3()) {
+ return;
+ }
+ SetQuicReloadableFlag(quic_h3_datagram, true);
+ session_.set_supports_webtransport(true);
+
+ EXPECT_FALSE(session_.SupportsWebTransport());
+ EXPECT_FALSE(session_.ShouldProcessIncomingRequests());
+
+ CompleteHandshake();
+
+ SettingsFrame server_settings;
+ server_settings.values[SETTINGS_H3_DATAGRAM] = 1;
+ server_settings.values[SETTINGS_WEBTRANS_DRAFT00] = 1;
+ std::string data =
+ std::string(1, kControlStream) + EncodeSettings(server_settings);
+ QuicStreamId stream_id =
+ GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3);
+ QuicStreamFrame frame(stream_id, /*fin=*/false, /*offset=*/0, data);
+ session_.OnStreamFrame(frame);
+ EXPECT_TRUE(session_.SupportsWebTransport());
+ EXPECT_TRUE(session_.ShouldProcessIncomingRequests());
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index 3752880..1302bf2 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -795,6 +795,11 @@
return;
}
+ if (!spdy_session()->ShouldProcessIncomingRequests()) {
+ spdy_session()->OnStreamWaitingForClientSettings(id());
+ return;
+ }
+
if (is_decoder_processing_input_) {
// Let the outermost nested OnDataAvailable() call do the work.
return;
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 424fa0a..c84203d 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -256,9 +256,11 @@
return &crypto_stream_;
}
- bool ShouldNegotiateWebTransport() override { return true; }
+ bool ShouldNegotiateWebTransport() override { return enable_webtransport_; }
+ void EnableWebTransport() { enable_webtransport_ = true; }
private:
+ bool enable_webtransport_ = false;
StrictMock<TestCryptoStream> crypto_stream_;
};
@@ -3080,6 +3082,7 @@
}
InitializeWithPerspective(kShouldProcessData, Perspective::IS_CLIENT);
+ session_->EnableWebTransport();
QuicSpdySessionPeer::EnableWebTransport(*session_);
EXPECT_CALL(*stream_, WriteHeadersMock(false));
@@ -3100,6 +3103,7 @@
}
Initialize(kShouldProcessData);
+ session_->EnableWebTransport();
QuicSpdySessionPeer::EnableWebTransport(*session_);
headers_[":method"] = "CONNECT";
