Support Http3 extended CONNECT. When QuicSpdySession::allow_extended_connect_ is true, ENABLE_CONNECT_PROTOCOL will be 1 in SETTINGS frame. The client sets allow_extended_connect_ to true once the received SETTINGS frame has ENABLE_CONNECT_PROTOCOL = 1. QuicSpdySession::allow_extended_connect_ is behind --gfe2_reloadable_flag_quic_verify_request_headers. Validate request pseudo headers, especially CONNECT with :protocol header when allow_extended_connect_ is true. Make webtransport support depends on allow_extended_connect_. And on server side, allow_extended_connect_ is always true if ShouldNegotiateWebTransport() returns true. Protected by quic_reloadable_flag_quic_verify_request_headers. PiperOrigin-RevId: 402397389
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc index 54f6fa9..ba3c7a7 100644 --- a/quic/core/http/end_to_end_test.cc +++ b/quic/core/http/end_to_end_test.cc
@@ -136,7 +136,10 @@ // Since QuicSpdyStream uses QuicHeaderList::empty() to detect too large // headers, it also fails when receiving empty headers. SpdyHeaderBlock headers; - headers["foo"] = "bar"; + headers[":authority"] = "test.example.com:443"; + headers[":path"] = "/path"; + headers[":method"] = "GET"; + headers[":scheme"] = "https"; stream->WriteHeaders(std::move(headers), /* fin = */ false, nullptr); } @@ -6530,6 +6533,58 @@ })); } +TEST_P(EndToEndTest, InvalidExtendedConnect) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + ASSERT_TRUE(Initialize()); + + if (!version_.UsesHttp3()) { + return; + } + // Missing :path header. + spdy::SpdyHeaderBlock headers; + headers[":scheme"] = "https"; + headers[":authority"] = "localhost"; + headers[":method"] = "CONNECT"; + headers[":protocol"] = "webtransport"; + + client_->SendMessage(headers, "", /*fin=*/false); + client_->WaitForResponse(); + // An early response should be received. + CheckResponseHeaders("400"); +} + +TEST_P(EndToEndTest, RejectExtendedConnect) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + // Disable extended CONNECT. + memory_cache_backend_.set_enable_extended_connect(false); + ASSERT_TRUE(Initialize()); + + if (!version_.UsesHttp3()) { + return; + } + // This extended CONNECT should be rejected. + spdy::SpdyHeaderBlock headers; + headers[":scheme"] = "https"; + headers[":authority"] = "localhost"; + headers[":method"] = "CONNECT"; + headers[":path"] = "/echo"; + headers[":protocol"] = "webtransport"; + + client_->SendMessage(headers, "", /*fin=*/false); + client_->WaitForResponse(); + CheckResponseHeaders("400"); + + // Vanilla CONNECT should be accepted. + spdy::SpdyHeaderBlock headers2; + headers2[":authority"] = "localhost"; + headers2[":method"] = "CONNECT"; + + client_->SendMessage(headers2, "body", /*fin=*/true); + client_->WaitForResponse(); + // No :path header, so 404. + CheckResponseHeaders("404"); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/http/http_constants.cc b/quic/core/http/http_constants.cc index 851fd91..94d42fe 100644 --- a/quic/core/http/http_constants.cc +++ b/quic/core/http/http_constants.cc
@@ -20,6 +20,7 @@ RETURN_STRING_LITERAL(SETTINGS_H3_DATAGRAM_DRAFT00); RETURN_STRING_LITERAL(SETTINGS_H3_DATAGRAM_DRAFT04); RETURN_STRING_LITERAL(SETTINGS_WEBTRANS_DRAFT00); + RETURN_STRING_LITERAL(SETTINGS_ENABLE_CONNECT_PROTOCOL); } return absl::StrCat("UNSUPPORTED_SETTINGS_TYPE(", identifier, ")"); }
diff --git a/quic/core/http/http_constants.h b/quic/core/http/http_constants.h index ce03a1e..213b7d9 100644 --- a/quic/core/http/http_constants.h +++ b/quic/core/http/http_constants.h
@@ -44,6 +44,8 @@ SETTINGS_H3_DATAGRAM_DRAFT04 = 0xffd277, // draft-ietf-webtrans-http3-00 SETTINGS_WEBTRANS_DRAFT00 = 0x2b603742, + // draft-ietf-httpbis-h3-websockets + SETTINGS_ENABLE_CONNECT_PROTOCOL = 0x08, }; // Returns HTTP/3 SETTINGS identifier as a string.
diff --git a/quic/core/http/quic_send_control_stream_test.cc b/quic/core/http/quic_send_control_stream_test.cc index 5943585..7c71abe 100644 --- a/quic/core/http/quic_send_control_stream_test.cc +++ b/quic/core/http/quic_send_control_stream_test.cc
@@ -130,8 +130,10 @@ "4040" // 0x40 as the reserved frame type "01" // 1 byte frame length "61"); // payload "a" - if (QuicSpdySessionPeer::LocalHttpDatagramSupport(&session_) == - HttpDatagramSupport::kDraft00And04) { + if ((!GetQuicReloadableFlag(quic_verify_request_headers) || + perspective() == Perspective::IS_CLIENT) && + QuicSpdySessionPeer::LocalHttpDatagramSupport(&session_) == + HttpDatagramSupport::kDraft00And04) { expected_write_data = absl::HexStringToBytes( "00" // stream type: control stream "04" // frame type: SETTINGS frame @@ -152,6 +154,54 @@ "01" // 1 byte frame length "61"); // payload "a" } + if (GetQuicReloadableFlag(quic_verify_request_headers) && + perspective() == Perspective::IS_SERVER && + QuicSpdySessionPeer::LocalHttpDatagramSupport(&session_) == + HttpDatagramSupport::kNone) { + expected_write_data = absl::HexStringToBytes( + "00" // stream type: control stream + "04" // frame type: SETTINGS frame + "0d" // frame length + "01" // SETTINGS_QPACK_MAX_TABLE_CAPACITY + "40ff" // 255 + "06" // SETTINGS_MAX_HEADER_LIST_SIZE + "4400" // 1024 + "07" // SETTINGS_QPACK_BLOCKED_STREAMS + "10" // 16 + "08" // SETTINGS_ENABLE_CONNECT_PROTOCOL + "01" // 1 + "4040" // 0x40 as the reserved settings id + "14" // 20 + "4040" // 0x40 as the reserved frame type + "01" // 1 byte frame length + "61"); // payload "a" + } + if (GetQuicReloadableFlag(quic_verify_request_headers) && + perspective() == Perspective::IS_SERVER && + QuicSpdySessionPeer::LocalHttpDatagramSupport(&session_) != + HttpDatagramSupport::kNone) { + expected_write_data = absl::HexStringToBytes( + "00" // stream type: control stream + "04" // frame type: SETTINGS frame + "11" // frame length + "01" // SETTINGS_QPACK_MAX_TABLE_CAPACITY + "40ff" // 255 + "06" // SETTINGS_MAX_HEADER_LIST_SIZE + "4400" // 1024 + "07" // SETTINGS_QPACK_BLOCKED_STREAMS + "10" // 16 + "08" // SETTINGS_ENABLE_CONNECT_PROTOCOL + "01" // 1 + "4040" // 0x40 as the reserved settings id + "14" // 20 + "4276" // SETTINGS_H3_DATAGRAM_DRAFT00 + "01" // 1 + "800ffd277" // SETTINGS_H3_DATAGRAM_DRAFT04 + "01" // 1 + "4040" // 0x40 as the reserved frame type + "01" // 1 byte frame length + "61"); // payload "a" + } auto buffer = std::make_unique<char[]>(expected_write_data.size()); QuicDataWriter writer(expected_write_data.size(), buffer.get());
diff --git a/quic/core/http/quic_spdy_client_stream.cc b/quic/core/http/quic_spdy_client_stream.cc index 7391fdc..4ba3fc9 100644 --- a/quic/core/http/quic_spdy_client_stream.cc +++ b/quic/core/http/quic_spdy_client_stream.cc
@@ -13,6 +13,7 @@ #include "quic/core/http/web_transport_http3.h" #include "quic/core/quic_alarm.h" #include "quic/platform/api/quic_logging.h" +#include "common/quiche_text_utils.h" #include "spdy/core/spdy_protocol.h" using spdy::SpdyHeaderBlock; @@ -188,4 +189,25 @@ return bytes_sent; } +bool QuicSpdyClientStream::AreHeadersValid( + const QuicHeaderList& header_list) const { + if (!GetQuicReloadableFlag(quic_verify_request_headers)) { + return true; + } + if (!QuicSpdyStream::AreHeadersValid(header_list)) { + return false; + } + // Verify the presence of :status header. + bool saw_status = false; + for (const std::pair<std::string, std::string>& pair : header_list) { + if (pair.first == ":status") { + saw_status = true; + } else if (absl::StrContains(pair.first, ":")) { + QUIC_DLOG(ERROR) << "Unexpected ':' in header " << pair.first << "."; + return false; + } + } + return saw_status; +} + } // namespace quic
diff --git a/quic/core/http/quic_spdy_client_stream.h b/quic/core/http/quic_spdy_client_stream.h index 8637d11..7d420f5 100644 --- a/quic/core/http/quic_spdy_client_stream.h +++ b/quic/core/http/quic_spdy_client_stream.h
@@ -74,6 +74,9 @@ // of client-side streams should be able to set the priority. using QuicSpdyStream::SetPriority; + protected: + bool AreHeadersValid(const QuicHeaderList& header_list) const override; + private: // The parsed headers received from the server. spdy::SpdyHeaderBlock response_headers_;
diff --git a/quic/core/http/quic_spdy_client_stream_test.cc b/quic/core/http/quic_spdy_client_stream_test.cc index 9043d0b..11f81e5 100644 --- a/quic/core/http/quic_spdy_client_stream_test.cc +++ b/quic/core/http/quic_spdy_client_stream_test.cc
@@ -128,6 +128,30 @@ IsStreamError(QUIC_BAD_APPLICATION_PAYLOAD)); } +TEST_P(QuicSpdyClientStreamTest, InvalidResponseHeader) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + auto headers = AsHeaderList(std::vector<std::pair<std::string, std::string>>{ + {":status", "200"}, {":path", "/foo"}}); + EXPECT_CALL(*connection_, + OnStreamReset(stream_->id(), QUIC_BAD_APPLICATION_PAYLOAD)); + stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(), + headers); + EXPECT_THAT(stream_->stream_error(), + IsStreamError(QUIC_BAD_APPLICATION_PAYLOAD)); +} + +TEST_P(QuicSpdyClientStreamTest, MissingStatusCode) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + auto headers = AsHeaderList( + std::vector<std::pair<std::string, std::string>>{{"key", "value"}}); + EXPECT_CALL(*connection_, + OnStreamReset(stream_->id(), QUIC_BAD_APPLICATION_PAYLOAD)); + stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(), + headers); + EXPECT_THAT(stream_->stream_error(), + IsStreamError(QUIC_BAD_APPLICATION_PAYLOAD)); +} + TEST_P(QuicSpdyClientStreamTest, TestFraming) { auto headers = AsHeaderList(headers_); stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
diff --git a/quic/core/http/quic_spdy_server_stream_base.cc b/quic/core/http/quic_spdy_server_stream_base.cc index 93f1c2b..7431d86 100644 --- a/quic/core/http/quic_spdy_server_stream_base.cc +++ b/quic/core/http/quic_spdy_server_stream_base.cc
@@ -4,9 +4,13 @@ #include "quic/core/http/quic_spdy_server_stream_base.h" +#include "absl/strings/string_view.h" +#include "quic/core/http/quic_spdy_session.h" #include "quic/core/quic_error_codes.h" -#include "quic/core/quic_session.h" +#include "quic/platform/api/quic_flag_utils.h" +#include "quic/platform/api/quic_flags.h" #include "quic/platform/api/quic_logging.h" +#include "common/quiche_text_utils.h" namespace quic { @@ -44,4 +48,87 @@ QuicSpdyStream::StopReading(); } +bool QuicSpdyServerStreamBase::AreHeadersValid( + const QuicHeaderList& header_list) const { + if (!GetQuicReloadableFlag(quic_verify_request_headers)) { + return true; + } + QUIC_RELOADABLE_FLAG_COUNT_N(quic_verify_request_headers, 3, 3); + if (!QuicSpdyStream::AreHeadersValid(header_list)) { + return false; + } + + bool saw_connect = false; + bool saw_protocol = false; + bool saw_path = false; + bool saw_scheme = false; + bool saw_method = false; + bool saw_authority = false; + bool is_extended_connect = false; + // Check if it is missing any required headers and if there is any disallowed + // ones. + for (const std::pair<std::string, std::string>& pair : header_list) { + if (pair.first == ":method") { + saw_method = true; + if (pair.second == "CONNECT") { + saw_connect = true; + if (saw_protocol) { + is_extended_connect = true; + } + } + } else if (pair.first == ":protocol") { + saw_protocol = true; + if (saw_connect) { + is_extended_connect = true; + } + } else if (pair.first == ":scheme") { + saw_scheme = true; + } else if (pair.first == ":path") { + saw_path = true; + } else if (pair.first == ":authority") { + saw_authority = true; + } else if (absl::StrContains(pair.first, ":")) { + QUIC_DLOG(ERROR) << "Unexpected ':' in header " << pair.first << "."; + return false; + } + if (is_extended_connect) { + if (!spdy_session()->allow_extended_connect()) { + QUIC_DLOG(ERROR) + << "Received extended-CONNECT request while it is disabled."; + return false; + } + } else if (saw_method && !saw_connect) { + if (saw_protocol) { + QUIC_DLOG(ERROR) << "Receive non-CONNECT request with :protocol."; + return false; + } + } + } + + if (is_extended_connect) { + if (saw_scheme && saw_path && saw_authority) { + // Saw all the required pseudo headers. + return true; + } + QUIC_DLOG(ERROR) << "Missing required pseudo headers for extended-CONNECT."; + return false; + } + // This is a vanilla CONNECT or non-CONNECT request. + if (saw_connect) { + // Check vanilla CONNECT. + if (saw_path || saw_scheme) { + QUIC_DLOG(ERROR) + << "Received invalid CONNECT request with disallowed pseudo header."; + return false; + } + return true; + } + // Check non-CONNECT request. + if (saw_method && saw_authority && saw_path && saw_scheme) { + return true; + } + QUIC_LOG(ERROR) << "Missing required pseudo headers."; + return false; +} + } // namespace quic
diff --git a/quic/core/http/quic_spdy_server_stream_base.h b/quic/core/http/quic_spdy_server_stream_base.h index 21ecc0b..d10a6d4 100644 --- a/quic/core/http/quic_spdy_server_stream_base.h +++ b/quic/core/http/quic_spdy_server_stream_base.h
@@ -22,6 +22,9 @@ // when the stream has not received all the data. void CloseWriteSide() override; void StopReading() override; + + protected: + bool AreHeadersValid(const QuicHeaderList& header_list) const override; }; } // namespace quic
diff --git a/quic/core/http/quic_spdy_server_stream_base_test.cc b/quic/core/http/quic_spdy_server_stream_base_test.cc index aa41b3c..92c2d08 100644 --- a/quic/core/http/quic_spdy_server_stream_base_test.cc +++ b/quic/core/http/quic_spdy_server_stream_base_test.cc
@@ -6,7 +6,9 @@ #include "absl/memory/memory.h" #include "quic/core/crypto/null_encrypter.h" +#include "quic/platform/api/quic_flags.h" #include "quic/platform/api/quic_test.h" +#include "quic/test_tools/qpack/qpack_encoder_test_utils.h" #include "quic/test_tools/quic_spdy_session_peer.h" #include "quic/test_tools/quic_stream_peer.h" #include "quic/test_tools/quic_test_utils.h" @@ -97,6 +99,230 @@ EXPECT_TRUE(stream_->write_side_closed()); } +TEST_F(QuicSpdyServerStreamBaseTest, AllowExtendedConnect) { + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":method", "CONNECT"); + header_list.OnHeader(":protocol", "webtransport"); + header_list.OnHeader(":path", "/path"); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeaderBlockEnd(128, 128); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_EQ(GetQuicReloadableFlag(quic_verify_request_headers) && + !session_.allow_extended_connect(), + stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, AllowExtendedConnectProtocolFirst) { + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":protocol", "webtransport"); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":method", "CONNECT"); + header_list.OnHeader(":path", "/path"); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeaderBlockEnd(128, 128); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_EQ(GetQuicReloadableFlag(quic_verify_request_headers) && + !session_.allow_extended_connect(), + stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, InvalidExtendedConnect) { + if (!session_.version().UsesHttp3()) { + return; + } + SetQuicReloadableFlag(quic_verify_request_headers, true); + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":method", "CONNECT"); + header_list.OnHeader(":protocol", "webtransport"); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeaderBlockEnd(128, 128); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_TRUE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, VanillaConnectAllowed) { + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":method", "CONNECT"); + header_list.OnHeaderBlockEnd(128, 128); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_FALSE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, InvalidVanillaConnect) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":method", "CONNECT"); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeaderBlockEnd(128, 128); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_TRUE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, InvalidNonConnectWithProtocol) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":method", "GET"); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeader(":path", "/path"); + header_list.OnHeader(":protocol", "webtransport"); + header_list.OnHeaderBlockEnd(128, 128); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_TRUE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, InvalidRequestWithoutScheme) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + // A request without :scheme should be rejected. + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":method", "GET"); + header_list.OnHeader(":path", "/path"); + header_list.OnHeaderBlockEnd(128, 128); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_TRUE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, InvalidRequestWithoutAuthority) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + // A request without :authority should be rejected. + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeader(":method", "GET"); + header_list.OnHeader(":path", "/path"); + header_list.OnHeaderBlockEnd(128, 128); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_TRUE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, InvalidRequestWithoutMethod) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + // A request without :method should be rejected. + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeader(":path", "/path"); + header_list.OnHeaderBlockEnd(128, 128); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_TRUE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, InvalidRequestWithoutPath) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + // A request without :path should be rejected. + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeader(":method", "POST"); + header_list.OnHeaderBlockEnd(128, 128); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_TRUE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, InvalidRequestHeader) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + // A request without :path should be rejected. + QuicHeaderList header_list; + header_list.OnHeaderBlockStart(); + header_list.OnHeader(":authority", "www.google.com:4433"); + header_list.OnHeader(":scheme", "http"); + header_list.OnHeader(":method", "POST"); + header_list.OnHeader("invalid:header", "value"); + header_list.OnHeaderBlockEnd(128, 128); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamHeaderList(/*fin=*/false, 0, header_list); + EXPECT_TRUE(stream_->rst_sent()); +} + +TEST_F(QuicSpdyServerStreamBaseTest, EmptyHeaders) { + SetQuicReloadableFlag(quic_verify_request_headers, true); + spdy::SpdyHeaderBlock empty_header; + quic::test::NoopQpackStreamSenderDelegate encoder_stream_sender_delegate; + quic::test::NoopDecoderStreamErrorDelegate decoder_stream_error_delegate; + auto qpack_encoder = + std::make_unique<quic::QpackEncoder>(&decoder_stream_error_delegate); + qpack_encoder->set_qpack_stream_sender_delegate( + &encoder_stream_sender_delegate); + std::string payload = + qpack_encoder->EncodeHeaderList(stream_->id(), empty_header, nullptr); + std::unique_ptr<char[]> headers_buffer; + quic::QuicByteCount headers_frame_header_length = + quic::HttpEncoder::SerializeHeadersFrameHeader(payload.length(), + &headers_buffer); + absl::string_view headers_frame_header(headers_buffer.get(), + headers_frame_header_length); + + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_BAD_APPLICATION_PAYLOAD), + _)); + stream_->OnStreamFrame(QuicStreamFrame( + stream_->id(), true, 0, absl::StrCat(headers_frame_header, payload))); + EXPECT_TRUE(stream_->rst_sent()); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index 0cd2644..929e369 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -453,7 +453,11 @@ spdy_framer_(SpdyFramer::ENABLE_COMPRESSION), spdy_framer_visitor_(new SpdyFramerVisitor(this)), debug_visitor_(nullptr), - destruction_indicator_(123456789) { + destruction_indicator_(123456789), + allow_extended_connect_( + GetQuicReloadableFlag(quic_verify_request_headers) && + perspective() == Perspective::IS_SERVER && + VersionUsesHttp3(transport_version())) { 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()); @@ -522,6 +526,10 @@ if (WillNegotiateWebTransport()) { settings_.values[SETTINGS_WEBTRANS_DRAFT00] = 1; } + if (allow_extended_connect()) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_verify_request_headers, 1, 3); + settings_.values[SETTINGS_ENABLE_CONNECT_PROTOCOL] = 1; + } } void QuicSpdySession::OnDecoderStreamError(QuicErrorCode error_code, @@ -1034,9 +1042,7 @@ H3SettingsToString(static_cast<Http3AndQpackSettingsIdentifiers>(id)), " with invalid value ", value); QUIC_PEER_BUG(bad received setting) << ENDPOINT << error_details; - // TODO(dschinazi) use QUIC_HTTP_INVALID_SETTING_VALUE instead of - // QUIC_HTTP_RECEIVE_SPDY_SETTING once cl/396439351 lands. - CloseConnectionWithDetails(QUIC_HTTP_RECEIVE_SPDY_SETTING, error_details); + CloseConnectionWithDetails(QUIC_HTTP_INVALID_SETTING_VALUE, error_details); return false; } @@ -1112,6 +1118,18 @@ } break; } + case SETTINGS_ENABLE_CONNECT_PROTOCOL: { + QUIC_DVLOG(1) << ENDPOINT + << "SETTINGS_ENABLE_CONNECT_PROTOCOL received with value " + << value; + if (!VerifySettingIsZeroOrOne(id, value)) { + return false; + } + if (perspective() == Perspective::IS_CLIENT) { + allow_extended_connect_ = value != 0; + } + break; + } case spdy::SETTINGS_ENABLE_PUSH: ABSL_FALLTHROUGH_INTENDED; case spdy::SETTINGS_MAX_CONCURRENT_STREAMS: @@ -1735,7 +1753,9 @@ bool QuicSpdySession::SupportsWebTransport() { return WillNegotiateWebTransport() && SupportsH3Datagram() && - peer_supports_webtransport_; + peer_supports_webtransport_ && + (!GetQuicReloadableFlag(quic_verify_request_headers) || + allow_extended_connect_); } bool QuicSpdySession::SupportsH3Datagram() const { @@ -1893,6 +1913,25 @@ return os; } +// Must not be called after Initialize(). +void QuicSpdySession::set_allow_extended_connect(bool allow_extended_connect) { + QUIC_BUG_IF(extended connect wrong version, + !GetQuicReloadableFlag(quic_verify_request_headers) || + !VersionUsesHttp3(transport_version())) + << "Try to enable/disable extended CONNECT in Google QUIC"; + QUIC_BUG_IF(extended connect on client, + !GetQuicReloadableFlag(quic_verify_request_headers) || + perspective() == Perspective::IS_CLIENT) + << "Enabling/disabling extended CONNECT on the client side has no effect"; + if (ShouldNegotiateWebTransport()) { + QUIC_BUG_IF(disable extended connect, !allow_extended_connect) + << "Disabling extended CONNECT with web transport enabled has no " + "effect."; + return; + } + allow_extended_connect_ = allow_extended_connect; +} + #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 d8503cc..b84f5a1 100644 --- a/quic/core/http/quic_spdy_session.h +++ b/quic/core/http/quic_spdy_session.h
@@ -283,12 +283,16 @@ qpack_maximum_blocked_streams_ = qpack_maximum_blocked_streams; } + // Should only be used by IETF QUIC server side. // Must not be called after Initialize(). // TODO(bnc): Move to constructor argument. void set_max_inbound_header_list_size(size_t max_inbound_header_list_size) { max_inbound_header_list_size_ = max_inbound_header_list_size; } + // Must not be called after Initialize(). + void set_allow_extended_connect(bool allow_extended_connect); + size_t max_outbound_header_list_size() const { return max_outbound_header_list_size_; } @@ -297,6 +301,8 @@ return max_inbound_header_list_size_; } + bool allow_extended_connect() const { return allow_extended_connect_; } + // Returns true if the session has active request streams. bool HasActiveRequestStreams() const; @@ -684,6 +690,10 @@ // Limited to kMaxUnassociatedWebTransportStreams; when the list is full, // oldest streams are evicated first. std::list<BufferedWebTransportStream> buffered_streams_; + + // On the server side, if true, advertise and accept extended CONNECT method. + // On the client side, true if the peer advertised extended CONNECT. + bool allow_extended_connect_; }; } // namespace quic
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 5c3e525..23611b3 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -238,7 +238,6 @@ CurrentSupportedVersions()), crypto_stream_(this), writev_consumes_all_data_(false) { - Initialize(); this->connection()->SetEncrypter( ENCRYPTION_FORWARD_SECURE, std::make_unique<NullEncrypter>(connection->perspective())); @@ -387,11 +386,18 @@ } protected: - explicit QuicSpdySessionTestBase(Perspective perspective) + explicit QuicSpdySessionTestBase(Perspective perspective, + bool allow_extended_connect) : connection_(new StrictMock<MockQuicConnection>( &helper_, &alarm_factory_, perspective, SupportedVersions(GetParam()))), session_(connection_) { + if (perspective == Perspective::IS_SERVER && + VersionUsesHttp3(transport_version()) && + GetQuicReloadableFlag(quic_verify_request_headers)) { + session_.set_allow_extended_connect(allow_extended_connect); + } + session_.Initialize(); session_.config()->SetInitialStreamFlowControlWindowToSend( kInitialStreamFlowControlWindowForTest); session_.config()->SetInitialSessionFlowControlWindowToSend( @@ -547,6 +553,7 @@ SettingsFrame settings; settings.values[SETTINGS_H3_DATAGRAM_DRAFT04] = 1; settings.values[SETTINGS_WEBTRANS_DRAFT00] = 1; + settings.values[SETTINGS_ENABLE_CONNECT_PROTOCOL] = 1; std::string data = std::string(1, kControlStream) + EncodeSettings(settings); QuicStreamId control_stream_id = @@ -612,7 +619,7 @@ class QuicSpdySessionTestServer : public QuicSpdySessionTestBase { protected: QuicSpdySessionTestServer() - : QuicSpdySessionTestBase(Perspective::IS_SERVER) {} + : QuicSpdySessionTestBase(Perspective::IS_SERVER, true) {} }; INSTANTIATE_TEST_SUITE_P(Tests, QuicSpdySessionTestServer, @@ -1858,7 +1865,7 @@ class QuicSpdySessionTestClient : public QuicSpdySessionTestBase { protected: QuicSpdySessionTestClient() - : QuicSpdySessionTestBase(Perspective::IS_CLIENT) {} + : QuicSpdySessionTestBase(Perspective::IS_CLIENT, false) {} }; INSTANTIATE_TEST_SUITE_P(Tests, QuicSpdySessionTestClient, @@ -3550,17 +3557,10 @@ session_.set_debug_visitor(&debug_visitor); CompleteHandshake(); - SettingsFrame server_settings; - server_settings.values[SETTINGS_H3_DATAGRAM_DRAFT04] = 1; - server_settings.values[SETTINGS_WEBTRANS_DRAFT00] = 1; - std::string data = - std::string(1, kControlStream) + EncodeSettings(server_settings); - QuicStreamId stream_id = - GetNthServerInitiatedUnidirectionalStreamId(transport_version(), 3); - QuicStreamFrame frame(stream_id, /*fin=*/false, /*offset=*/0, data); - EXPECT_CALL(debug_visitor, OnPeerControlStreamCreated(stream_id)); - EXPECT_CALL(debug_visitor, OnSettingsFrameReceived(server_settings)); - session_.OnStreamFrame(frame); + EXPECT_CALL(debug_visitor, OnPeerControlStreamCreated(_)); + EXPECT_CALL(debug_visitor, OnSettingsFrameReceived(_)); + ReceiveWebTransportSettings(); + EXPECT_TRUE(session_.ShouldProcessIncomingRequests()); EXPECT_TRUE(session_.SupportsWebTransport()); } @@ -3720,6 +3720,63 @@ EXPECT_EQ(web_transport->NumberOfAssociatedStreams(), 0u); } +class QuicSpdySessionTestServerNoExtendedConnect + : public QuicSpdySessionTestBase { + public: + QuicSpdySessionTestServerNoExtendedConnect() + : QuicSpdySessionTestBase(Perspective::IS_SERVER, false) {} +}; + +INSTANTIATE_TEST_SUITE_P(Tests, QuicSpdySessionTestServerNoExtendedConnect, + ::testing::ValuesIn(AllSupportedVersions()), + ::testing::PrintToStringParamName()); + +// Tests that receiving SETTINGS_ENABLE_CONNECT_PROTOCOL = 1 doesn't enable +// server session to support extended CONNECT. +TEST_P(QuicSpdySessionTestServerNoExtendedConnect, + WebTransportSettingNoEffect) { + if (!version().UsesHttp3()) { + return; + } + + EXPECT_FALSE(session_.SupportsWebTransport()); + EXPECT_TRUE(session_.ShouldProcessIncomingRequests()); + + CompleteHandshake(); + + ReceiveWebTransportSettings(); + EXPECT_FALSE(session_.allow_extended_connect()); + EXPECT_FALSE(session_.SupportsWebTransport()); + EXPECT_TRUE(session_.ShouldProcessIncomingRequests()); +} + +TEST_P(QuicSpdySessionTestServerNoExtendedConnect, BadExtendedConnectSetting) { + if (!version().UsesHttp3()) { + return; + } + SetQuicReloadableFlag(quic_verify_request_headers, true); + + EXPECT_FALSE(session_.SupportsWebTransport()); + EXPECT_TRUE(session_.ShouldProcessIncomingRequests()); + + CompleteHandshake(); + + // ENABLE_CONNECT_PROTOCOL setting value has to be 1 or 0; + SettingsFrame settings; + settings.values[SETTINGS_ENABLE_CONNECT_PROTOCOL] = 2; + std::string data = std::string(1, kControlStream) + EncodeSettings(settings); + QuicStreamId control_stream_id = + session_.perspective() == Perspective::IS_SERVER + ? GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3) + : GetNthServerInitiatedUnidirectionalStreamId(transport_version(), 3); + QuicStreamFrame frame(control_stream_id, /*fin=*/false, /*offset=*/0, data); + EXPECT_CALL(*connection_, + CloseConnection(QUIC_HTTP_INVALID_SETTING_VALUE, _, _)); + EXPECT_QUIC_PEER_BUG( + session_.OnStreamFrame(frame), + "Received SETTINGS_ENABLE_CONNECT_PROTOCOL with invalid value"); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc index c25d33a..284a180 100644 --- a/quic/core/http/quic_spdy_stream.cc +++ b/quic/core/http/quic_spdy_stream.cc
@@ -606,8 +606,22 @@ // TODO(b/134706391): remove |fin| argument. headers_decompressed_ = true; header_list_ = header_list; + bool header_too_large = VersionUsesHttp3(transport_version()) + ? header_list_size_limit_exceeded_ + : header_list.empty(); + // Validate request headers if it did not exceed size limit. If it did, + // OnHeadersTooLarge() should have already handled it previously. + if (!header_too_large && !AreHeadersValid(header_list)) { + QUIC_CODE_COUNT_N(quic_validate_request_header, 1, 2); + OnInvalidHeaders(); + return; + } + QUIC_CODE_COUNT_N(quic_validate_request_header, 2, 2); - MaybeProcessReceivedWebTransportHeaders(); + if (!GetQuicReloadableFlag(quic_verify_request_headers) || + !header_too_large) { + MaybeProcessReceivedWebTransportHeaders(); + } if (VersionUsesHttp3(transport_version())) { if (fin) { @@ -1760,5 +1774,14 @@ } } +bool QuicSpdyStream::AreHeadersValid( + const QuicHeaderList& /*header_list*/) const { + // TODO(b/202433856) check each header name to be valid token and isn't a + // disallowed header. + return true; +} + +void QuicSpdyStream::OnInvalidHeaders() { Reset(QUIC_BAD_APPLICATION_PAYLOAD); } + #undef ENDPOINT // undef for jumbo builds } // namespace quic
diff --git a/quic/core/http/quic_spdy_stream.h b/quic/core/http/quic_spdy_stream.h index 7ea2d1d..cd9d99b 100644 --- a/quic/core/http/quic_spdy_stream.h +++ b/quic/core/http/quic_spdy_stream.h
@@ -371,6 +371,11 @@ void OnWriteSideInDataRecvdState() override; + virtual bool AreHeadersValid(const QuicHeaderList& header_list) const; + + // Reset stream upon invalid request headers. + virtual void OnInvalidHeaders(); + private: friend class test::QuicSpdyStreamPeer; friend class test::QuicStreamPeer;
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index 48b654b..b5905e7 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -3047,6 +3047,7 @@ InitializeWithPerspective(kShouldProcessData, Perspective::IS_CLIENT); session_->set_local_http_datagram_support(HttpDatagramSupport::kDraft00And04); session_->EnableWebTransport(); + session_->OnSetting(SETTINGS_ENABLE_CONNECT_PROTOCOL, 1); QuicSpdySessionPeer::EnableWebTransport(session_.get()); QuicSpdySessionPeer::SetHttpDatagramSupport(session_.get(), HttpDatagramSupport::kDraft00); @@ -3072,6 +3073,7 @@ InitializeWithPerspective(kShouldProcessData, Perspective::IS_CLIENT); session_->set_local_http_datagram_support(HttpDatagramSupport::kDraft00And04); session_->EnableWebTransport(); + session_->OnSetting(SETTINGS_ENABLE_CONNECT_PROTOCOL, 1); QuicSpdySessionPeer::EnableWebTransport(session_.get()); QuicSpdySessionPeer::SetHttpDatagramSupport(session_.get(), HttpDatagramSupport::kDraft04);
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc index 162cb04..6ecc76b 100644 --- a/quic/core/quic_error_codes.cc +++ b/quic/core/quic_error_codes.cc
@@ -239,6 +239,7 @@ RETURN_STRING_LITERAL(QUIC_HTTP_RECEIVE_SPDY_SETTING); RETURN_STRING_LITERAL(QUIC_HTTP_RECEIVE_SPDY_FRAME); RETURN_STRING_LITERAL(QUIC_HTTP_RECEIVE_SERVER_PUSH); + RETURN_STRING_LITERAL(QUIC_HTTP_INVALID_SETTING_VALUE); RETURN_STRING_LITERAL(QUIC_HPACK_INDEX_VARINT_ERROR); RETURN_STRING_LITERAL(QUIC_HPACK_NAME_LENGTH_VARINT_ERROR); RETURN_STRING_LITERAL(QUIC_HPACK_VALUE_LENGTH_VARINT_ERROR); @@ -694,6 +695,8 @@ return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::ID_ERROR)}; case QUIC_HTTP_RECEIVE_SPDY_SETTING: return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::SETTINGS_ERROR)}; + case QUIC_HTTP_INVALID_SETTING_VALUE: + return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::SETTINGS_ERROR)}; case QUIC_HTTP_RECEIVE_SPDY_FRAME: return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::FRAME_UNEXPECTED)};
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h index 860f107..7e4fd09 100644 --- a/quic/core/quic_error_codes.h +++ b/quic/core/quic_error_codes.h
@@ -513,6 +513,8 @@ // HTTP/3 session received SERVER_PUSH stream, which is an error because // PUSH_PROMISE is not accepted. QUIC_HTTP_RECEIVE_SERVER_PUSH = 205, + // HTTP/3 session received invalid SETTING value. + QUIC_HTTP_INVALID_SETTING_VALUE = 207, // HPACK header block decoding errors. // Index varint beyond implementation limit. @@ -603,7 +605,7 @@ QUIC_INVALID_CHARACTER_IN_FIELD_VALUE = 206, // No error. Used as bound while iterating. - QUIC_LAST_ERROR = 207, + QUIC_LAST_ERROR = 208, }; // QuicErrorCodes is encoded as four octets on-the-wire when doing Google QUIC, // or a varint62 when doing IETF QUIC. Ensure that its value does not exceed
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 4331652..994bbdd 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -85,6 +85,8 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_pass_path_response_to_validator, true) // If true, quic dispatcher supports one connection to use multiple connection IDs. QUIC_FLAG(FLAGS_quic_restart_flag_quic_dispatcher_support_multiple_cid_per_connection_v2, true) +// If true, quic server will send ENABLE_CONNECT_PROTOCOL setting and and endpoint will validate required request/response headers and extended CONNECT mechanism. +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_verify_request_headers, false) // If true, record addresses that server has sent reset to recently, and do not send reset if the address lives in the set. QUIC_FLAG(FLAGS_quic_restart_flag_quic_use_recent_reset_addresses, true) // If true, refactor how QUIC TLS server disables resumption. No behavior change.
diff --git a/quic/test_tools/quic_test_backend.h b/quic/test_tools/quic_test_backend.h index 66e1213..6aa175e 100644 --- a/quic/test_tools/quic_test_backend.h +++ b/quic/test_tools/quic_test_backend.h
@@ -6,6 +6,7 @@ #define QUICHE_QUIC_TEST_TOOLS_QUIC_TEST_BACKEND_H_ #include "quic/tools/quic_memory_cache_backend.h" +#include "common/platform/api/quiche_logging.h" namespace quic { namespace test { @@ -21,11 +22,19 @@ bool SupportsWebTransport() override { return enable_webtransport_; } void set_enable_webtransport(bool enable_webtransport) { + QUICHE_DCHECK(!enable_webtransport || enable_extended_connect_); enable_webtransport_ = enable_webtransport; } + bool SupportsExtendedConnect() override { return enable_extended_connect_; } + + void set_enable_extended_connect(bool enable_extended_connect) { + enable_extended_connect_ = enable_extended_connect; + } + private: bool enable_webtransport_ = false; + bool enable_extended_connect_ = true; }; } // namespace test
diff --git a/quic/test_tools/quic_test_server.cc b/quic/test_tools/quic_test_server.cc index f6e2c34..020a34c 100644 --- a/quic/test_tools/quic_test_server.cc +++ b/quic/test_tools/quic_test_server.cc
@@ -94,25 +94,24 @@ crypto_stream_factory_(nullptr) {} std::unique_ptr<QuicSession> CreateQuicSession( - QuicConnectionId id, - const QuicSocketAddress& self_address, - const QuicSocketAddress& peer_address, - absl::string_view alpn, - const ParsedQuicVersion& version, - absl::string_view sni) override { + QuicConnectionId id, const QuicSocketAddress& self_address, + const QuicSocketAddress& peer_address, absl::string_view /*alpn*/, + const ParsedQuicVersion& version, absl::string_view /*sni*/) override { QuicReaderMutexLock lock(&factory_lock_); - if (session_factory_ == nullptr && stream_factory_ == nullptr && - crypto_stream_factory_ == nullptr) { - return QuicSimpleDispatcher::CreateQuicSession( - id, self_address, peer_address, alpn, version, sni); - } + // The QuicServerSessionBase takes ownership of |connection| below. QuicConnection* connection = new QuicConnection( id, self_address, peer_address, helper(), alarm_factory(), writer(), /* owns_writer= */ false, Perspective::IS_SERVER, ParsedQuicVersionVector{version}); std::unique_ptr<QuicServerSessionBase> session; - if (stream_factory_ != nullptr || crypto_stream_factory_ != nullptr) { + if (session_factory_ == nullptr && stream_factory_ == nullptr && + crypto_stream_factory_ == nullptr) { + session = std::make_unique<QuicSimpleServerSession>( + config(), GetSupportedVersions(), connection, this, session_helper(), + crypto_config(), compressed_certs_cache(), server_backend()); + } else if (stream_factory_ != nullptr || + crypto_stream_factory_ != nullptr) { session = std::make_unique<CustomStreamSession>( config(), GetSupportedVersions(), connection, this, session_helper(), crypto_config(), compressed_certs_cache(), stream_factory_, @@ -122,6 +121,14 @@ config(), connection, this, session_helper(), crypto_config(), compressed_certs_cache(), server_backend()); } + if (VersionUsesHttp3(version.transport_version) && + GetQuicReloadableFlag(quic_verify_request_headers)) { + QUICHE_DCHECK(session->allow_extended_connect()); + // Do not allow extended CONNECT request if the backend doesn't support + // it. + session->set_allow_extended_connect( + server_backend()->SupportsExtendedConnect()); + } session->Initialize(); return session; }
diff --git a/quic/tools/quic_simple_server_backend.h b/quic/tools/quic_simple_server_backend.h index 6411567..67aeac9 100644 --- a/quic/tools/quic_simple_server_backend.h +++ b/quic/tools/quic_simple_server_backend.h
@@ -67,6 +67,7 @@ return response; } virtual bool SupportsWebTransport() { return false; } + virtual bool SupportsExtendedConnect() { return true; } }; } // namespace quic
diff --git a/quic/tools/quic_simple_server_stream.cc b/quic/tools/quic_simple_server_stream.cc index d7e2a70..84e7071 100644 --- a/quic/tools/quic_simple_server_stream.cc +++ b/quic/tools/quic_simple_server_stream.cc
@@ -59,7 +59,11 @@ if (!SpdyUtils::CopyAndValidateHeaders(header_list, &content_length_, &request_headers_)) { QUIC_DVLOG(1) << "Invalid headers"; - SendErrorResponse(); + if (!response_sent_) { + // QuicSpdyStream::OnInitialHeadersComplete() may have already sent error + // response. + SendErrorResponse(); + } } ConsumeHeaderList(); if (!fin && !response_sent_) { @@ -344,6 +348,9 @@ void QuicSimpleServerStream::SendErrorResponse(int resp_code) { QUIC_DVLOG(1) << "Stream " << id() << " sending error response."; + if (!reading_stopped()) { + StopReading(); + } Http2HeaderBlock headers; if (resp_code <= 0) { headers[":status"] = "500"; @@ -411,6 +418,11 @@ WriteTrailers(std::move(response_trailers), nullptr); } +void QuicSimpleServerStream::OnInvalidHeaders() { + QUIC_DVLOG(1) << "Invalid headers"; + SendErrorResponse(400); +} + const char* const QuicSimpleServerStream::kErrorResponseBody = "bad"; const char* const QuicSimpleServerStream::kNotFoundResponseBody = "file not found";
diff --git a/quic/tools/quic_simple_server_stream.h b/quic/tools/quic_simple_server_stream.h index d138492..cd04bd7 100644 --- a/quic/tools/quic_simple_server_stream.h +++ b/quic/tools/quic_simple_server_stream.h
@@ -43,6 +43,8 @@ // data (or a FIN) to be read. void OnBodyAvailable() override; + void OnInvalidHeaders() override; + // Make this stream start from as if it just finished parsing an incoming // request whose headers are equivalent to |push_request_headers|. // Doing so will trigger this toy stream to fetch response and send it back. @@ -66,7 +68,7 @@ // Sends a basic 500 response using SendHeaders for the headers and WriteData // for the body. virtual void SendErrorResponse(); - void SendErrorResponse(int resp_code); + virtual void SendErrorResponse(int resp_code); // Sends a basic 404 response using SendHeaders for the headers and WriteData // for the body.
diff --git a/quic/tools/quic_simple_server_stream_test.cc b/quic/tools/quic_simple_server_stream_test.cc index aa07703..b75246f 100644 --- a/quic/tools/quic_simple_server_stream_test.cc +++ b/quic/tools/quic_simple_server_stream_test.cc
@@ -71,7 +71,7 @@ // Expose protected QuicSimpleServerStream methods. void DoSendResponse() { SendResponse(); } - void DoSendErrorResponse() { SendErrorResponse(); } + void DoSendErrorResponse() { QuicSimpleServerStream::SendErrorResponse(); } spdy::Http2HeaderBlock* mutable_headers() { return &request_headers_; } void set_body(std::string body) { body_ = std::move(body); } @@ -94,9 +94,9 @@ QuicSimpleServerStream::SendResponse(); } - void SendErrorResponse() override { + void SendErrorResponse(int resp_code) override { send_error_response_was_called_ = true; - QuicSimpleServerStream::SendErrorResponse(); + QuicSimpleServerStream::SendErrorResponse(resp_code); } private: @@ -208,6 +208,7 @@ header_list_.OnHeader(":authority", "www.google.com"); header_list_.OnHeader(":path", "/"); header_list_.OnHeader(":method", "POST"); + header_list_.OnHeader(":scheme", "https"); header_list_.OnHeader("content-length", "11"); header_list_.OnHeaderBlockEnd(128, 128); @@ -737,7 +738,7 @@ QuicHeaderList header_list; header_list.OnHeaderBlockStart(); header_list.OnHeader(":authority", "www.google.com:4433"); - header_list.OnHeader(":method", "CONNECT-SILLY"); + header_list.OnHeader(":method", "CONNECT"); header_list.OnHeaderBlockEnd(128, 128); EXPECT_CALL(*stream_, WriteHeadersMock(/*fin=*/false)); stream_->OnStreamHeaderList(/*fin=*/false, kFakeFrameLen, header_list); @@ -747,7 +748,7 @@ UsesHttp3() ? absl::StrCat(header.AsStringView(), body_) : body_; stream_->OnStreamFrame( QuicStreamFrame(stream_->id(), /*fin=*/false, /*offset=*/0, data)); - EXPECT_EQ("CONNECT-SILLY", StreamHeadersValue(":method")); + EXPECT_EQ("CONNECT", StreamHeadersValue(":method")); EXPECT_EQ(body_, StreamBody()); EXPECT_TRUE(stream_->send_response_was_called()); EXPECT_FALSE(stream_->send_error_response_was_called()); @@ -760,20 +761,25 @@ QuicHeaderList header_list; header_list.OnHeaderBlockStart(); header_list.OnHeader(":authority", "www.google.com:4433"); - header_list.OnHeader(":method", "CONNECT-SILLY"); + header_list.OnHeader(":method", "CONNECT"); // QUIC requires lower-case header names. header_list.OnHeader("InVaLiD-HeAdEr", "Well that's just wrong!"); header_list.OnHeaderBlockEnd(128, 128); + + if (UsesHttp3()) { + EXPECT_CALL(session_, + MaybeSendStopSendingFrame(_, QuicResetStreamError::FromInternal( + QUIC_STREAM_NO_ERROR))) + .Times(1); + } else { + EXPECT_CALL( + session_, + MaybeSendRstStreamFrame( + _, QuicResetStreamError::FromInternal(QUIC_STREAM_NO_ERROR), _)) + .Times(1); + } EXPECT_CALL(*stream_, WriteHeadersMock(/*fin=*/false)); stream_->OnStreamHeaderList(/*fin=*/false, kFakeFrameLen, header_list); - QuicBuffer header = HttpEncoder::SerializeDataFrameHeader( - body_.length(), SimpleBufferAllocator::Get()); - std::string data = - UsesHttp3() ? absl::StrCat(header.AsStringView(), body_) : body_; - stream_->OnStreamFrame( - QuicStreamFrame(stream_->id(), /*fin=*/false, /*offset=*/0, data)); - EXPECT_EQ("CONNECT-SILLY", StreamHeadersValue(":method")); - EXPECT_EQ(body_, StreamBody()); EXPECT_FALSE(stream_->send_response_was_called()); EXPECT_TRUE(stream_->send_error_response_was_called()); }