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