Supports extended CONNECT behavior in `OgHttp2Session`. This fixes the Envoy `websocket_integration_test` and `tcp_tunneling_integration_test` test cases. http://sponge2/59408995-0532-4766-8561-bdb3c680061b PiperOrigin-RevId: 422815076
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc index 57afa54..d256156 100644 --- a/http2/adapter/header_validator.cc +++ b/http2/adapter/header_validator.cc
@@ -23,6 +23,10 @@ bool ValidateRequestHeaders(const std::vector<std::string>& pseudo_headers, absl::string_view method, bool allow_connect) { + QUICHE_VLOG(2) << "Request pseudo-headers: [" + << absl::StrJoin(pseudo_headers, ", ") + << "], allow_connect: " << allow_connect + << ", method: " << method; if (allow_connect && method == "CONNECT") { static const std::vector<std::string>* kConnectHeaders = new std::vector<std::string>(
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc index 9b2809b..3203560 100644 --- a/http2/adapter/oghttp2_adapter_test.cc +++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -62,14 +62,16 @@ TestFrameSequence().ClientPreface().Ping(17).Serialize()); } -TEST_F(OgHttp2AdapterTest, InitialSettings) { +TEST_F(OgHttp2AdapterTest, InitialSettingsNoExtendedConnect) { DataSavingVisitor client_visitor; OgHttp2Adapter::Options client_options{.perspective = Perspective::kClient, - .max_header_list_bytes = 42}; + .max_header_list_bytes = 42, + .allow_extended_connect = false}; auto client_adapter = OgHttp2Adapter::Create(client_visitor, client_options); DataSavingVisitor server_visitor; - OgHttp2Adapter::Options server_options{.perspective = Perspective::kServer}; + OgHttp2Adapter::Options server_options{.perspective = Perspective::kServer, + .allow_extended_connect = false}; auto server_adapter = OgHttp2Adapter::Create(server_visitor, server_options); testing::InSequence s; @@ -122,6 +124,72 @@ } } +TEST_F(OgHttp2AdapterTest, InitialSettings) { + DataSavingVisitor client_visitor; + OgHttp2Adapter::Options client_options{.perspective = Perspective::kClient, + .max_header_list_bytes = 42}; + ASSERT_TRUE(client_options.allow_extended_connect); + auto client_adapter = OgHttp2Adapter::Create(client_visitor, client_options); + + DataSavingVisitor server_visitor; + OgHttp2Adapter::Options server_options{.perspective = Perspective::kServer}; + ASSERT_TRUE(server_options.allow_extended_connect); + auto server_adapter = OgHttp2Adapter::Create(server_visitor, server_options); + + testing::InSequence s; + + // Client sends the connection preface, including the initial SETTINGS. + EXPECT_CALL(client_visitor, OnBeforeFrameSent(SETTINGS, 0, 12, 0x0)); + EXPECT_CALL(client_visitor, OnFrameSent(SETTINGS, 0, 12, 0x0, 0)); + { + int result = client_adapter->Send(); + EXPECT_EQ(0, result); + absl::string_view data = client_visitor.data(); + EXPECT_THAT(data, testing::StartsWith(spdy::kHttp2ConnectionHeaderPrefix)); + data.remove_prefix(strlen(spdy::kHttp2ConnectionHeaderPrefix)); + EXPECT_THAT(data, EqualsFrames({spdy::SpdyFrameType::SETTINGS})); + } + + // Server sends the connection preface, including the initial SETTINGS. + EXPECT_CALL(server_visitor, OnBeforeFrameSent(SETTINGS, 0, 6, 0x0)); + EXPECT_CALL(server_visitor, OnFrameSent(SETTINGS, 0, 6, 0x0, 0)); + { + int result = server_adapter->Send(); + EXPECT_EQ(0, result); + absl::string_view data = server_visitor.data(); + EXPECT_THAT(data, EqualsFrames({spdy::SpdyFrameType::SETTINGS})); + } + + // Client processes the server's initial bytes, including initial SETTINGS. + EXPECT_CALL(client_visitor, OnFrameHeader(0, 6, SETTINGS, 0x0)); + EXPECT_CALL(client_visitor, OnSettingsStart()); + EXPECT_CALL(client_visitor, + OnSetting(Http2Setting{ + Http2KnownSettingsId::ENABLE_CONNECT_PROTOCOL, 1u})) + .Times(2); + EXPECT_CALL(client_visitor, OnSettingsEnd()); + { + const int64_t result = client_adapter->ProcessBytes(server_visitor.data()); + EXPECT_EQ(server_visitor.data().size(), static_cast<size_t>(result)); + } + + // Server processes the client's initial bytes, including initial SETTINGS. + EXPECT_CALL(server_visitor, OnFrameHeader(0, 12, SETTINGS, 0x0)); + EXPECT_CALL(server_visitor, OnSettingsStart()); + EXPECT_CALL(server_visitor, + OnSetting(Http2Setting{Http2KnownSettingsId::ENABLE_PUSH, 0u})) + .Times(2); + EXPECT_CALL( + server_visitor, + OnSetting(Http2Setting{Http2KnownSettingsId::MAX_HEADER_LIST_SIZE, 42u})) + .Times(2); + EXPECT_CALL(server_visitor, OnSettingsEnd()); + { + const int64_t result = server_adapter->ProcessBytes(client_visitor.data()); + EXPECT_EQ(client_visitor.data().size(), static_cast<size_t>(result)); + } +} + TEST_F(OgHttp2AdapterTest, AutomaticSettingsAndPingAcks) { const std::string frames = TestFrameSequence().ClientPreface().Ping(42).Serialize(); @@ -3983,7 +4051,8 @@ TEST(OgHttp2AdapterServerTest, ServerForbidsProtocolPseudoheaderBeforeAck) { DataSavingVisitor visitor; - OgHttp2Adapter::Options options{.perspective = Perspective::kServer}; + OgHttp2Adapter::Options options{.perspective = Perspective::kServer, + .allow_extended_connect = false}; auto adapter = OgHttp2Adapter::Create(visitor, options); const std::string initial_frames = @@ -4061,27 +4130,19 @@ /*fin=*/true) .Serialize(); + // After sending SETTINGS with `ENABLE_CONNECT_PROTOCOL`, oghttp2 matches + // nghttp2 in allowing this, even though the `allow_extended_connect` option + // is false. EXPECT_CALL(visitor, OnFrameHeader(3, _, HEADERS, 0x5)); EXPECT_CALL(visitor, OnBeginHeadersForStream(3)); EXPECT_CALL(visitor, OnHeaderForStream(3, _, _)).Times(5); - EXPECT_CALL(visitor, - OnInvalidFrame( - 3, Http2VisitorInterface::InvalidFrameError::kHttpMessaging)); + EXPECT_CALL(visitor, OnEndHeadersForStream(3)); + EXPECT_CALL(visitor, OnEndStream(3)); stream_result = adapter->ProcessBytes(stream3_frames); EXPECT_EQ(static_cast<size_t>(stream_result), stream3_frames.size()); - // The server sends a RST_STREAM for the offending stream. - EXPECT_TRUE(adapter->want_write()); - EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 3, _, 0x0)); - EXPECT_CALL(visitor, - OnFrameSent(RST_STREAM, 3, _, 0x0, - static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR))); - EXPECT_CALL(visitor, OnCloseStream(3, Http2ErrorCode::HTTP2_NO_ERROR)); - - send_result = adapter->Send(); - EXPECT_EQ(0, send_result); - EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::RST_STREAM})); + EXPECT_FALSE(adapter->want_write()); } TEST(OgHttp2AdapterServerTest, ServerAllowsProtocolPseudoheaderAfterAck) { @@ -4187,8 +4248,8 @@ adapter->SubmitRst(1, Http2ErrorCode::INTERNAL_ERROR); // Server initial SETTINGS and SETTINGS ack. - EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x0)); - EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1)); EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc index ad1fd9d..59ce29b 100644 --- a/http2/adapter/oghttp2_session.cc +++ b/http2/adapter/oghttp2_session.cc
@@ -1337,6 +1337,8 @@ void OgHttp2Session::OnHeaderStatus( Http2StreamId stream_id, Http2VisitorInterface::OnHeaderResult result) { QUICHE_DCHECK_NE(result, Http2VisitorInterface::HEADER_OK); + QUICHE_VLOG(1) << "OnHeaderStatus(stream_id=" << stream_id + << ", result=" << result << ")"; const bool should_reset_stream = result == Http2VisitorInterface::HEADER_RST_STREAM || result == Http2VisitorInterface::HEADER_FIELD_INVALID || @@ -1447,6 +1449,9 @@ settings.push_back({Http2KnownSettingsId::MAX_HEADER_LIST_SIZE, *options_.max_header_list_bytes}); } + if (options_.allow_extended_connect && IsServerSession()) { + settings.push_back({Http2KnownSettingsId::ENABLE_CONNECT_PROTOCOL, 1u}); + } return settings; } @@ -1459,6 +1464,12 @@ if (setting.id == Http2KnownSettingsId::MAX_CONCURRENT_STREAMS) { pending_max_inbound_concurrent_streams_ = setting.value; } + if (setting.id == ENABLE_CONNECT_PROTOCOL && setting.value == 1u && + IsServerSession()) { + // Allow extended CONNECT semantics even before SETTINGS are acked, to + // make things easier for clients. + headers_handler_.AllowConnect(); + } } // Copy the (small) map of settings we are about to send so that we can set @@ -1471,9 +1482,6 @@ } else if (id_and_value.first == spdy::SETTINGS_HEADER_TABLE_SIZE) { decoder_.GetHpackDecoder()->ApplyHeaderTableSizeSetting( id_and_value.second); - } else if (id_and_value.first == ENABLE_CONNECT_PROTOCOL && - id_and_value.second == 1u) { - headers_handler_.AllowConnect(); } } });
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h index a64eea2..a1b96d2 100644 --- a/http2/adapter/oghttp2_session.h +++ b/http2/adapter/oghttp2_session.h
@@ -58,6 +58,10 @@ // error while processing bytes. If true, subsequent processing will also // mark all input data as consumed. bool blackhole_data_on_connection_error = true; + // Whether to advertise support for the extended CONNECT semantics described + // in RFC 8441. If true, this endpoint will send the appropriate setting in + // initial SETTINGS. + bool allow_extended_connect = true; }; OgHttp2Session(Http2VisitorInterface& visitor, Options options);