Skip to content

split the authority before IDN conversion in isValidAuthority - #438

Open
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:url-authority-split-before-idn
Open

sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:url-authority-split-before-idn

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor
  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? Claude Code (Anthropic) was used to investigate the behaviour and to prepare the change, the test and this description. The reproducer, the new test and the full default Maven build were run on the result before submitting.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body.

UrlValidator.isValidAuthority passes the whole authority, userinfo and port included, to DomainValidator.unicodeToASCII and only then splits the converted text with AUTHORITY_PATTERN. I got there from a plain false negative: isValid("http://президент.рф:8080/") is false although the same host without a port passes, and printing the converted authority showed xn--d1abbgf6aiiy.xn--:8080-uye2a, the port punycoded into the last label. The ordering has a worse side, because nameprep folds compatibility characters to ASCII: a fullwidth commercial at (U+FF20) or colon (U+FF1A) comes back as a real @ or :, so isValid("http://example.com\uFF20apache.org/") is true with example.com read as userinfo and only apache.org checked as the host, while java.net.URI sees a single registry-based authority with no userinfo. That is host confusion through Unicode normalisation (the HostSplit pattern): the validator approves a URL after parsing a structure the URL does not have, and user:pass@hôst.com is likewise checked as the unrelated host hst-zlb.com.

The fix stays in the regex: the host group of AUTHORITY_REGEX now admits non-ASCII characters, so the raw authority is split first and the whole-authority conversion goes away, since DomainValidator.isValid already converts the host it is handed. An all-ASCII authority matches exactly as before, and the format code point, boundary hyphen and port range checks still run on the userinfo and port paths. It belongs in isValidAuthority because that is the only place that knows where the host starts and ends; a caller cannot undo a split made on the wrong string. testIdnAuthority fails on master at its first assertion and passes with the change, and the default mvn goal with -Ddoclint=all is green (3300 tests, Checkstyle, PMD, SpotBugs, japicmp and RAT clean). The comment on unicodeToASCII is corrected because UrlValidator no longer calls it.

isValidAuthority converted the whole authority with IDN.toASCII and split the result. Nameprep folds the fullwidth '@' and ':' to ASCII and punycode encodes a whole label, so the conversion invented delimiters that are not in the URL and pulled the userinfo or port into the host label. Match the raw authority instead and let DomainValidator convert the host alone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant