Repository navigation
Match the documented parameter names to the signatures in NIOHPACK - #562
Merged
Merged
Conversation
glbrntt
approved these changes
Sep 1, 2026
glbrntt
enabled auto-merge (squash)
September 1, 2026 05:14
glbrntt
pushed a commit
that referenced
this pull request
Sep 1, 2026
`isValidFieldName` validates a field name with `allSatisfy` over the
tchar set. `allSatisfy` returns `true` for an empty collection, so the
empty field name passes.
The file already states the rule it is implementing. The doc comment at
line 312 quotes:
```
token = 1*tchar
```
The derivation that follows is correct — I expanded `0x21, 0x23...0x27,
0x2a, 0x2b, 0x2d, 0x2e, 0x30...0x39, 0x5e...0x7a, 0x7c, 0x7e` byte by
byte against tchar minus `A-Z` and every one of the four rewriting steps
in lines 315-341 holds, including the `%x5E-60 / %x61-7A` → `%x5E-7A`
collapse. It is the `1*` quantifier that is missing: the code implements
`*tchar`.
### Scope
All three block types accept an empty name today, since they share
`HeaderFieldName.init`:
```
request: empty name -> accepted
response: empty name -> accepted
trailers: empty name -> accepted
```
An empty name serialises as `: value`, and RFC 9113 § 8.2.1 lists field
names as something a recipient is required to validate and treat as
malformed when the generic syntax is violated. This is a conformance gap
in a validator that already does the rest of that section — the
uppercase check, the CR/LF/NUL value check and the connection-specific
header check are all there.
(`":"` alone is already rejected, though as `unknownPseudoHeader` rather
than a name error, since `dropFirst()` leaves an empty base name that
matches no known pseudo-header. That path is unchanged here.)
### The change
An explicit early return, mirroring the shape swift-http-types uses for
the same production in `tokenValidity`:
```swift
if buffer.isEmpty {
return .invalid
}
```
### Tests
Added to `HTTP2FramePayloadToHTTP1CodecCRLFTests.swift`, which already
exercises `validateRequestBlock`, `validateResponseBlock` and
`validateTrailersBlock` directly for the § 8.2.1 value rules. Five
cases: the empty name in each of the three block types, plus `"a b"` and
an ordinary `x-custom-header` to keep the surrounding behaviour pinned.
### Verification
**I could not run `swift test` here**: this machine has Command Line
Tools without XCTest, so the test targets fail at `import XCTest` with
`no such module`. Pre-existing and unrelated — it reproduces on an
unmodified checkout (18 occurrences). **CI should be the authority;
treat my local run as unverified.**
`swift build` succeeds. I linked a throwaway executable against
`NIOHTTP2` to exercise the change directly:
```
PASS request: empty name -> rejected
PASS response: empty name -> rejected
PASS trailers: empty name -> rejected
PASS request: "a b" -> rejected
PASS request: "Host" (uppercase) -> rejected
PASS request: valid name -> accepted
PASS request: pseudo-headers only -> accepted
```
Before the change the first three were accepted.
### Conflicts
No open PR touches `HPACKHeaders+Validation.swift`. #531 and #562 touch
NIOHPACK, and #531 (`+ Fix a bug`) is about `isASCII` in
`HPACKHeader.swift` — no overlap with `isValidFieldName`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The base64 helper looks like it came over from Foundation with
Data(base64Encoded:options:)still in the doc comment, and the parameter is a Bool now. The other two seem to be plain leftovers. All three are internal, so DocC never builds them and nothing flags the mismatch. Nothing else touched.