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);