fix: scope IDNA ASCII web-compat leniency to the whole domain#134
Open
OmarAlJarrah wants to merge 1 commit into
Open
fix: scope IDNA ASCII web-compat leniency to the whole domain#134OmarAlJarrah wants to merge 1 commit into
OmarAlJarrah wants to merge 1 commit into
Conversation
Idna.domainToAsciiForUrl decided the WHATWG-compat "keep an invalid
xn-- label as-is" leniency per label, so a mixed domain like
xn--a.bücher kept the malformed "xn--a" label because its sibling
happened to be non-ASCII, even though xn--a.bücher is not an ASCII
string as a whole. The WHATWG "domain to ASCII" algorithm gates this
leniency on the whole percent-decoded domain being ASCII ("If domain
is an ASCII string:"), not on any individual label, and SPEC.md's
HOST-48 already documented the whole-domain condition correctly - the
implementation had drifted from both the spec text and real browsers.
Decide the leniency on the whole domain instead: an all-ASCII domain
is lowercased and kept verbatim regardless of ToASCII validity: any
domain with even one non-ASCII code point runs the full UTS-46
pipeline over the entire string, so an invalid label is fatal for the
whole domain again, matching xn--a.ß already being a required
rejection in the WPT ToASCII conformance corpus. This also drops the
now-unused raw-label splitting machinery (and its non-ASCII
dot-separator handling) that existed only to support the per-label
decision.
Idnaref only ports the pure Idna.domainToAscii pipeline used by
IdnaConformanceTest, not this URL-layer leniency wrapper, so the
known-failures baseline is unaffected; the conformance suite stays
green untouched.
Closes #107
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.
Summary
Idna.domainToAsciiForUrldecided the WHATWG web-compat ToASCII leniency ("keep an invalidxn--label as-is instead of failing") per label instead of per whole domain. That let a mixed domain likexn--a.bücherkeep the malformedxn--alabel — which decodes to a disallowed C1 control character and isn't valid Punycode — purely because a sibling label happened to be non-ASCII, even though the domain as a whole is not an ASCII string.asciiLenientDomainToAsciito decide leniency on the whole raw domain: an all-ASCII domain is lowercased and kept verbatim regardless of ToASCII validity (unchanged); a domain with any non-ASCII code point anywhere now runs the full UTS-46 pipeline over the entire string, so an invalid label is fatal for the whole domain again. This also removes the raw-label-splitting machinery (and the non-ASCII dot-separator constants) that existed only to support the old per-label decision, since it's no longer needed.SPEC.md's[HOST-48]text untouched — it already documents the whole-domain condition ("when the percent-decoded domain is already an ASCII string") correctly. It was the implementation that had drifted from the spec, not the other way around.Why the whole-domain reading is correct
I checked this against the vendored WHATWG URL Standard (
.claude/references/whatwg-url-standard.txt) before touching anything. The "domain to ASCII" algorithm's leniency step reads:If domain is an ASCII string:— that's a whole-domain gate, evaluated once, before any label splitting. There is no per-label variant of this check anywhere in the spec.The per-label behavior was introduced deliberately in an earlier commit, on the reasoning that real browsers apply the leniency per label and that gating it on the whole domain was an inconsistency (
xn--a.examplekept a bad label but the otherwise-identicalxn--a.caférejected the whole thing over the same label). That reasoning doesn't hold up against the spec text: the asymmetry it was "fixing" is exactly what real WHATWG user agents do — an ASCII-only host gets the compatibility pass, a host with any non-ASCII label does not.This is also corroborated by data already in this repo: the WPT ToASCII conformance corpus (
IdnaConformanceData.kt) hasIdnaCase("xn--a.ß", null)as a required rejection, and it's not inIdnaConformanceKnownFailures.kt— meaning the pure UTS-46 pipeline (Idna.domainToAscii, tested directly against WPT) already correctly rejects this exact shape of mixed domain. Only the URL-layer leniency wrapper had the bug.tools/internal/idnaref(the Go reference used for the idnaref lock-step ratchet) only portsIdna.domainToAscii, the pure pipeline — it never touchesdomainToAsciiForUrl/asciiLenientDomainToAscii. Since this fix only changes the lenient wrapper and leavesdomainToAsciiitself untouched, idnaref andIdnaConformanceKnownFailures.ktdon't need regenerating; I ran the full IDNA conformance suite to confirm the known-failures baseline still matches exactly.Test plan
Idn.toAscii("xn--a.bücher")now fails); confirmed it fails against the pre-fix code, then passes after the fix:kuri:jvmTest :kuri:ktlintCheck :kuri:detekt :kuri:apiCheckgreen:kuri:jsNodeTestgreen (commonMain touched)IdnaConformanceTest, WPT corpus + known-failures ratchet) green, unchanged:kuri:macosArm64Test(native) greenCloses #107