Updates header validation behavior when encountering a too-long header field.
It turns out that nghttp2 does not invoke OnInvalidFrame(), and does not signal a user callback failure. Replicating this behavior requires adding some new status enum values and further complicating OgHttp2Session::OnHeaderStatus().
PiperOrigin-RevId: 421043626
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc
index 3a70628..0c1d873 100644
--- a/http2/adapter/header_validator.cc
+++ b/http2/adapter/header_validator.cc
@@ -65,7 +65,7 @@
key.size() + value.size() > max_field_size_.value()) {
QUICHE_VLOG(2) << "Header field size is " << key.size() + value.size()
<< ", exceeds max size of " << max_field_size_.value();
- return HEADER_FIELD_INVALID;
+ return HEADER_FIELD_TOO_LONG;
}
const absl::string_view validated_key = key[0] == ':' ? key.substr(1) : key;
if (validated_key.find_first_not_of(kHttp2HeaderNameAllowedChars) !=
diff --git a/http2/adapter/header_validator.h b/http2/adapter/header_validator.h
index fc8c226..b21b299 100644
--- a/http2/adapter/header_validator.h
+++ b/http2/adapter/header_validator.h
@@ -34,6 +34,7 @@
enum HeaderStatus {
HEADER_OK,
HEADER_FIELD_INVALID,
+ HEADER_FIELD_TOO_LONG,
};
HeaderStatus ValidateSingleHeader(absl::string_view key,
absl::string_view value);
diff --git a/http2/adapter/header_validator_test.cc b/http2/adapter/header_validator_test.cc
index 09c4d43..5fe05a8 100644
--- a/http2/adapter/header_validator_test.cc
+++ b/http2/adapter/header_validator_test.cc
@@ -28,7 +28,7 @@
status = v.ValidateSingleHeader(
"name2",
"Antidisestablishmentariansism is supercalifragilisticexpialodocious.");
- EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID, status);
+ EXPECT_EQ(HeaderValidator::HEADER_FIELD_TOO_LONG, status);
}
TEST(HeaderValidatorTest, NameHasInvalidChar) {
diff --git a/http2/adapter/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index 8e39ef4..0e8619f 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -127,6 +127,8 @@
// The headers are a violation of HTTP messaging semantics and will be reset
// with error code PROTOCOL_ERROR.
HEADER_HTTP_MESSAGING,
+ // The headers caused a compression context error.
+ HEADER_COMPRESSION_ERROR,
};
virtual OnHeaderResult OnHeaderForStream(Http2StreamId stream_id,
absl::string_view key,
diff --git a/http2/adapter/nghttp2_callbacks.cc b/http2/adapter/nghttp2_callbacks.cc
index 2d05bee..d96214d 100644
--- a/http2/adapter/nghttp2_callbacks.cc
+++ b/http2/adapter/nghttp2_callbacks.cc
@@ -174,6 +174,7 @@
case Http2VisitorInterface::HEADER_OK:
return 0;
case Http2VisitorInterface::HEADER_CONNECTION_ERROR:
+ case Http2VisitorInterface::HEADER_COMPRESSION_ERROR:
return NGHTTP2_ERR_CALLBACK_FAILURE;
case Http2VisitorInterface::HEADER_RST_STREAM:
case Http2VisitorInterface::HEADER_FIELD_INVALID:
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 5a82c97..2d87254 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -3332,9 +3332,8 @@
EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
EXPECT_CALL(visitor, OnFrameHeader(1, _, CONTINUATION, 0)).Times(3);
EXPECT_CALL(visitor, OnFrameHeader(1, _, CONTINUATION, 4));
- EXPECT_CALL(
- visitor,
- OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
+ EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kHeaderError));
+ // Further header processing is skipped, as the header field is too large.
const int64_t result = adapter->ProcessBytes(frames);
EXPECT_EQ(static_cast<int64_t>(frames.size()), result);
@@ -3343,19 +3342,19 @@
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
- EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
- EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
- EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, 4, 0x0));
- EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, 1, 4, 0x0, 1));
- EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+ // Since the library opted not to process the header, it generates a GOAWAY
+ // with error code COMPRESSION_ERROR.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::COMPRESSION_ERROR)));
int send_result = adapter->Send();
// Some bytes should have been serialized.
EXPECT_EQ(0, send_result);
- // SETTINGS, SETTINGS ack, and RST_STREAM.
+ // SETTINGS and GOAWAY.
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::RST_STREAM}));
+ spdy::SpdyFrameType::GOAWAY}));
}
TEST(OgHttp2AdapterServerTest, ServerRejectsStreamData) {
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index a91af7b..09d3d93 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -226,6 +226,19 @@
validator_.StartHeaderBlock();
}
+Http2VisitorInterface::OnHeaderResult InterpretHeaderStatus(
+ HeaderValidator::HeaderStatus status) {
+ switch (status) {
+ case HeaderValidator::HEADER_OK:
+ return Http2VisitorInterface::HEADER_OK;
+ case HeaderValidator::HEADER_FIELD_INVALID:
+ return Http2VisitorInterface::HEADER_FIELD_INVALID;
+ case HeaderValidator::HEADER_FIELD_TOO_LONG:
+ return Http2VisitorInterface::HEADER_COMPRESSION_ERROR;
+ }
+ return Http2VisitorInterface::HEADER_CONNECTION_ERROR;
+}
+
void OgHttp2Session::PassthroughHeadersHandler::OnHeader(
absl::string_view key,
absl::string_view value) {
@@ -233,11 +246,12 @@
QUICHE_VLOG(2) << "Early return; status not HEADER_OK";
return;
}
- const auto validation_result = validator_.ValidateSingleHeader(key, value);
+ const HeaderValidator::HeaderStatus validation_result =
+ validator_.ValidateSingleHeader(key, value);
if (validation_result != HeaderValidator::HEADER_OK) {
- QUICHE_VLOG(2) << "RST_STREAM with result "
+ QUICHE_VLOG(2) << "Header validation failed with result "
<< static_cast<int>(validation_result);
- result_ = Http2VisitorInterface::HEADER_FIELD_INVALID;
+ result_ = InterpretHeaderStatus(validation_result);
return;
}
result_ = visitor_.OnHeaderForStream(stream_id_, key, value);
@@ -1287,6 +1301,9 @@
fatal_visitor_callback_failure_ = true;
LatchErrorAndNotify(Http2ErrorCode::INTERNAL_ERROR,
ConnectionError::kHeaderError);
+ } else if (result == Http2VisitorInterface::HEADER_COMPRESSION_ERROR) {
+ LatchErrorAndNotify(Http2ErrorCode::COMPRESSION_ERROR,
+ ConnectionError::kHeaderError);
}
}
