[balsa] Handle empty field names

[bencebeky]

Title: [balsa] Handle empty field names

Description:

This is a bug in Balsa, which is protected by envoy.reloadable_features.http1_use_balsa_parser. This flag defaults to false and no deployment is known to enable it, so the impact of this bug is minimal. When the flag is false, another HTTP/1 parser called http-parser is used instead of Balsa.

ConnectionImpl::completeCurrentHeader() normally clears current_header_value_ at https://github.com/envoyproxy/envoy/blob/48364f3adc34048501fb261aef74fb8fada0b0c9/source/common/http/http1/codec_impl.cc#L549, then it asserts that it is empty at https://github.com/envoyproxy/envoy/blob/48364f3adc34048501fb261aef74fb8fada0b0c9/source/common/http/http1/codec_impl.cc#L564. However, if current_header_field_ is empty but current_header_value_ is not when completeCurrentHeader() is called, then it does not clear current_header_value_, and the assertion fails.

When parsing a field line (header line) with empty name, for example, ": foo", http-parser signals an error when encountering the colon, as it is an invalid token for the field name. Balsa, however, parser the line as one with an empty name. If this is followed by another field line, then onHeaderFieldImpl() callscompleteCurrentHeader()`, and the assertion fails.

However, if the field line with empty name is the last one, then completeCurrentHeader() is never called, so there is no assertion failure. This is because there is a check for empty names at https://quiche.googlesource.com/quiche/+/afbd9cde746c772422d20744b3d61597e29267ef/quiche/balsa/balsa_frame.cc#604, but unfortunately it happens after the BalsaVisitorInterface::OnHeader() call at https://quiche.googlesource.com/quiche/+/afbd9cde746c772422d20744b3d61597e29267ef/quiche/balsa/balsa_frame.cc#458.

This bug was discovered by the fuzz test //test/common/http/http1:http1_codec_impl_fuzz_test, with a field line :scheme: https, which was correctly parsed as empty name, and value scheme: https.

[bencebeky]

/assign @bencebeky

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions[bot]]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

[bencebeky]

This has been fixed by #25475, which also added a regression test.