Conversation
`validateFieldNames` checks each name with `allSatisfy`, which is true for an empty collection, so a request carrying an empty field name passed validation. RFC 9110 defines `field-name = token` and `token = 1*tchar`. Guard on the empty name before the character check.
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.
validateFieldNameschecks each name withallSatisfyover the tchar set.allSatisfyreturnstruefor an empty collection, so a request carrying an empty field name passes validation and is sent.RFC 9110 § 5.1 defines
field-name = token, and § 5.6.2 definestoken = 1*tchar— at least one character. The tchar set in this function is correct; I checked all fifteen symbols plus DIGIT and ALPHA against the ABNF. It is the1*quantifier that is missing.The change
A guard on the empty name before the character check, so it lands in
invalidFieldNameslike any other malformed name.Tests
Added
testEmptyHeaderFieldNameIsRejectedtoRequestValidationTests.swift, next to the existingtestInvalidHeaderFieldNamesandtestValidHeaderFieldNames, which already pin this function from both sides.Verification
I could not run
swift testhere: this machine has Command Line Tools without XCTest, so the test target fails atimport XCTestwithno such module. Pre-existing and unrelated — it reproduces on an unmodified checkout (126 occurrences). CI should be the authority; treat my local run as unverified.swift buildsucceeds. I linked a throwaway executable againstAsyncHTTPClientto exercisevalidateAndSetTransportFramingdirectly:Before the change the first case was accepted; the others were already handled.
Cross-reference
I found the same class of gap —
allSatisfystanding in for1*tchar— in swift-nio-http2 while checking whether this was specific to one implementation, and opened apple/swift-nio-http2#563 for it. Noting it here only so the two are easy to read together; the fixes are independent and neither depends on the other.Conflicts
No open PR touches
Sources/AsyncHTTPClient/RequestValidation.swift. #291 shows up in a filename search but only modifiesTests/AsyncHTTPClientTests/RequestValidationTests.swift, in a streamwriter-API change last updated in 2020.