Skip to content

apply the bidi rule to every label of a bidi domain name in to_ascii - #1251

Closed
Arawoof06 wants to merge 1 commit into
ada-url:mainfrom
Arawoof06:bidi-domain-every-label
Closed

Arawoof06 wants to merge 1 commit into
ada-url:mainfrom
Arawoof06:bidi-domain-every-label

Conversation

@Arawoof06

Copy link
Copy Markdown
Contributor

is_label_valid only runs the RFC 5893 Bidi conditions on a label that has an RTL code point of its own, so the other labels of a Bidi domain name are never held to them and to_ascii accepts hosts such as ا.1x, x.1.א and ا.a- that UTS 46 (and whatwg-url, tr46, ICU) reject; only the URL standard's ASCII carve-out keeps xn--mgb.1x valid. to_ascii now classifies the mapped domain up front and validates every label with that flag, restarting the walk once when the RTL code points turn out to sit inside an ACE label of a non-ASCII domain. The same change is ada-url/idna#91, which this file is vendored from.

@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 61.53846% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.67%. Comparing base (f841678) to head (8465053).

Files with missing lines Patch % Lines
src/ada_idna.cpp 61.53% 1 Missing and 9 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1251      +/-   ##
==========================================
+ Coverage   65.59%   65.67%   +0.07%     
==========================================
  Files          43       43              
  Lines        8694     8716      +22     
  Branches     3879     3887       +8     
==========================================
+ Hits         5703     5724      +21     
+ Misses        758      754       -4     
- Partials     2233     2238       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Sep 14, 2026

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
❌ 1 regressed benchmark
✅ 49 untouched benchmarks
⏩ 4 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
SetHash 1.7 µs 1.8 µs -6.42%
SetSearchAlternatingLength 3.9 µs 3.8 µs +3.87%
SetSearchInsertAndClear 2.4 µs 2.3 µs +3.82%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing Arawoof06:bidi-domain-every-label (8465053) with main (f841678)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@lemire

lemire commented Sep 14, 2026

Copy link
Copy Markdown
Member

I encourage you to submit the PR to idna and we then get the fix from idna.

It is too messy if we change everything at once.

@lemire

lemire commented Sep 14, 2026

Copy link
Copy Markdown
Member

@anonrig I recommend closing this PR, not that the change is not welcome, but just because it is duplicated effort.

@anonrig anonrig closed this Sep 14, 2026
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