Add a new message status for sending HTTP Datagrams before SETTINGS are received.
Previously, `QuicSpdySession::SendHttp3Datagram` returned `MESSAGE_STATUS_INTERNAL_ERROR` if it was called before a SETTINGS frame was received. In this CL, I added a new enum called `MESSAGE_STATUS_SETTINGS_NOT_RECEIVED` to distinguish this case from other internal errors. This will allow a caller to take proper measures (e.g., drop or buffer datagrams) based on the return value of the method.
In QUICHE, callers report errors or log the `MESSAGE_STATUS_INTERNAL_ERROR`, so I added the `MESSAGE_STATUS_SETTINGS_NOT_RECEIVED` case in the callers to preserve the existing behavior.
In Envoy, `HttpDatagramHandler` is the only caller of the method. This change will not alter its behavior since both message statuses will be treated the same and reset the stream.
In Chromium, `QuicChromiumClientStream::Handle::WriteConnectUdpPayload()` treats both message statuses the same, so this CL will not change its behavior. After this CL is landed, the caller no longer needs to check `stream_->SupportsH3Datagram()` and instead can treat `MESSAGE_STATUS_SETTINGS_NOT_RECEIVED` the same way it treats `MESSAGE_STATUS_BLOCKED`.
PiperOrigin-RevId: 670652644
diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc
index d53a4b2..13869f2 100644
--- a/quiche/quic/core/http/quic_spdy_session.cc
+++ b/quiche/quic/core/http/quic_spdy_session.cc
@@ -1761,9 +1761,18 @@
MessageStatus QuicSpdySession::SendHttp3Datagram(QuicStreamId stream_id,
absl::string_view payload) {
if (!SupportsH3Datagram()) {
- QUIC_BUG(send http datagram too early)
- << "Refusing to send HTTP Datagram before SETTINGS received";
- return MESSAGE_STATUS_INTERNAL_ERROR;
+ if (LocalHttpDatagramSupport() == HttpDatagramSupport::kNone) {
+ QUIC_BUG(http datagram disabled locally)
+ << "Cannot send HTTP Datagram when disabled locally";
+ return MESSAGE_STATUS_UNSUPPORTED;
+ } else if (!settings_received_) {
+ QUIC_DLOG(INFO)
+ << "Refusing to send HTTP Datagram before SETTINGS received";
+ return MESSAGE_STATUS_SETTINGS_NOT_RECEIVED;
+ } else {
+ QUIC_DLOG(INFO) << "Refusing to send HTTP Datagram without peer support";
+ return MESSAGE_STATUS_UNSUPPORTED;
+ }
}
// Stream ID is sent divided by four as per the specification.
uint64_t stream_id_to_write = stream_id / kHttpDatagramStreamIdDivisor;
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc
index 5ede717..7deb354 100644
--- a/quiche/quic/core/http/quic_spdy_stream_test.cc
+++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -3377,6 +3377,44 @@
MESSAGE_STATUS_SUCCESS);
}
+TEST_P(QuicSpdyStreamTest, SendHttpDatagramWithoutLocalSupport) {
+ if (!UsesHttp3()) {
+ return;
+ }
+ Initialize(kShouldProcessData);
+ session_->set_local_http_datagram_support(HttpDatagramSupport::kNone);
+ std::string http_datagram_payload = {1, 2, 3, 4, 5, 6};
+ EXPECT_QUIC_BUG(stream_->SendHttp3Datagram(http_datagram_payload),
+ "Cannot send HTTP Datagram when disabled locally");
+}
+
+TEST_P(QuicSpdyStreamTest, SendHttpDatagramBeforeReceivingSettings) {
+ if (!UsesHttp3()) {
+ return;
+ }
+ Initialize(kShouldProcessData);
+ session_->set_local_http_datagram_support(HttpDatagramSupport::kRfc);
+ std::string http_datagram_payload = {1, 2, 3, 4, 5, 6};
+ EXPECT_EQ(stream_->SendHttp3Datagram(http_datagram_payload),
+ MESSAGE_STATUS_SETTINGS_NOT_RECEIVED);
+}
+
+TEST_P(QuicSpdyStreamTest, SendHttpDatagramWithoutPeerSupport) {
+ if (!UsesHttp3()) {
+ return;
+ }
+ Initialize(kShouldProcessData);
+ // Support HTTP Datagrams locally, but not by the peer.
+ session_->set_local_http_datagram_support(HttpDatagramSupport::kRfc);
+ SettingsFrame settings;
+ settings.values[SETTINGS_H3_DATAGRAM] = 0;
+ session_->OnSettingsFrame(settings);
+
+ std::string http_datagram_payload = {1, 2, 3, 4, 5, 6};
+ EXPECT_EQ(stream_->SendHttp3Datagram(http_datagram_payload),
+ MESSAGE_STATUS_UNSUPPORTED);
+}
+
TEST_P(QuicSpdyStreamTest, GetMaxDatagramSize) {
if (!UsesHttp3()) {
return;
diff --git a/quiche/quic/core/quic_types.cc b/quiche/quic/core/quic_types.cc
index a180fb1..31314cb 100644
--- a/quiche/quic/core/quic_types.cc
+++ b/quiche/quic/core/quic_types.cc
@@ -265,6 +265,7 @@
RETURN_STRING_LITERAL(MESSAGE_STATUS_UNSUPPORTED);
RETURN_STRING_LITERAL(MESSAGE_STATUS_BLOCKED);
RETURN_STRING_LITERAL(MESSAGE_STATUS_TOO_LARGE);
+ RETURN_STRING_LITERAL(MESSAGE_STATUS_SETTINGS_NOT_RECEIVED);
RETURN_STRING_LITERAL(MESSAGE_STATUS_INTERNAL_ERROR);
default:
return absl::StrCat("Unknown(", static_cast<int>(message_status), ")");
diff --git a/quiche/quic/core/quic_types.h b/quiche/quic/core/quic_types.h
index f3c5ff2..af3380b 100644
--- a/quiche/quic/core/quic_types.h
+++ b/quiche/quic/core/quic_types.h
@@ -645,6 +645,9 @@
// write blocked.
MESSAGE_STATUS_TOO_LARGE, // Failed to send message because the message is
// too large to fit into a single packet.
+ MESSAGE_STATUS_SETTINGS_NOT_RECEIVED, // Failed to send message because
+ // SETTINGS frame has not been received
+ // yet.
MESSAGE_STATUS_INTERNAL_ERROR, // Failed to send message because connection
// reaches an invalid state.
};
@@ -872,11 +875,11 @@
// ParsedClientHello contains client hello information extracted from a fully
// received client hello.
struct QUICHE_EXPORT ParsedClientHello {
- std::string sni; // QUIC crypto and TLS.
- std::string uaid; // QUIC crypto only.
- std::vector<uint16_t> supported_groups; // TLS only.
+ std::string sni; // QUIC crypto and TLS.
+ std::string uaid; // QUIC crypto only.
+ std::vector<uint16_t> supported_groups; // TLS only.
std::vector<uint16_t> cert_compression_algos; // TLS only.
- std::vector<std::string> alpns; // QUIC crypto and TLS.
+ std::vector<std::string> alpns; // QUIC crypto and TLS.
// The unvalidated retry token from the last received packet of a potentially
// multi-packet client hello. TLS only.
std::string retry_token;
diff --git a/quiche/quic/core/web_transport_interface.h b/quiche/quic/core/web_transport_interface.h
index f4ee35e..d3a6a12 100644
--- a/quiche/quic/core/web_transport_interface.h
+++ b/quiche/quic/core/web_transport_interface.h
@@ -39,6 +39,7 @@
case MESSAGE_STATUS_ENCRYPTION_NOT_ESTABLISHED:
case MESSAGE_STATUS_INTERNAL_ERROR:
case MESSAGE_STATUS_UNSUPPORTED:
+ case MESSAGE_STATUS_SETTINGS_NOT_RECEIVED:
return webtransport::DatagramStatus(
webtransport::DatagramStatusCode::kInternalError,
absl::StrCat("Internal error: ", MessageStatusToString(status)));
diff --git a/quiche/quic/qbone/qbone_session_base.cc b/quiche/quic/qbone/qbone_session_base.cc
index 31cf8e1..289794b 100644
--- a/quiche/quic/qbone/qbone_session_base.cc
+++ b/quiche/quic/qbone/qbone_session_base.cc
@@ -172,6 +172,9 @@
case MESSAGE_STATUS_BLOCKED:
QUIC_BUG(quic_bug_10987_5) << "MESSAGE_STATUS_BLOCKED";
break;
+ case MESSAGE_STATUS_SETTINGS_NOT_RECEIVED:
+ QUIC_BUG(quic_bug_10987_8) << "MESSAGE_STATUS_SETTINGS_NOT_RECEIVED";
+ break;
case MESSAGE_STATUS_INTERNAL_ERROR:
QUIC_BUG(quic_bug_10987_6) << "MESSAGE_STATUS_INTERNAL_ERROR";
break;
