Reject HTTP/1 header forms that desync framing, drop chunked trailers

[chenBright]

What problem does this PR solve?

Issue Number: resolve

Problem Summary:

Four header forms are parsed here in a way a conforming front-end does not, and
one legal form is not parsed at all. In a client -> proxy -> bRPC chain both
parties parse the same bytes independently, so every disagreement about where a
message ends turns the tail of message N into the head of message N+1 for one of
them. The primitive behind request smuggling and response desync.

1. Content-Length : 5 — this fork's TOKEN(c) maps SP to itself. Upstream
   has a STRICT_TOKEN() and a HTTP_PARSER_STRICT branch. Both were dropped
   here, leaving the lenient path as the only path. A space before the colon
   therefore keeps header_state on the framing header, so the value still
   drives our framing while the field name we report is "Content-Length ".
   RFC 7230 3.2.6 excludes SP from tchar, and 3.2.4 forbids whitespace before
   the colon.

2. Content-Length: 1 3 — interior blanks were skipped, so we read 13 where
   a proxy reads 1 or rejects. Same class as CVE-2022-32213, fixed upstream by
   http-parser nodejs/http-parser@01da95f and nodejs/http-parser@cd88eef.
   Neither was picked up here.

3. Transfer-Encoding:\r\n chunked, Content-Length:\r\n 5 — both obs-fold
   states (RFC 7230 3.2.4) carry header_state across the fold, so a folded
   framing value still drove our framing while a front-end that does not unfold
   sees a different message.

4. chunked trailers — after the last chunk the parser sets F_TRAILING and
   returns to s_header_field_start, so trailer fields reach the same
   on_header_field / on_header_value callbacks as the header section. Neither
   looked at F_TRAILING, so a trailer landed in the header map like a real
   header: appended to an existing entry of the same name, or added as a new one.
   A front-end only filters the header section, so any chunked request could put
   an Authorization or X-Forwarded-For past it:

   POST / HTTP/1.1
   Host: a.com
   X-Forwarded-For: 10.0.0.1        <- set by the proxy
   Transfer-Encoding: chunked
   
   5
   hello
   0
   X-Forwarded-For: 6.6.6.6         <- invisible to the proxy, appended by us
   Authorization: Bearer stolen
   

5. Transfer-Encoding: gzip, chunked — found while auditing the TOKEN()
   call sites for (1). TOKEN(' ') is truthy, which made the
   else if (c == ' ' || c == '\t') arm of
   h_matching_transfer_encoding_token_start dead code: the OWS after the comma
   started a new coding name, so chunked never matched. This is the form
   RFC 7230 7 spells out. Requests were rejected with
   HPE_INVALID_TRANSFER_ENCODING per RFC 7230 3.3.3, and responses were read to
   EOF instead of as chunked.

What is changed and the side effects?

Changed:

• Reintroduce STRICT_TOKEN(). This fork's tokens[] already omits SP as
  RFC 7230 3.2.6 does, so the strict form is the plain lookup and TOKEN() is
  the leniency layered on top.

• Reject SP before the colon of Content-Length, Transfer-Encoding,
  Connection and Upgrade. Those four are the only terminal header_states
  reachable in that switch arm. Any other name is h_general or still
  matching, and keeps the historical leniency unless the new flag is on.

• Backport nodejs/http-parser@01da95f and nodejs/http-parser@cd88eef.

• Reject obs-fold of a value whose header_state decides where the body ends
  (is_framing_header_state(), i.e. the Content-Length and Transfer-Encoding
  states). Both fold states need the check: s_header_value_lws for a value
  that has started, s_header_value_discard_lws for one that has not.

  • Use STRICT_TOKEN() unconditionally in h_matching_transfer_encoding_token_start,
    as upstream does, so the OWS after a comma is skipped instead of starting a coding name.

  • Add the h_content_length*, h_transfer_encoding, h_upgrade and
    h_matching_transfer_encoding_* cases missing from
    http_parser_header_state_name(), which printed h_unknown for them.

• on_header_field and on_header_value return early when F_TRAILING is
  set, so trailers are discarded rather than merged. parser->flags is
  cleared at the start of each message, so the check is scoped to one trailer.

Side effects:

• Performance effects:

• Breaking backward compatibility:

---

Check List:

• Please make sure your changes are compilable.
• When providing us with a new feature, it is best to add related tests.
• Please follow Contributor Covenant Code of Conduct.

[Copilot]

🟡 Changes recommended

The current changes introduce a few correctness/test-isolation issues (notably test dependence on default gflags and inconsistent handling of HTAB as OWS in Content-Length whitespace states) that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens bRPC’s embedded HTTP/1 parser to eliminate several parsing discrepancies that can lead to request smuggling / response desync in proxy chains, and it drops chunked trailers so they can’t be merged into the main header map.

Changes:

• Add strict/compat toggles for header token parsing (http_strict_header_token) and obs-fold handling for framing headers (http_allow_obs_fold).
• Tighten parsing of Content-Length and Transfer-Encoding to reject ambiguous forms (e.g., interior whitespace in Content-Length, framing-header obs-fold) and correctly handle Transfer-Encoding lists with OWS after commas.
• Discard chunked trailers in HttpMessage callbacks to prevent trailer fields from being merged into the header map; add extensive unit tests covering the above.

File summaries

File	Description
test/brpc_http_message_unittest.cpp	Adds regression tests for strict header tokens, framing-header whitespace/obs-fold rejection, TE list parsing, and dropping chunked trailers.
src/brpc/details/http_parser.h	Adds two parser config bitfields to control obs-fold and strict header token handling.
src/brpc/details/http_parser.cpp	Implements strict token parsing for header field names, more robust Content-Length parsing, framing-header obs-fold rejection, and correct TE list parsing with OWS after comma.
src/brpc/details/http_message.cpp	Adds gflags for new parser behaviors, wires them into HttpMessage parser init, and drops trailer callbacks when F_TRAILING is set.

Review details

Suppressed comments (3)

src/brpc/details/http_parser.cpp:1699

• When transitioning from Content-Length digits to the trailing-whitespace state, only SP is recognized. If you intend to support RFC 7230 OWS, this should also treat HTAB as whitespace so that trailing tabs don’t incorrectly trigger HPE_INVALID_CONTENT_LENGTH.

            if (ch == ' ') {
              parser->header_state = h_content_length_ws;
              break;
            }

src/brpc/details/http_parser.cpp:1726

• In the Content-Length trailing-whitespace state, only SP is skipped. If HTAB is treated as OWS elsewhere (e.g., after ':'), it should likely be accepted here too (or the comment should be updated to avoid claiming OWS support).

          case h_content_length_ws:
            if (ch == ' ') break;
            SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);

test/brpc_http_message_unittest.cpp:547

• This test asserts --http_allow_obs_fold is false by default, which can break when the test binary is executed with a non-default flag value. Set the flag explicitly (and save/restore it) so the test is independent of external flag configuration.

    ASSERT_FALSE(brpc::FLAGS_http_allow_obs_fold);
    ASSERT_EQ(brpc::HPE_INVALID_HEADER_TOKEN, ParseHttpErrno(folded_te_value));

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.