Validate header names on the outbound path

[avalyset]

Stacked on #1325: the first two commits here (4aa3d90, ae42b9a) are #1325's. Rebase onto master follows once #1325 merges.

Follow-up to #1325, as discussed there in #1325 (comment).

Cause

_reject_illegal_characters is called from validate_headers but not from validate_outbound_headers. Once #1325 lands it is the only check in the inbound chain with no outbound counterpart.

It could not simply be added to the outbound chain, because it carries two value rules in addition to the four name classes of RFC 9113 section 8.2.1.

Change

_reject_illegal_characters is split into _reject_illegal_name_characters (uppercase 0x41-0x5a, <= 0x20, >= 0x7f, colon after position 0) and _reject_illegal_value_characters (NUL/LF/CR, surrounding SP/HTAB). validate_headers calls both in the previous order, so inbound behaviour is unchanged. validate_outbound_headers calls the name half only, first in the chain, mirroring the inbound order.

The value rules deliberately stay inbound-only: _strip_surrounding_whitespace already normalises the whitespace case before outbound validation, so enabling that half would only change behaviour for normalize_outbound_headers=False.

The uppercase message is made direction-neutral (Uppercase header name present: ...) because the check now runs both ways; this also changes the inbound message. Received header value surrounded by whitespace is left alone, as that rule is still inbound-only.

Measured

Through send_headers, decoding what reaches the wire:

name	before, normalize=True	before, normalize=False	after, normalize=True	after, normalize=False
X-Foo (class 1)	sent as x-foo	sent as X-Foo	sent as x-foo	ProtocolError
foo bar (class 2)	sent	sent	ProtocolError	ProtocolError
foo\x7f (class 3)	sent	sent	ProtocolError	ProtocolError
foo:bar (class 4)	sent	sent	ProtocolError	ProtocolError

src/h2/utilities.py is +21/-5. Seven of the eleven added tests fail against the unmodified file; the other four are regression guards that pass either way. Full suite 1673 passed, ruff check src/ and mypy --strict-bytes clean.

Note

This touches validate_outbound_headers in utilities.py, the same function as #1325. Whichever lands second, I will rebase it.

🤖 Generated with Claude Code

[feiiiiii5]

Checked this against RFC 9113 section 8.2.1 directly rather than only through the added tests, and the rule is right. One sequencing note at the end.

Name rule, all 256 byte values. I drove the new name pass over every byte as the name a<byte>b. The RFC forbids 0x00-0x20, 0x41-0x5A and 0x7F-0xFF — 188 values — and the pass rejects exactly that set, with no value outside those ranges rejected by the range check. The one extra rejection is : mid-name, which is the second sentence of 8.2.1 rather than the byte ranges, so it is correct rather than an extra restriction.

The pseudo-header case, which is the one most likely to break here. Applying an inbound-only rule to outbound headers would be a problem if it rejected the leading colon, since every outbound request needs :method, :path, :scheme and :authority. It does not:

b':method'  accepted      b'a:b'   rejected
b':x'       accepted      b'abc:'  rejected
b'x-custom' accepted      b'::x'   rejected

End-to-end, the two paths now agree. With a complete valid request header set and one header under test, validate_headers and validate_outbound_headers return the same verdict for every sampled illegal byte (0x00-0x2F, 0x41-0x5A, 0x7F, 0x80, 0xFF — 77 cases): 0 disagreements. Before this change the outbound path applied no name-character rule at all, so this is the part that matters and it holds.

Suite and coverage. 1673 passed. src/h2/utilities.py remains at 100% (270 statements, 144 branches, 0 missing), so splitting the generator in two does not cost branch coverage and fail_under=100 still holds.

One thing to sort out: this conflicts with your #1325. Applying #1325 on top of this branch conflicts in both src/h2/utilities.py and tests/test_invalid_headers.py, for two independent reasons:

• both edit the head of validate_outbound_headers — this adds the name pass above the existing empty-name pass, while Reject empty header names on the outbound path #1325 renames _reject_sent_empty_header_names to _reject_empty_header_names in that same spot;
• this renames _reject_illegal_characters to _reject_illegal_name_characters, and that old name is the hunk context line Reject empty header names on the outbound path #1325's first hunk anchors on.

Nothing wrong with either change on its own — #1325 is a clean consolidation and this is a clean addition — but as they stand one of them will need a rebase at merge time. Deciding which lands first, or folding them into one PR, would avoid that. Flagging it because the two are yours and the pairing is not obvious from either PR on its own.

[avalyset]

Thanks for running the full byte range and the two-path comparison. That is a stronger check than the tests alone give, and it matches what I see on the branch.

On sequencing: #1325 lands first, since Kriechi has already read it. I have rebased this branch onto #1325, so the two now stack; the diff here carries #1325's two commits until it merges, after which I rebase onto master. Only src/h2/utilities.py conflicts, since both branches add one pass at the head of validate_outbound_headers. tests/test_invalid_headers.py merges cleanly, because the two test blocks land in different places. The resolution chains the two passes, name characters first, then empty names. A name with no bytes passes the character rule, so no name fails both and the order changes no verdict. Full suite on the stack: 1694 passed.

[feiiiiii5]

Thanks — and I checked the disjointness argument rather than just accepting it, because it's the thing the whole resolution rests on. It holds, and there's a stronger reason underneath it.

The per-name claim is correct. I ran both predicates over 30 names (empty, uppercase, mixed case, space/tab/newline/NUL/0x7f/0x80/0xff, leading and interior colons, :authority, :status, te, bare \x0b):

only the character rule: 17 names   (incl. b'Content-Length', b'a b', b'a:b', b':', b'\x0b')
only the empty-name rule:  1 name   (b'')
both:                      0 names
neither:                   11 names

The reason is structural: every branch of _reject_illegal_name_characters needs at least one byte — an uppercase byte, a byte <= 0x20 or >= 0x7f, or a colon found from index 1 — so a name that trips it is necessarily non-empty and therefore cannot trip the empty check.

But the order doesn't matter for a stronger reason than that, which is worth knowing when you rebase. The two passes are lazily chained generators, not two sequential loops. validate_outbound_headers builds g1 = _reject_illegal_name_characters(...) and then g2 = _reject_empty_header_names(g1), so the two interleave as g2 pulls from g1 — each pass only runs as far as the next header the one above it yields. The upshot is that the first header in list order that violates either rule is the one that raises, whichever pass is listed first. I confirmed by running four multi-bad-header inputs through both orderings by hand:

headers [(b'', b'v'), (b'BAD', b'v')]   char-first: zero length     empty-first: zero length
headers [(b'BAD', b'v'), (b'', b'v')]   char-first: uppercase       empty-first: uppercase
headers [(b'', b'v'), (b'a b', b'v')]   char-first: zero length     empty-first: zero length
headers [(b'host', b'x'), (b'', b'v')]  char-first: zero length     empty-first: zero length

Same message both ways in every case. So your resolution is safe by construction, and the per-name disjointness argument — correct as it is — isn't actually load-bearing. That's a small reassurance for the rebase: the conflict in src/h2/utilities.py doesn't depend on which way you resolve it.

One asymmetry I checked while I was in there, since it's the kind of thing that bites later: the colon search uses find(b":", 1), so a leading colon is allowed (pseudo-headers) while an interior one isn't, and b":" is caught by the character rule rather than the empty one. Both behave as intended in the stacked branch.

Glad the byte-range run was useful. I'll leave the sequencing to you and Kriechi.