Skip to content

Reject over-long hostnames and invalid A-labels in the hostname format - #1377

Open
ravismash wants to merge 14 commits into
networknt:masterfrom
ravismash:fix/hostname-format
Open

ravismash wants to merge 14 commits into
networknt:masterfrom
ravismash:fix/hostname-format

Conversation

@ravismash

@ravismash ravismash commented Nov 4, 2026 •

Copy link
Copy Markdown

Reject over-long hostnames and invalid A-labels in the hostname format

The hostname format was a single LDH regex, so it accepted names longer than 253 characters and any xn-- label, including invalid Punycode such as xn--X. RFC 1123 hostnames that use the xn-- prefix must be valid A-labels (RFC 5890, section 2.3.2.1).

hostname

  • New HostnameFormat: the existing LDH rule, a 253-character limit, and an A-label check for labels that start with xn-- in any case.
  • An A-label must be at most 63 characters of valid Punycode that encodes back to itself, and its U-label must pass the RFC 5891 hyphen, leading combining mark, CONTEXTJ/CONTEXTO, bidi and disallowed-character rules. The upstream suite's A-label group needs these.

hostname and idn-hostname

  • A-labels are decoded with a small RFC 3492 Punycode class instead of java.net.IDN, which follows IDNA2003 and rejects labels IDNA2008 allows in context (U+00DF, U+03C2, ZWJ/ZWNJ after a virama).
  • A code point that the bundled Unicode 5.2 Appendix B table lists as unassigned is classified with the JDK's Unicode data, approximating the RFC 5892 derivation: only Ll, Lm, Lo, Mn, Mc and Nd code points that NFKC and lower-casing leave unchanged are accepted. Newer letters such as Adlam pass; emoji (xn--ls8h.la), symbols, upper case and bidi controls do not.
  • ZWJ and ZWNJ are accepted after any virama (Canonical_Combining_Class 9), the hyphen-position rule counts code points, and the lazily loaded Unicode tables are published safely across threads.
  • The IDNA rule loop now steps by code point. It used to test supplementary-plane characters as lone surrogates and reject them.

idn-hostname

  • The 63-character limit is measured in A-label form for U-labels too (Punycode.encode instead of IDN.toASCII), and names are limited to 253 characters in A-label form. Label length is checked before the quadratic decode.

Tests

  • PunycodeTest (RFC 3492 section 7.1 samples, overflow, truncated input, a delimiter at index 0), RFC5892Test and HostnameFormatTest (253/254, 63/64, trailing dot, upper case).
  • hostname.json (draft 4 to 2020-12) and idn-hostname.json (draft 7 to 2020-12) refreshed from the current JSON-Schema-Test-Suite, plus draft-next from 2020-12. The refreshed idn-hostname.json adds 12 cases this validator didn't support before either: UTS 46 mapping, trailing separators and RFC 5893 digit rules. They are marked disabled with a reason, for a follow-up PR.

mvn -B test passes locally.

java.net.IDN implements IDNA2003, so it rejects A-labels whose U-label has
code points that IDNA2008 allows in context (U+00DF, U+03C2, U+200C, U+200D).
Decode the Punycode directly, require it to round-trip, and leave the
code points to the RFC 5892 rules.
@stevehu

stevehu commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Review of this PR. I ran a scratch probe on the PR branch for the behavioural points. The length limit and the Punycode decode of invalid A-labels work as intended, but there are a few issues.

1. Valid hostnames are now rejected (HostnameFormat.java:32)
Every xn-- label must now pass the IDNA2008 rules in RFC5892.isValid. Results on the PR branch:

  • xn--ls8h.la (a registered emoji domain) now gives hostname=false. RFC 5892 disallows symbols.
  • A-labels built from U+1F600 and from U+1E900 (Adlam, Unicode 9) are also rejected, because ucd/RFC5892-appendix-B.txt is the old Appendix B table and later code points count as UNASSIGNED.

hostname is the RFC 1123 LDH check, and these names were valid before. The CHANGELOG doesn't mention this. If the stricter policy is intended, it should be called out. Otherwise hostname could limit itself to the Punycode round trip and the structural checks the test suite requires.

2. Case sensitivity (RFC5892.java:181)
xn--bcher-kva.example is valid, but XN--BCHER-KVA.EXAMPLE (and xn--Bcher-kva.example) is rejected. The basic code points are copied verbatim into the U-label, so upper-case letters hit isDisallowed. Lower-casing the label (Locale.ROOT) before decoding would fix it.

3. idn-hostname U-label and A-label disagree (RFC5892.java:160)
The U-label branch still uses IDNA2003 IDN.toASCII for the 63-character check. "ß".repeat(40) as a U-label is rejected (the ACE is 80 characters), while its real A-label (46 characters) is accepted. Computing the length from Punycode.encode in the U-label branch would make them consistent.

4. Tests

  • There are no Java unit tests for Punycode (RFC 3492 section 7.1 vectors, overflow guards, truncated input, delimiter at index 0). There are none for HostnameFormat boundaries (253/254, trailing dot, upper case) or for the idn-hostname change either.
  • idn-hostname.json wasn't refreshed even though the PR says xn--zca29lwxobi7a and xn--11b2ezcw70k are fixed under it.
  • draft-next/optional/format/hostname.json still has 26 cases, against 68 in 2020-12.

5. Minor

  • The cheap 63 < label.length() check runs after decode, re-encode, normalization and the rules. Decode is O(n^2), and idn-hostname has no overall length cap. A roughly 220 KB label took about 356 ms. Check the length first.
  • Punycode is a new public class but only RFC5892 needs it, so package-private would avoid new API surface. The length == output.length grow branch in decode is dead, since decoded length never exceeds input length.

🤖 Generated with Claude Code

…ints

- hostname: check A-labels with RFC5892.isValidALabel, which does not reject
  code points that the bundled Unicode 5.2 table lists as unassigned when the
  JDK assigns them (e.g. xn--ls8h.la, U+1F600, Adlam)
- Lower-case A-labels before decoding, so XN--BCHER-KVA and xn--Bcher-kva pass
- idn-hostname: measure the 63-character limit with Punycode.encode instead of
  IDN.toASCII, and limit names to 253 characters in A-label form
- Check label length before the quadratic Punycode decode
- Test IDNA rules once per code point instead of once per char
- Make Punycode package-private and drop the unreachable grow branch
- Add PunycodeTest, RFC5892Test and HostnameFormatTest
- Refresh idn-hostname.json, and draft-next hostname.json and idn-hostname.json
…rt yet

The current suite adds idn-hostname cases for UTS 46 mapping, trailing label
separators and the RFC 5893 digit rules. These are existing gaps, not
regressions, so mark them disabled with a reason instead of widening this PR.
hostname accepted xn--ls8h.la while idn-hostname rejected it, because only
hostname ignored the Unicode 5.2 table's UNASSIGNED entries. Use one rule set
for both, so idn-hostname accepts every name hostname accepts.
@ravismash

Copy link
Copy Markdown
Author

Thanks for the careful review. I pushed a commit that addresses each point:

  1. Valid hostnames rejected. hostname now checks A-labels with a separate RFC5892.isValidALabel. It still applies the hyphen, leading combining mark, CONTEXTJ/CONTEXTO, bidi and DISALLOWED rules, because the upstream suite's A-label group needs them (xn--hello-txk, xn--07jt112bpxg, the virama and middle-dot cases). It no longer rejects a code point that the Unicode 5.2 Appendix B table lists as unassigned when the JDK assigns it. So xn--ls8h.la, U+1F600 (xn--e28h) and Adlam (xn--bd6h, xn--9d6hc) pass. The CHANGELOG now spells out the stricter policy against plain LDH matching. idn-hostname keeps the strict table.
  2. Case. A-labels are lower-cased (ASCII, Locale.ROOT) before decoding, and the round trip compares against the lower-cased form. XN--BCHER-KVA.EXAMPLE and xn--Bcher-kva.example pass.
  3. U-label vs A-label length. The U-label branch now measures "xn--" + Punycode.encode(label), and IDN.toASCII is gone. "ß".repeat(40) passes in both forms. Names are also limited to 253 characters in A-label form.
  4. Tests. I added PunycodeTest (RFC 3492 section 7.1 samples A, B, L, R and S, overflow, truncated input, a delimiter at index 0 and non-basic code points), RFC5892Test (idn-hostname lengths, case, round trip and the hostname leniency) and HostnameFormatTest (253/254, 63/64, trailing dot, upper case). idn-hostname.json is refreshed for draft 7, 2019-09 and 2020-12. The refreshed file adds 12 cases that idn-hostname didn't support before this PR either: UTS 46 mapping (fullwidth characters, zero width space, non-NFC input), trailing label separators, and the RFC 5893 digit rules. I marked those disabled with a reason rather than widening this PR. I'm happy to take them in a follow-up. I also refreshed draft-next hostname and idn-hostname from 2020-12, with $schema swapped to draft/next. Note that JsonSchemaTestSuiteTest doesn't run draft-next, and upstream has since renamed it to v1.
  5. Minor. Label length is checked before decoding in both formats. Punycode is package-private, and the unreachable grow branch is removed.

While working on this, I found that the IDNA rule loop stepped by char, so any supplementary-plane code point was tested as a lone low surrogate and rejected. It now steps by code point.


Generated by Claude Code

@ravismash

Copy link
Copy Markdown
Author

One more change after my last reply. I said idn-hostname would keep the strict Unicode 5.2 table, but that left hostname accepting xn--ls8h.la while idn-hostname rejected it. The latest commit applies the same policy to both: a code point the table lists as unassigned is accepted when the JDK assigns it. idn-hostname now accepts every name hostname accepts. I also updated the PR description to match the current state.


Generated by Claude Code

@stevehu

stevehu commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Review of the latest changes (head dd60657). The full suite passes locally (10,892 run, 0 failures, 66 skipped). Findings 1-3 were reproduced with probes against this branch and against master.

1. Character.isDefined bypass accepts post-5.2 symbols, emoji and bidi controls (RFC5892.java:92)

!isUnassigned(c) || Character.isDefined(c) lets through every code point the Unicode 5.2 table lists as unassigned once the JDK defines it, whatever its category. Master's idn-hostname rejects U+1F600, a+U+20BF, U+1C90 and 1+U+2066+1 (LRI between digits). This branch accepts all of them, and hostname accepts the A-label forms (xn--e28h, xn--a-0qn, xn--j4f, xn--11-88t). IDNA2008 disallows these, and the result now depends on the JDK's Unicode version. No upstream suite case requires it, and RFC5892Test.allowsCodePointsNewerThanTheBundledTable and HostnameFormatTest.accepts pin it. Suggested fix: refresh the bundled table, or allow newly assigned code points only when their General_Category is Ll/Lo/Lm/Mn/Mc/Nd.

2. hostname now rejects valid non-Devanagari A-labels (RFC5892.java:384)

The ZWJ/ZWNJ rules only accept a preceding U+094D (VIRAMA = 0x94D), not any Canonical_Combining_Class=Virama character as RFC 5892 A.1/A.2 require. xn--3zc0eybz95j (Sinhala), xn--bwca1om25e (Malayalam) and xn--p5ba1o498f (Bengali) now return false from hostname, which accepted them before. Only the Devanagari xn--11b2ezcw70k passes.

3. Hyphen-position check indexes UTF-16 chars, not code points (RFC5892.java:173)

U+20000 U+20001 - - a is accepted by idn-hostname, and its A-label by hostname. The BMP equivalent (U+4E00 U+4E01 --a) is rejected. This is reachable from hostname for the first time.

4. Lazy table load is not thread-safe (RFC5892.java:76; not run, from reading the code)

isDisallowed and the other table readers check BitSet.isEmpty() outside the synchronized loader, so a second thread can read half-loaded tables. This pre-dates the PR, but hostname now reaches it.

5. "idn-hostname accepts what hostname accepts" does not hold (RFC5892Test.java:92)

Example.COM and ab--cd.example are valid hostname values that RFC5892.isValid rejects. example.com. and example.com.. go the other way. Please add a cross-format test or correct the comment and commit message.

Minor

  • HostnameFormat.java:32: split plus a per-label prefix check runs on every value, even when there is no xn-- label.
  • RFC5892.java:218: isAscii duplicates AbsoluteIris.isAscii (different DEL boundary) and allocates a stream per call.

… hyphens

- Accept a code point the Unicode 5.2 table lists as unassigned only if the
  JDK classifies it as Ll, Lm, Lo, Mn, Mc or Nd and NFKC and lower-casing
  leave it unchanged, so emoji, symbols, upper case and bidi controls added
  since are rejected (an approximation of the RFC 5892 derivation)
- Accept ZWJ and ZWNJ after any Canonical_Combining_Class Virama code point,
  not only U+094D, and read the preceding code point, not char
- Count the RFC 5891 4.2.3.1 hyphen positions in code points
- Apply RFC 5893 rules 1, 3 and 4 to any label with an R, AL or AN character
- Reject Cherokee small letters, which case folding maps to the capitals
- Publish the lazily loaded RFC 5892 and joining-type tables with a volatile
  flag instead of checking BitSet.isEmpty() outside the lock
- Walk hostname labels without splitting, and check ASCII without a stream
- Replace the incorrect "idn-hostname accepts everything hostname accepts"
  test with tests for the new rules
@ravismash

Copy link
Copy Markdown
Author

Thanks again, these were all real. The new commit addresses each one:

  1. New code points. A code point the Unicode 5.2 table lists as unassigned is now classified with the JDK's Unicode data, approximating the RFC 5892 derivation. It is accepted only if it is Ll, Lm, Lo, Mn, Mc or Nd and NFKC plus lower-casing leave it unchanged (the LetterDigits and Unstable rules). So xn--e28h, xn--a-0qn, xn--j4f, xn--11-88t and xn--ls8h.la are rejected, and newer letters such as Adlam small letters (xn--9d6hc) still pass. The tests now pin both sides. Refreshing the Appendix B table would be the exact fix. I kept this PR to the approximation, but I'm happy to do the refresh separately if you'd prefer.
  2. Virama. ZWJ and ZWNJ are accepted after any code point with Canonical_Combining_Class Virama (a sorted table of the 65 code points, Unicode 15.1). testZeroWidthJoiner now reads the preceding code point, not the preceding char. xn--3zc0eybz95j, xn--bwca1om25e and xn--p5ba1o498f pass.
  3. Hyphen positions are counted in code points, so U+20000 U+20001 --a is rejected like U+4E00 U+4E01 --a.
  4. Thread safety. The RFC 5892 tables, and the joining-type tables in UnicodeDatabase that had the same pattern, are now published with a volatile flag that is set after loading, inside the lock.
  5. The cross-format claim. You're right, it was wrong. I removed it and replaced that test with tests for the rules above. The commit message on dd60657 overstates it too. The remaining differences: example.com. is a trailing separator that idn-hostname still accepts, and Example.COM needs the UTS 46 mapping. Both are in the disabled suite cases I'd like to fix in a follow-up. ab--cd is an R-LDH label, which RFC 5890 reserves, so I left idn-hostname rejecting it.

I also checked the whole path against Python's idna library (an independent IDNA2008 implementation, Unicode 18 tables) on about 2,600 generated labels. They cover every virama with ZWJ and ZWNJ, hyphen positions, the context rules, Bidi, upper case, and code points from the BMP and the supplementary planes. Two more fixes came out of that. A label with an R, AL or AN character now has to satisfy RFC 5893 rules 1, 3 and 4 (1א was accepted). And Cherokee small letters (Unicode 8) are now rejected, because case folding maps them to the capitals, which stay PVALID. The only remaining difference from idna is a。: idn-hostname still accepts a trailing halfwidth full stop, which is one of the disabled suite cases for the follow-up.

Minor: HostnameFormat now walks the labels with indexOf and only builds a substring for an xn-- label. RFC5892.isAscii is now a plain loop. I kept it separate from AbsoluteIris.isAscii because that one treats DEL differently.


Generated by Claude Code

Found by comparing against Python idna 3.20 on every assigned code point:
- KATAKANA MIDDLE DOT needs a Hiragana, Katakana or Han character
  anywhere in the label (RFC 5892 A.7), not only right after it.
- Greek, Hebrew and Kana/Han checks use Character.UnicodeScript, not
  code blocks (Coptic in the Greek block, U+30FC is Common).
- The ZWNJ joining-type scan steps over supplementary transparent
  marks by code point instead of by UTF-16 char.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U6XXMPvkRmkr5kqQuxPXm4
Found by comparing against Python idna 3.20 on every assigned code point:
- U+0CF1 and U+0CF2 are PVALID and U+19DA is DISALLOWED since Unicode
  6.0 (RFC 6452), unlike the bundled Unicode 5.2 table.
- New letters that case folding maps to another letter, such as
  U+1C80..U+1C88, are rejected, using lower(upper(c)) as the fold.
- U+180F, a default-ignorable mark, is rejected.
- New Cherokee capitals such as U+13F5 are accepted.

The derivation now matches the idna PVALID table for all 41,890 code
points assigned after Unicode 5.2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U6XXMPvkRmkr5kqQuxPXm4
@ravismash

ravismash commented Nov 4, 2026 •

Copy link
Copy Markdown
Author

Two more commits after a wider check. I compared the validator against Python idna 3.20 on about 460,000 labels: every assigned Unicode 15.0 code point (alone, before and after an ASCII letter), every virama with ZWJ/ZWNJ, every joining-type pair around ZWNJ, the CONTEXTO characters next to several scripts, Bidi class pairs, and hyphen and length boundaries.

It found three issues in the existing CONTEXTO/CONTEXTJ code that hostname A-labels now go through:

  • KATAKANA MIDDLE DOT required the next character to be Kana or Han. RFC 5892 A.7 accepts a Hiragana, Katakana or Han character anywhere in the label, so 一・ (xn--vek768f) is now valid.
  • The Greek, Hebrew and Kana/Han checks used code blocks, not scripts. Coptic letters in the Greek block passed after U+0375, and U+30FC (Common) passed for the middle dot. They now use Character.UnicodeScript.
  • The ZWNJ joining-type scan stepped back one UTF-16 char at a time, so a supplementary transparent mark (e.g. Adlam U+1E944) broke the check.

It also found four gaps in how code points outside the Unicode 5.2 table are classified:

  • RFC 6452: Unicode 6.0 made U+0CF1 and U+0CF2 PVALID and U+19DA DISALLOWED, and IDNA2008 kept those values, so they now override the bundled table.
  • New letters that case folding maps to another letter (U+1C80..U+1C88, Cyrillic rounded/tall forms) are now rejected.
  • U+180F (a default-ignorable Mongolian variation selector) is now rejected.
  • New Cherokee capitals such as U+13F5 are now accepted.

With these, the derivation matches the idna PVALID table for all 41,890 code points assigned after Unicode 5.2, and the only differences left are the trailing-separator and UTS 46 cases already disabled in the suite. Each fix has a unit test. The comparison harness is local only and not part of the PR.

Ready for another look when you have time.


Generated by Claude Code

@stevehu

stevehu commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Review of head de45715 (the three commits since dd60657). The earlier points are fixed: volatile publication of the tables, virama by code point, hyphen position by code point, the RFC 6452 overrides, and the Cherokee and U+1C80 handling. I probed the branch directly. Ranked most-severe first.

1. Bidi rule is applied per label, not to the whole name (RFC5892.java:255)
RFC 5893 §2 applies the rule to every label of a name that contains an RTL label. 1a.אב, אב.1, אב.a1 and אב.example1 are all accepted by idn-hostname, but 1a and 1 start with EN, so rule 1 should reject them. The upstream cases 0a.א and 0א are still disabled. The CHANGELOG does not say the rule is label-only.

2. Newer code points are judged by the running JDK's Unicode version (RFC5892.java:125)
Code points the bundled 5.2 table lists as unassigned are classified from the JDK's data plus three hand-picked exceptions (U+0CF1/0CF2/19DA, Cherokee, U+180F). The same name can be valid on JDK 21/25 and invalid on JDK 17, and each review round adds another special case. The root fix is regenerating RFC5892-appendix-B.txt from the current IANA IDNA table, which you offered to do separately.

3. Virama table is fixed at Unicode 15.1 (RFC5892.java:35)
Comparing it with JDK 25's ccc=9 code points shows four missing: U+113CE, U+113CF, U+113D0 (Tulu-Tigalari) and U+1612F (Gurung Khema). The characters are accepted but a ZWJ/ZWNJ after one is rejected.

4. Test gaps (RFC5892Test.java:198)
assertFalse(isValid('a', 0x180F)) passes vacuously on a JDK whose Unicode data predates 14.0, because the code point is unassigned there. No test covers concurrent first use of the lazily loaded tables, so the thread-safety fix can regress unnoticed.

5. Dead code in UnicodeDatabase.java:61
isGreek, isHebrew, isKatakana and their block-based BitSets have no callers now that the rules use UnicodeScript. The public methods keep the block-versus-script mistake this PR just fixed.

6. Hyphen check allocates per label (RFC5892.java:227)
unicode.codePoints().limit(4).toArray() builds a stream and an array for every label, including all-ASCII ones, where the old charAt test allocated nothing.

7. CHANGELOG (CHANGELOG.md:11)
One bullet of about 3,000 characters covers both formats and omits the limits above (label-only Bidi, JDK-dependent code points, virama list stops at 15.1).

- RFC 5893: once a label has an R, AL or AN character, every label of
  the name must satisfy the Bidi Rule, in idn-hostname and hostname.
  Re-enables the three suite cases for rules 1 and 4.
- Virama list updated to Unicode 16.0 (Tulu-Tigalari, Gurung Khema).
- Hyphen-position check no longer allocates.
- Removes the unused block-based isGreek/isHebrew/isKatakana.
- Tests: concurrent first use of the tables in a fresh class loader,
  name-wide Bidi, Unicode 16 virama; U+180F is skipped on JDKs that
  do not know it.
- CHANGELOG split per change, with the known limits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U6XXMPvkRmkr5kqQuxPXm4
The Unicode 16 additions were appended out of order, so the binary search
missed U+11D45 and U+11D97. Adds a test that the table is sorted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U6XXMPvkRmkr5kqQuxPXm4
@ravismash

ravismash commented Nov 5, 2026 •

Copy link
Copy Markdown
Author

Thanks for probing it directly. Pushed a commit for this round:

  1. Bidi across the name. Fixed. Once any label has an R, AL or AN character, every label must satisfy RFC 5893, in both idn-hostname and hostname. 1a.אב, אב.1 and 0a.א are now rejected, and the three disabled rule 1/rule 4 suite cases are enabled again. אב.a1 and אב.example1 stay valid: rule 1 holds (they start with L) and rule 6 lets an LTR label end with EN.
  2. JDK-dependent code points. Agreed that regenerating RFC5892-appendix-B.txt from a current IANA table is the root fix, and I'll send it as a separate PR so this one doesn't grow further. Once that's in, the derivation and its special cases can go. For now the CHANGELOG states the JDK dependence as a known limit.
  3. Viramas. Added U+113CE, U+113CF, U+113D0 and U+1612F, so the list is now Unicode 16.0, plus a test (skipped on JDKs without Unicode 16).
  4. Tests. The U+180F test now assumes Character.isDefined(0x180F). A new test loads fresh copies of the utils classes in a child-first class loader for 20 rounds and has 8 threads hit isValid at once, using both the IDNA and the joining-type tables.
  5. Dead code. Removed isGreek, isHebrew, isKatakana and their BitSets.
  6. Hyphen check. It uses offsetByCodePoints and charAt now, with no allocation.
  7. CHANGELOG. Split into one bullet per change, with a "Known limits" entry: JDK-dependent verdicts, the virama list at 16.0, and no UTS 46 mapping or trailing-separator rejection.
    Ready for another look when you have time.

@stevehu

stevehu commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Review of head df07704 (the two commits since de45715). All seven earlier points are addressed. I ran the PR's tests (pass), checked the virama table against JDK 25's Unicode 16 data (exact match), and confirmed the concurrency test fails 3/3 without synchronized. Ranked most-severe first.

1. hostname now rejects RFC 1123 names with a digit-first label beside an RTL A-label (HostnameFormat.java:45)
Probed: example.xn--mgbaam7a8h and a1.xn--4dbc pass, but 3com.xn--mgbaam7a8h, 123.xn--mgbaam7a8h and 1a.xn--4dbc fail. They are valid hostnames and pass on 3.0.9. Rule 1 of RFC 5893 is being applied to plain ASCII LDH labels in a format that is RFC 1123, not IDNA2008.

2. satisfiesBidiDomainRule throws on bad input (RFC5892.java:212)
{"", "xn--4dbc"} throws StringIndexOutOfBoundsException and {null, "xn--4dbc"} throws NPE. HostnameFormat is safe only because its regex rejects empty labels first. The Javadoc states no precondition.

3. No HostnameFormat-level test for the new Bidi branch (RFC5892Test.java:245)
Only direct RFC5892 calls cover it; HostnameFormatTest is unchanged.

4. Virama table content is untested (RFC5892Test.java:221)
Only sort order is checked, and the Unicode 16 test skips on Java 17 and 21. The ccc=9 set can be derived from Normalizer reordering on any JDK.

5. CHANGELOG omits the stricter hostname behaviour (CHANGELOG.md:19)
The Bidi bullet's examples are idn-hostname only; nothing says ASCII hostname values can now fail.

6. A-labels are decoded twice per hostname (HostnameFormat.java:39)
isValidALabel then satisfiesBidiDomainRule each decode and re-encode, and split allocates despite the "no allocation" comment. RFC5892.java:284 is also redundant now.

7. Altitude: split Bidi-domain logic behind a public half-validated API (RFC5892.java:212)
A single RFC5892.isValidHostname(String) would walk once and keep the rule in one place.

8. Hyphen check can be one startsWith (RFC5892.java:254)
unicode.startsWith("--", unicode.offsetByCodePoints(0, 2)) behind a two-code-point guard expresses RFC 5891 4.2.3.1 directly.

- hostname applies the RFC 5893 Bidi Rule within each A-label only, so
  3com.xn--mgbaam7a8h and 1a.xn--4dbc are valid again, as on 3.0.9.
- Remove the public satisfiesBidiDomainRule. The cross-label rule stays
  private to idn-hostname, which checks only the labels without
  right-to-left characters, since isValidULabel already checked the rest.
- HostnameFormat walks the labels once, with no split or second decode.
- The hyphen check is one startsWith at the third code point.
- Test the virama table against the JDK's combining classes, derived from
  NFD reordering, on any JDK.
- HostnameFormat tests for RTL A-labels; CHANGELOG states the stricter
  hostname behaviour and which format applies the cross-label rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U6XXMPvkRmkr5kqQuxPXm4
@ravismash

Copy link
Copy Markdown
Author

Thanks for the careful pass. Commit 954f45a addresses all eight points.

  1. Regression for digit-first labels. You're right: hostname is RFC 1123, and applying RFC 5893 rule 1 to plain LDH labels was wrong. hostname now applies the Bidi Rule within each A-label only, so 3com.xn--mgbaam7a8h, 123.xn--mgbaam7a8h and 1a.xn--4dbc pass again, as on 3.0.9. The cross-label rule stays in idn-hostname, where it belongs.
  2. satisfiesBidiDomainRule throws. Removed. It is no longer needed once hostname stops applying the cross-label rule.
  3. No HostnameFormat test for the Bidi branch. Added HostnameFormatTest cases: the five names from your probe, plus two RTL A-labels together (accepted), and xn--1-0hc.example (rejected, since 1א breaks rule 1 within the A-label).
  4. Virama table content. New test viramaTableMatchesTheCombiningClassesOfTheRunningJdk derives the ccc=9 set from NFD reordering. X has class 9 exactly when X U+3099 (ccc 8) and U+05B0 X (ccc 10) both reorder. Up to Java 25 it must equal the table's defined entries; on a later JDK every entry must still be ccc 9. I checked locally on Java 21: 65 defined entries, exact match.
  5. CHANGELOG. It now says hostname rejects A-labels that break the Bidi Rule (e.g. xn--1-0hc), that other labels stay RFC 1123 LDH labels, and that the cross-label rule applies to idn-hostname only.
  6. Double decode / split / redundancy. HostnameFormat walks the labels once, with no split, and decodes each A-label once. In idn-hostname, the name-level pass now checks only the labels without RTL characters. The others were already checked in isValidULabel, so line 284 is no longer redundant.
  7. Altitude. With (1) and (2), no half-validated public API is left. hostname uses isValidALabel, and the whole-name rule is private inside RFC5892.isValid.
  8. Hyphen check. Now 4 <= len && unicode.startsWith("--", unicode.offsetByCodePoints(0, 2)). Four chars always hold at least two code points, so the offset exists.

The differential probe against Python idna 3.20 still shows 0 mismatches on Java 24. That covers 462,802 single code points, 400k fuzz labels and 180k names. For names, hostname is now checked against its own expectation (Bidi within A-labels only); 7,684 of those names differ between the two formats. Full suite: 10,919 tests, 0 failures, 57 skipped (unchanged from the last round).

@stevehu

stevehu commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Review of head 954f45a (the commit since df07704). All eight earlier points are addressed: hostname stays RFC 1123 with the Bidi Rule inside each A-label only, satisfiesBidiDomainRule is gone, each A-label is decoded once, and the hyphen check is a single startsWith. I ran RFC5892Test, HostnameFormatTest, PunycodeTest, FormatValidatorTest and the suite (all pass) and probed the A-label cases. I did not run the full suite. Only minor points remain, ranked most-severe first.

1. CHANGELOG example (CHANGELOG.md:19)
0ا is described as invalid "in both formats", but hostname rejects it because the LDH regex refuses non-ASCII, not because of the Bidi Rule. For the hostname half, use an A-label such as xn--1-0hc.

2. Virama test cutoff (RFC5892Test.java:229)
Runtime.version().feature() <= 25 is hard-coded, and on a later JDK the test only checks containsAll, so a missing newer virama passes silently. It also scans only Mn/Mc code points, so a ccc=9 code point of another category would never be derived.

3. hostname Bidi coverage is one case (HostnameFormatTest.java:76)
Only rule 1 (xn--1-0hc) is tested at the format level. xn--0-zhc74b (rule 4) and xn--a-0hc (rule 5) are rejected today but not pinned.

4. Double scan and allocation (RFC5892.java:201)
isValid calls hasRightToLeft on each label right after isValidULabel already did, and always allocates uLabels and keeps every non-RTL label, even when the name is never a Bidi domain name.

5. Test duplication and style (RFC5892Test.java:229)
The new test repeats the reflective VIRAMAS read, uses inline java.util.Set/TreeSet/reflect.Field names although other java.util types are imported, and the java.text.Normalizer import is out of order.

- idn-hostname checks each label once: checkULabel reports whether a
  valid label is right-to-left, so isValid needs no second scan and no
  label array. Earlier labels are re-checked only when a name turns out
  to be a Bidi domain name.
- The virama test scans every defined code point and requires an exact
  match with the JDK's combining classes, with no JDK version cutoff.
- Pin RFC 5893 rules 4 and 5 for hostname A-labels.
- CHANGELOG: use an A-label as the hostname Bidi example.
- Share the reflective VIRAMAS read and tidy the test imports.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U6XXMPvkRmkr5kqQuxPXm4
@ravismash

Copy link
Copy Markdown
Author

Thanks. Commit cf8e08e covers all five.

  1. CHANGELOG example. Agreed, 0ا fails hostname on the LDH pattern, not on the Bidi Rule. The line now gives 0ا as the idn-hostname example and xn--1-0hc (1א) as the hostname one.
  2. Virama test cutoff. The version cutoff and the containsAll branch are gone. The test now scans every defined code point, not just Mn/Mc, and requires an exact match. A JDK whose Unicode data adds a virama fails the test, which is the signal to extend VIRAMAS. CI runs 17, 21 and 25, all at or below Unicode 16, so it is green today. The full scan takes well under a second.
  3. hostname Bidi coverage. Added xn--0-zhc74b (rule 4, א0٠) and xn--a-0hc (rule 5, aא) to HostnameFormatTest's rejects, next to xn--1-0hc (rule 1).
  4. Double scan and allocation. isValidULabel became checkULabel. It returns invalid, left-to-right or right-to-left, so isValid no longer calls hasRightToLeft again and allocates no label array. When the first right-to-left label appears, the labels before it are checked against the Bidi Rule once. After that, each later label is checked as it is reached. Names without right-to-left labels do no extra work.
  5. Test style. One viramas() helper serves both tests. Field, Set and TreeSet are imported, and the imports are in order.

Full suite: 10,921 tests, 0 failures. The Python idna differential probe still shows 0 mismatches on Java 24, across 1,042,802 labels and names.

@stevehu

stevehu commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Review of head cf8e08e (the commit since 954f45a). All five earlier points are addressed. I ran RFC5892Test, HostnameFormatTest, PunycodeTest, FormatValidatorTest and the suite (all pass), and compared the old and new RFC5892.isValid on 400,000 random names (about 118,000 valid): no differences. Only minor points remain.

1. Virama test fails hard on any newer-Unicode JDK (RFC5892Test.java:248)
assertEquals(derived, table) now runs on every JDK, so one that ships Unicode data newer than 16.0 and defines a new virama turns this test red in unrelated builds. Last round it was too lenient; this is the opposite extreme. Failing only when the JDK's data is not newer than the table, or logging the missing code points, would still reach maintainers without blocking other work.

2. Status constants are misnamed (RFC5892.java:233)
LEFT_TO_RIGHT is returned for any label without R/AL/AN characters, including digit-first labels like 1a. The constants are declared mid-class, and the @return Javadoc reads as a fragment. NO_RTL/HAS_RTL, or a small enum, says what is actually known.

3. Earlier A-labels are decoded twice (RFC5892.java:209)
When the first right-to-left label comes after other labels, each earlier A-label goes through toUnicode again (a Punycode decode plus an encode round trip). The isACE(label) ? toUnicode(label) : label expression is also duplicated from the loop head.

- The virama test always fails on a code point the table wrongly lists.
  A missing virama fails only when the JDK's Unicode data is not newer
  than the table's 16.0; on a newer JDK it is logged instead.
- checkULabel returns a LabelStatus enum (INVALID, NO_RTL, HAS_RTL),
  declared with the other constants.
- Keep the U-labels of A-labels, allocated only when a name has one, so
  an earlier A-label is not decoded again when the first right-to-left
  label appears.
- Test the Punycode decoder's surrogate and out-of-range branches.
- CHANGELOG: only Adlam small letters are accepted, not the capitals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U6XXMPvkRmkr5kqQuxPXm4
@ravismash

Copy link
Copy Markdown
Author

Thanks. Commit 263c44b covers all three.

  1. Virama test on newer JDKs. A code point the table lists but the JDK does not give ccc 9 still fails on every JDK, because that is a real error in the table. A missing virama fails only when the JDK's Unicode data is not newer than the table's 16.0. To tell, the test looks for any UnicodeScript constant after the seven scripts added in 16.0: the JDK appends new scripts before UNKNOWN. On a newer JDK, the missing code points are logged as a warning instead of failing unrelated builds.
  2. Status constants. checkULabel now returns a small LabelStatus enum (INVALID, NO_RTL, HAS_RTL), declared with the other constants, and the Javadoc says what each value means.
  3. Earlier A-labels decoded twice. isValid keeps the U-label of each A-label in an array that it allocates only when the name contains an A-label, so all-ASCII and pure U-label names allocate nothing. When the first right-to-left label appears, the earlier labels are read from that array instead of being decoded again, and the duplicated isACE ? toUnicode : label expression is gone. New asserts cover an earlier A-label that passes (xn--bcher-kva.xn--4dbc) and one that fails (a.xn--1-eha.xn--4dbc, since 1ü starts with a digit).

While I was there I also went over the whole diff again. I added tests for the two Punycode decoder branches that had none: a lone surrogate (ib9b) and a code point above U+10FFFF (bb00h). The CHANGELOG now says Adlam small letters, since the capitals are rejected as upper case. I also moved the test's script table to the top of the class.

@stevehu

stevehu commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Review of head 263c44b (the commit since cf8e08e). All three earlier points are addressed. I ran RFC5892Test, HostnameFormatTest, PunycodeTest, FormatValidatorTest and the suite (all pass), and compared the old and new RFC5892.isValid on 400,000 random names: no differences. Two minor points remain.

1. Unicode-version heuristic is undocumented and unverified (RFC5892Test.java:267)
jdkUnicodeIsNewerThan16 infers the JDK's Unicode version from the ordinal of UnicodeScript.UNKNOWN relative to seven script names. On JDK 25 it returns false (last=169, UNKNOWN=170), as intended. It relies on undocumented enum ordering, detects only Unicode versions that add a script, and has no test of its own. On a newer JDK the missing-virama signal is only an slf4j warning that a green build hides. Reporting through JUnit (TestReporter, or assumeTrue with a message) would put it in the test report.

2. Earlier-label fallback has three states (RFC5892.java:220)
null != decoded && null != decoded[m] ? decoded[m] : labels[m] remembers a U-label that is already the loop's unicode variable. A new transformation of a label, such as UTS 46 mapping, would have to update both decoded[n] and the fallback. A single lazily allocated array filled with unicode for every label has one source of truth.

- Report missing post-16.0 viramas through JUnit (assumeTrue) instead of
  an slf4j warning, and detect a JDK newer than Unicode 16.0 from the
  scripts added in 17.0 (documented UnicodeScript.valueOf behaviour)
  rather than enum ordering, with a test for the detection.
- Keep one source of truth for U-labels in isValid: each label is
  replaced by its U-label in place, so the Bidi back-check reads labels[m]
  with no fallback and no extra array.
@ravismash

Copy link
Copy Markdown
Author

Thanks for the extra differential run. Both points are addressed in the new commit.

Unicode version check. The test no longer relies on enum ordering. It asks whether any script added in Unicode 17.0 (Beria Erfe, Sidetic, Tai Yo, Tolong Siki) resolves through UnicodeScript.valueOf, which is documented to throw for an unknown name. Any later version includes those scripts, so this also covers versions that add no script of their own. A new test checks the detection against Character.isDefined over those four blocks, which are unassigned in 16.0. On a newer JDK, missing viramas now abort the test through assumeTrue with the list in the message, so they show in the JUnit report as a skipped test instead of only in an slf4j warning. The slf4j import is gone from the test.
One source of truth for U-labels. Once a label's U-label is known, isValid writes it back into labels[n], the local array from split. The Bidi back-check reads labels[m] directly, with no fallback and no second array, so a future transformation such as UTS 46 mapping only has to change one place. This also removes the remaining allocation for names with A-labels.
The old and new isValid return the same verdict on all 180,001 names from the differential probe set.

@stevehu

stevehu commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Review of head d9bb48d (the commit since 263c44b). Both earlier points are addressed. I ran RFC5892Test, HostnameFormatTest, PunycodeTest and FormatValidatorTest (all pass; I did not run the full suite). I compared the old and new RFC5892.isValid on 1.2 million random names (about 114,000 valid): no differences. I also checked the four Unicode 17.0 script names and their blocks against the 17.0 data files: they are unassigned in 16.0, and the Canonical_Combining_Class 9 set in 17.0 is identical to VIRAMAS. Only minor points remain, ranked most-severe first.

1. Any number of trailing separators is accepted (RFC5892.java:177)
split drops every trailing empty string, so a.., a.... and xn--4dbc.. are valid idn-hostname values, while a..b is not. A 253-character name followed by 50 dots is also valid. CHANGELOG.md:26 says the format "still accepts a trailing label separator" (singular). The behaviour is the same on master, but this PR documents the limit and adds the 253 limit. split(regex, -1) with at most one trailing empty label dropped would fix it, or the CHANGELOG and code comment could say any number.

2. The new version-detection test only does work on JDK 26 or newer (RFC5892Test.java:265)
CI runs Java 17, 21 and 25. On all three, defined and jdkUnicodeIsNewerThan16() are both false, so assertEquals(false, false) passes without testing anything. The assumeTrue branch in the virama test does not run there either. On JDK 26 missing is empty (17.0 adds no virama), so the skip path first fires on Unicode 18. Passing the script lookup in as a parameter would let a test exercise the true result.

3. isACE allocates a substring per call (RFC5892.java:358)
It is called once per label in isValid and again in isValidALabel. HostnameFormat in this PR uses regionMatches(true, ...) to avoid copying. value.regionMatches(true, 0, ACE_PREFIX, 0, ACE_PREFIX_LENGTH) does the same job without the allocation or the separate length check.

4. isValid Javadoc omits the cross-label Bidi rule (RFC5892.java:169)
The @return says every label is valid and the name is at most 253 characters. It does not say that 1a.אב and 1a.xn--4dbc are rejected because the name is a Bidi domain name, or that trailing separators are accepted.

This branch has not been deployed

No deployments
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.

3 participants