Repository navigation
lab-ops-hub: judge U+0345 by its UTS-46 mapping in the container validator - #610
Conversation
…dator (#596) UTS-46 maps U+0345 (combining Greek ypogegrammeni) to U+03B9 (iota, bidi class L), but NFKC leaves it unchanged, so the container judged it as a combining mark (bidi NSM) — legal mid-label anywhere — while the Worker judges the mapped letter. Inside an RTL label the mapped form is R + L + R, a bidi-rule violation: א\u0345א.com, א\u0345.com and the A-label spelling xn--jua51kba were certified by the container and refused by the Worker. _judge_idna_label now applies the U+0345 -> U+03B9 mapping before judging, covering both the raw-label and A-label-decoded call sites. At label start the mapping also closes the former leading-mark over-refusal (the mapped label starts with a letter, matching the Worker); that behaviour change is pinned by test. Tests: new Python test_judges_u0345_by_its_uts46_mapping (RTL spellings + A-label refused; Greek/LTR controls and the label-start spelling accepted) and a TS pin of the Worker's verdicts. Python 38/38, TS 31/31 (node --test); Bun differential agrees on all six spellings; ablation: the new Python test fails with only app.py reverted.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 58 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 762d203562
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # violation the Worker refuses, while at label start the mapped | ||
| # letter is an ordinary first character, closing the former | ||
| # leading-mark over-refusal (#596). | ||
| label = label.translate({0x0345: "ι"}) |
There was a problem hiding this comment.
_judge_idna_label also receives the decoded payload of xn-- labels, but UTS-46 mapping applies to raw U-label input, not after A-label decoding. Consequently, invalid A-labels such as xn--jua (decoding to U+0345) and xn--abc-blc (decoding to U+0345abc) are now translated to valid labels and accepted by the container, while Bun/WHATWG rejects both. Keep decoded A-labels on the unmapped validation path so redirects and initial requests cannot pass container validation with a hostname the Worker rejects.
Useful? React with 👍 / 👎.
| # violation the Worker refuses, while at label start the mapped | ||
| # letter is an ordinary first character, closing the former | ||
| # leading-mark over-refusal (#596). | ||
| label = label.translate({0x0345: "ι"}) |
There was a problem hiding this comment.
Because this translation happens only inside the per-label validator, the earlier whole-host length check still measures the unmapped punycode. For example, four labels consisting of U+0345 followed by 54 Greek alphas encode to 251 characters before this mapping but 255 afterward; the container now accepts that raw host because every mapped label is individually 63 characters, whereas the Worker's explicit 253-character guard rejects its canonical hostname. Map the raw host before computing encoded_labels, or recheck the mapped total, so an overlong redirect target is not accepted and passed to the resolver.
Useful? React with 👍 / 👎.
Fixes #596.
Problem
UTS-46 maps U+0345 (combining Greek ypogegrammeni) to U+03B9 (ι, bidi class L), but the container validator's mapped form is NFKC +
lower(), and NFKC leaves U+0345 unchanged. The container therefore judged it as a combining mark (bidi NSM, legal mid-label in any context) while the Worker judges the mapped letter. Inside an RTL label the mapped form is R + L + R — a bidi-rule violation — soא\u0345א.com,א\u0345.comand the A-label spellingxn--jua51kbawere certified by the container and refused by the Worker.Solution
_judge_idna_labelnow applies the U+0345 → U+03B9 UTS-46 mapping before judging, so both call sites (raw labels and A-label decoded forms) judge the UTS-46-mapped label. The stale "known residual" comment that documented U+0345 as a leading mark is updated. Side effect, pinned by test: at label start the mapping closes the former over-refusal —\u0345abc.comis now accepted, matching the Worker (U+0345 maps to a letter, so the label starts with one).Verification
ae38cd65: all three RTL spellings container-ACCEPT / Worker-refuse.ex\u0345ample.com,α\u0345.com,\u0345abc.comACCEPT — identical to the Worker's verdicts under Bun on all six spellings.node --test). Ablation: the new Python test fails with onlyapp.pyreverted.ruff formatclean.Single-repo:
_judge_idna_labelexists only in developing-today/code.Need help on this PR? Tag
@codesmith-botwith what you need.