Reject leading and trailing label hyphens in DomainValidator unicodeToASCII#424
Merged
garydgregory merged 1 commit intoJul 17, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Closes VALIDATOR-501 by making DomainValidator.unicodeToASCII reject domains where any label starts or ends with - even when the label contains non-ASCII characters that would otherwise be punycode-encoded into an ASCII form that slips past the existing label regex checks. This aligns IDN-handling behavior with the existing all-ASCII validation path and fixes the shared behavior for DomainValidator, UrlValidator, and the email domain validation that rely on unicodeToASCII.
Changes:
- Enable and expand the previously
@Disabledregression test for VALIDATOR-501. - Add a pre-conversion scan in
unicodeToASCIIto detect leading/trailing hyphens at label boundaries (using RFC 3490 dot separators) and return the original input so existing regex validation rejects it.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/java/org/apache/commons/validator/routines/DomainValidator.java | Adds label-boundary hyphen detection to prevent IDN punycode from bypassing LDH-style label rules in downstream regex validation. |
| src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java | Enables and extends a regression test to ensure non-ASCII labels with boundary hyphens are rejected while interior hyphens remain valid. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
DomainValidator.isValid accepts a non-ASCII domain label that begins or ends with a hyphen: isValid("-tést.fr") and isValid("tést-.fr") both return true, though the all-ASCII forms "-test.fr" and "test-.fr" are correctly rejected. This is VALIDATOR-501, whose DomainValidatorTest.testInvalidDomains501 has been sitting @disabled. The divergence is in the shared unicodeToASCII helper: for a label carrying a non-ASCII character it defers to IDN.toASCII, which with the default flags does not apply the LDH rule and punycode-encodes the boundary hyphen ("-tést" becomes "xn---tst-cpa"), so the converted label starts and ends alphanumeric, DOMAIN_LABEL_REGEX matches, and the hyphen the ASCII path would have caught slips through. UrlValidator.isValidAuthority and the EmailValidator domain check share unicodeToASCII, so they inherit the same hole.
The fix scans the original input before conversion and, when any label opens or closes with a hyphen, returns it unchanged so the label regex rejects it, matching the format-code-point guard added a few lines above. It splits on the four label separators from RFC 3490 section 3.1 and leaves an interior hyphen alone, so "a-é.fr" keeps validating while "-é.fr" does not. Keeping the check in the shared helper closes the gap for DomainValidator, UrlValidator and EmailValidator in one place rather than in each caller. I enabled testInvalidDomains501 and added the "-é.fr"/"a-é.fr" pair; the full suite stays green.
mvn; that'smvnon the command line by itself.