Call visitor_.OnInvalidFrame() for oghttp2 header errors.
This CL causes OgHttp2Session to call visitor_.OnInvalidFrame() when
encountering a stream-level header error, including header validation errors.
Based on the OnInvalidFrame() return value, OgHttp2Session can treat the error
as a stream-level error (RST_STREAM) or a connection-level error (RST_STREAM +
GOAWAY). This call and handling allow the following frame flooding tests
involving header validation to pass with cl/392724171:
- Http2FloodMitigationTest.UpstreamZerolenHeaderAllowed
http://sponge2/46bed1d4-d301-482a-9721-9e8e0c0a0baa
- Http2FloodMitigationTest.UpstreamZerolenHeader
http://sponge2/3156868a-dd44-4c0a-a7dd-7107f1ed229c
PiperOrigin-RevId: 407608309
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 6dd180e..56fcff5 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -1908,6 +1908,65 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(NgHttp2AdapterTest, ServerConnectionErrorWhileHandlingHeaders) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"},
+ {"Accept", "uppercase, oh boy!"}},
+ /*fin=*/false)
+ .WindowUpdate(1, 2000)
+ .Data(1, "This is the request body.")
+ .WindowUpdate(0, 2000)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "POST"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
+ EXPECT_CALL(visitor, OnErrorDebug);
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader))
+ .WillOnce(testing::Return(false));
+ // Translation to nghttp2 treats this error as a general parsing error.
+ EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kParseError));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(result, NGHTTP2_ERR_CALLBACK_FAILURE);
+
+ EXPECT_TRUE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, 4, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, 1, 4, 0x0,
+ static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::PROTOCOL_ERROR));
+
+ int send_result = adapter->Send();
+ // Some bytes should have been serialized.
+ EXPECT_EQ(0, send_result);
+ // SETTINGS ack and RST_STREAM
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM}));
+}
+
TEST(NgHttp2AdapterTest, ServerErrorAfterHandlingHeaders) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 05ee09d..ec879f2 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -1708,6 +1708,9 @@
EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
EXPECT_CALL(visitor, OnHeaderForStream(1, "accept", "some bogus value!"))
.WillOnce(testing::Return(Http2VisitorInterface::HEADER_RST_STREAM));
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
EXPECT_CALL(visitor, OnWindowUpdate(1, 2000));
EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0));
@@ -1740,6 +1743,72 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(OgHttp2AdapterServerTest, ServerConnectionErrorWhileHandlingHeaders) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"},
+ {"Accept", "uppercase, oh boy!"}},
+ /*fin=*/false)
+ .WindowUpdate(1, 2000)
+ .Data(1, "This is the request body.")
+ .WindowUpdate(0, 2000)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "POST"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader))
+ .WillOnce(testing::Return(false));
+ EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kHeaderError));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_LT(result, 0);
+
+ EXPECT_TRUE(adapter->want_write());
+
+ 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));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, 4, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, 1, 4, 0x0,
+ static_cast<int>(Http2ErrorCode::INTERNAL_ERROR)));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::NO_ERROR));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::INTERNAL_ERROR)));
+
+ int send_result = adapter->Send();
+ // Some bytes should have been serialized.
+ EXPECT_EQ(0, send_result);
+ // SETTINGS ack
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM,
+ spdy::SpdyFrameType::GOAWAY}));
+}
+
TEST(OgHttp2AdapterServerTest, ServerErrorAfterHandlingHeaders) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 7c8790d..21dd589 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -963,6 +963,13 @@
if (it == streams_reset_.end()) {
EnqueueFrame(absl::make_unique<spdy::SpdyRstStreamIR>(
stream_id, spdy::ERROR_CODE_INTERNAL_ERROR));
+
+ const bool ok = visitor_.OnInvalidFrame(
+ stream_id, Http2VisitorInterface::InvalidFrameError::kHttpHeader);
+ if (!ok) {
+ LatchErrorAndNotify(Http2ErrorCode::INTERNAL_ERROR,
+ ConnectionError::kHeaderError);
+ }
}
} else if (result == Http2VisitorInterface::HEADER_CONNECTION_ERROR) {
LatchErrorAndNotify(Http2ErrorCode::INTERNAL_ERROR,
