Add BalsaVisitorInterface::OnHeader(). This method is called as soon as each header is parsed. This allows embedders to validate headers and potentially signal errors earlier. The motivation for this new method is Http1ServerConnectionImplTest.HeaderInvalidCharsRejection in Envoy, see https://sourcegraph.com/github.com/envoyproxy/envoy@4fd31f2/-/blob/test/common/http/http1/codec_impl_test.cc?L1115-1134. This test does not include the end of the header block, therefore BalsaVisitorInterface::ProcessHeaders() is never called. With this new method BalsaParser in Envoy will be able to validate the headers and make this test pass. PiperOrigin-RevId: 460514650
diff --git a/quiche/balsa/balsa_frame.cc b/quiche/balsa/balsa_frame.cc index d80b4ba..8e693cb 100644 --- a/quiche/balsa/balsa_frame.cc +++ b/quiche/balsa/balsa_frame.cc
@@ -447,6 +447,15 @@ CleanUpKeyValueWhitespace(stream_begin, line_begin, current, line_end, ¤t_header_line); } + + const absl::string_view key( + stream_begin + current_header_line.first_char_idx, + current_header_line.key_end_idx - current_header_line.first_char_idx); + const absl::string_view value( + stream_begin + current_header_line.value_begin_idx, + current_header_line.last_char_idx - + current_header_line.value_begin_idx); + visitor_->OnHeader(key, value); } return true;
diff --git a/quiche/balsa/balsa_frame_test.cc b/quiche/balsa/balsa_frame_test.cc index 7895cd9..f695829 100644 --- a/quiche/balsa/balsa_frame_test.cc +++ b/quiche/balsa/balsa_frame_test.cc
@@ -539,6 +539,8 @@ MOCK_METHOD(void, OnRawBodyInput, (absl::string_view input), (override)); MOCK_METHOD(void, OnBodyChunkInput, (absl::string_view input), (override)); MOCK_METHOD(void, OnHeaderInput, (absl::string_view input), (override)); + MOCK_METHOD(void, OnHeader, (absl::string_view key, absl::string_view value), + (override)); MOCK_METHOD(void, OnTrailerInput, (absl::string_view input), (override)); MOCK_METHOD(void, ProcessHeaders, (const FakeHeaders& headers)); MOCK_METHOD(void, ProcessTrailers, (const FakeHeaders& headers)); @@ -580,6 +582,8 @@ balsa_frame_.set_balsa_trailer(&trailer_); balsa_frame_.set_balsa_visitor(&visitor_mock_); balsa_frame_.set_is_request(true); + + EXPECT_CALL(visitor_mock_, OnHeader).Times(AnyNumber()); } void VerifyFirstLineParsing(const std::string& firstline, @@ -1220,17 +1224,28 @@ "funky: monkeys\r\n" "\r\n"; + InSequence s; + + // OnHeader() visitor method is called as soon as headers are parsed. + EXPECT_CALL(visitor_mock_, OnHeader("Connection", "close")); + EXPECT_CALL(visitor_mock_, OnHeader("transfer-encoding", "chunked")); ASSERT_EQ(headers.size(), balsa_frame_.ProcessInput(headers.data(), headers.size())); + testing::Mock::VerifyAndClearExpectations(&visitor_mock_); + ASSERT_EQ(chunks.size(), balsa_frame_.ProcessInput(chunks.data(), chunks.size())); + EXPECT_CALL(visitor_mock_, OnHeader("crass", "monkeys")); + EXPECT_CALL(visitor_mock_, OnHeader("funky", "monkeys")); + FakeHeaders fake_trailers; fake_trailers.AddKeyValue("crass", "monkeys"); fake_trailers.AddKeyValue("funky", "monkeys"); + EXPECT_CALL(visitor_mock_, ProcessTrailers(fake_trailers)); EXPECT_CALL(visitor_mock_, OnTrailerInput(_)).Times(AtLeast(1)); - EXPECT_CALL(visitor_mock_, ProcessTrailers(fake_trailers)); + EXPECT_EQ(trailer.size(), balsa_frame_.ProcessInput(trailer.data(), trailer.size())); @@ -2336,8 +2351,10 @@ EXPECT_CALL(visitor_mock, OnResponseFirstLineInput( "HTTP/1.1 \t 200 Ok all is well", "HTTP/1.1", "200", "Ok all is well")); + EXPECT_CALL(visitor_mock, OnHeader); EXPECT_CALL(visitor_mock, ProcessHeaders(fake_headers)); EXPECT_CALL(visitor_mock, HeaderDone()); + EXPECT_CALL(visitor_mock, OnHeader); EXPECT_CALL(visitor_mock, ProcessTrailers(fake_headers_in_trailer)); EXPECT_CALL(visitor_mock, MessageDone()); } @@ -2460,6 +2477,7 @@ EXPECT_CALL(visitor_mock_, OnRequestFirstLineInput("GET / HTTP/1.1", "GET", "/", "HTTP/1.1")); EXPECT_CALL(visitor_mock_, OnHeaderInput(_)); + EXPECT_CALL(visitor_mock_, OnHeader).Times(AnyNumber()); EXPECT_CALL(visitor_mock_, HandleError(BalsaFrameEnums::INVALID_HEADER_FORMAT)); @@ -2488,6 +2506,7 @@ EXPECT_CALL(visitor_mock_, OnResponseFirstLineInput); EXPECT_CALL(visitor_mock_, OnHeaderInput); + EXPECT_CALL(visitor_mock_, OnHeader).Times(AnyNumber()); EXPECT_CALL(visitor_mock_, ProcessHeaders); EXPECT_CALL(visitor_mock_, HeaderDone); EXPECT_CALL(visitor_mock_, OnChunkLength(3)); @@ -2498,6 +2517,7 @@ EXPECT_CALL(visitor_mock_, OnChunkExtensionInput); EXPECT_CALL(visitor_mock_, OnRawBodyInput); EXPECT_CALL(visitor_mock_, OnRawBodyInput); + EXPECT_CALL(visitor_mock_, OnHeader).Times(AnyNumber()); const auto expected_error = invalid_name_char ? BalsaFrameEnums::INVALID_TRAILER_NAME_CHARACTER : BalsaFrameEnums::INVALID_TRAILER_FORMAT; @@ -2565,6 +2585,8 @@ EXPECT_CALL(visitor_mock_, OnRequestFirstLineInput("GET / HTTP/1.1", "GET", "/", "HTTP/1.1")); EXPECT_CALL(visitor_mock_, OnHeaderInput(_)); + EXPECT_CALL(visitor_mock_, OnHeader("i", "")); + EXPECT_CALL(visitor_mock_, OnHeader("", "val")); EXPECT_CALL(visitor_mock_, HandleWarning(BalsaFrameEnums::HEADER_MISSING_COLON)) .Times(27);
diff --git a/quiche/balsa/balsa_visitor_interface.h b/quiche/balsa/balsa_visitor_interface.h index b05d7ad..7dc9de7 100644 --- a/quiche/balsa/balsa_visitor_interface.h +++ b/quiche/balsa/balsa_visitor_interface.h
@@ -51,6 +51,14 @@ virtual void OnHeaderInput(absl::string_view input) = 0; // Summary: + // BalsaFrame passes each header through this function as soon as it is + // parsed. + // Argument: + // key - the header name. + // value - the associated header value. + virtual void OnHeader(absl::string_view key, absl::string_view value) = 0; + + // Summary: // BalsaFrame passes the raw trailer data through this function. This is not // cleaned up in any way. Note that trailers only occur in a message if // there was a chunked encoding, and not always then.
diff --git a/quiche/balsa/noop_balsa_visitor.h b/quiche/balsa/noop_balsa_visitor.h index bedd948..4045cf8 100644 --- a/quiche/balsa/noop_balsa_visitor.h +++ b/quiche/balsa/noop_balsa_visitor.h
@@ -30,6 +30,8 @@ void OnRawBodyInput(absl::string_view /*input*/) override {} void OnBodyChunkInput(absl::string_view /*input*/) override {} void OnHeaderInput(absl::string_view /*input*/) override {} + void OnHeader(absl::string_view /*key*/, + absl::string_view /*value*/) override {} void OnTrailerInput(absl::string_view /*input*/) override {} void ProcessHeaders(const BalsaHeaders& /*headers*/) override {} void ProcessTrailers(const BalsaHeaders& /*trailer*/) override {}