Auto-port 5.0: HTTP/2: Parse request-target path like Vert.x#16855
Merged
Conversation
Motivation: `HttpConversionUtil.toHttp2Headers` currently depends on `java.net.URI` for absolute-form request-target parsing. On JDKs that still enforce older URI syntax, path or query characters that appear in real HTTP request-targets can make HTTP/1.x to HTTP/2 conversion fail before `:path` is produced. Netty only needs URI parsing for the lower-frequency `scheme://authority` validation/extraction path. The hot path/query extraction can follow the same lightweight parsing shape used by Vert.x while avoiding full URI parsing and avoiding a try/catch fallback. Modification: - Split request-target path and query parsing into Vert.x-shaped `parsePath` and `parseQuery` helpers, with comments for Netty-specific differences. - Keep `URI` parsing for `scheme://authority` validation/extraction only after stripping path/query/fragment data. - Preserve origin-form and asterisk-form behavior. - Add regression tests for characters rejected by `java.net.URI`, authority-only and missing-authority absolute-form targets, empty query/fragment handling, and malformed authority validation. - Add a Jazzer fuzz test that compares the new behavior against the old URI-based conversion using broad `consumeString(128)` request-target input and narrow documented compatibility exceptions. Result: HTTP/2 conversion no longer relies on full `java.net.URI` parsing for request-target path/query extraction, while preserving meaningful existing behavior and continuing to validate/extract scheme and authority through URI where appropriate. Verification performed locally: - `./mvnw -pl codec-http2 -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn test` - `JAZZER_FUZZ=1 ./mvnw -pl codec-http2 -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn -Dcheckstyle.skip=true -Dtest=HttpConversionUtilFuzzTest test` - `./mvnw -pl codec-http2 -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn -Dcheckstyle.skip=true -Dsurefire.failIfNoSpecifiedTests=false -Dtest=HttpConversionUtilTest,HttpConversionUtilFuzzTest test` --------- Co-authored-by: multicode <multicode@yawk.at> (cherry picked from commit a42c7fc)
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.
Auto-port of #16810 to 5.0
Cherry-picked commit: a42c7fc
Motivation:
HttpConversionUtil.toHttp2Headerscurrently depends onjava.net.URIfor absolute-form request-target parsing. On JDKs that still enforce older URI syntax, path or query characters that appear in real HTTP request-targets can make HTTP/1.x to HTTP/2 conversion fail before:pathis produced.Netty only needs URI parsing for the lower-frequency
scheme://authorityvalidation/extraction path. The hot path/query extraction can follow the same lightweight parsing shape used by Vert.x while avoiding full URI parsing and avoiding a try/catch fallback.Modification:
parsePathandparseQueryhelpers, with comments for Netty-specific differences.URIparsing forscheme://authorityvalidation/extraction only after stripping path/query/fragment data.java.net.URI, authority-only and missing-authority absolute-form targets, empty query/fragment handling, and malformed authority validation.consumeString(128)request-target input and narrow documented compatibility exceptions.Result:
HTTP/2 conversion no longer relies on full
java.net.URIparsing for request-target path/query extraction, while preserving meaningful existing behavior and continuing to validate/extract scheme and authority through URI where appropriate.Verification performed locally:
./mvnw -pl codec-http2 -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn testJAZZER_FUZZ=1 ./mvnw -pl codec-http2 -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn -Dcheckstyle.skip=true -Dtest=HttpConversionUtilFuzzTest test./mvnw -pl codec-http2 -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn -Dcheckstyle.skip=true -Dsurefire.failIfNoSpecifiedTests=false -Dtest=HttpConversionUtilTest,HttpConversionUtilFuzzTest test