Intl.Locale: throw RangeError for structurally valid tags that exceed ICU capacity by robobun · Pull Request #35900 · oven-sh/bun

robobun · 2026-07-26T08:02:23Z

Problem

new Intl.Locale(tag) throws TypeError (failed to initialize Locale) for a structurally valid BCP-47 tag once it exceeds ICU's internal ULOC_FULLNAME_CAPACITY buffer (~180 chars, e.g. 23 distinct variant subtags). Every other Intl entry point throws RangeError for the same input, and so does Node:

const tag = 'en' + Array.from({ length: 23 }, (_, i) => '-' + String(i).padStart(5, 'v') + 'ab').join('');
// tag.length === 186, structurally valid (23 x 7-char variants)
new Intl.Locale(tag);          // TypeError: failed to initialize Locale   <- only this one
new Intl.DateTimeFormat(tag);  // RangeError
new Intl.NumberFormat(tag);    // RangeError
new Intl.Collator(tag);        // RangeError
Intl.getCanonicalLocales(tag); // RangeError
'a'.localeCompare('b', tag);   // RangeError

So catch (e) { if (e instanceof RangeError) /* invalid locale */ } misses only the Intl.Locale constructor.

Cause

IntlLocale::initializeLocale validates the tag with isStructurallyValidLanguageTag (throwing RangeError on failure) before reaching LocaleIDBuilder::canonicalize() / toCanonical(). At that point the tag is known to be well-formed; the only failure mode is ICU's fixed-size buffer overflow. Those two failure sites threw throwTypeError, while the equivalent path in canonicalizeLocaleList (used by every other Intl constructor) throws createRangeError.

Fix

The source change is in JavaScriptCore: oven-sh/WebKit#350 switches both sites to throwRangeError.

This PR:

Bumps WEBKIT_VERSION to the Node.js for loops are faster #350 preview tarball (autobuild-preview-pr-350-db4518f8, branched from the previous WEBKIT_VERSION 549170099226f816 so the only delta is the IntlLocale change).
Adds a test in test/js/web/intl/intl.test.ts asserting all seven Intl paths agree on RangeError for the oversized tag, and that 22 variants (under capacity) still constructs.

Once oven-sh/WebKit#350 is merged, WEBKIT_VERSION should be updated to the merged main sha.

Verification

fail-before (system bun):  Intl.Locale -> TypeError, Intl.Locale+options -> TypeError
pass-after  (bun bd):      Intl.Locale -> RangeError, Intl.Locale+options -> RangeError
bun bd test test/js/web/intl/intl.test.ts: 33 pass, 0 fail

Note: the source change lives in scripts/build/deps/webkit.ts (not src/), so the fail-before gate's git stash -- src/ cannot revert it; both gate runs will link the fixed WebKit and pass.

no test proof · iteration 1 · Platform-specific test-only change; deferring to CI.

… ICU capacity new Intl.Locale(tag) threw TypeError ("failed to initialize Locale") for a structurally valid BCP-47 tag once it overflowed ICU's ULOC_FULLNAME_CAPACITY buffer (23+ variant subtags, ~186 chars). Every other Intl entry point (DateTimeFormat, NumberFormat, Collator, getCanonicalLocales, localeCompare) throws RangeError for the same input via canonicalizeLocaleList, and so does V8/Node. The source change lives in JavaScriptCore (IntlLocale.cpp); this bumps WEBKIT_VERSION to the oven-sh/WebKit#350 preview tarball and adds a test asserting every Intl path agrees on RangeError for an oversized-but-valid tag.

robobun · 2026-07-26T08:02:27Z

Companion WebKit change: oven-sh/WebKit#350 (preview tarball autobuild-preview-pr-350-db4518f8 is published for all platforms).

CI state for b71ad1a (#82299): intl.test.ts passes on every platform (Linux, macOS, Windows x64/aarch64, musl, android, freebsd). All other test failures are [flaky] and passed on retry. The single hard failure is binary-size.

Binary-size check: the ~+530 KB delta is against canary baseline #79916 (commit ae4b17d, 12 main commits behind this PR's base). Those 12 commits include node:quic (#32602), node:repl (#31827), the tls rewrite (#34598) and node:inspector (#31823); that is where the growth comes from. The WebKit libJavaScriptCore.a between 549170099226f816 and preview-pr-350-db4518f8 differs by 320 bytes, with every other lib in the tarball byte-identical. The baseline is stale because recent main builds (#81770, #81444, #81275, #81265) all failed upstream of the binary-size job so no fresh binary-sizes.json artifact exists for comparison; builds #82295/#82296/#82297/#82298 on other PRs hit the identical failure against the same stale baseline.

Ready for review. Once oven-sh/WebKit#350 merges, WEBKIT_VERSION should be updated to the merged main sha.

coderabbitai · 2026-07-26T08:02:31Z

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5ce39c0e-4a9e-4eee-9518-8efe773f4edc

📥 Commits

Reviewing files that changed from the base of the PR and between 44f6469 and b71ad1a.

📒 Files selected for processing (2)

scripts/build/deps/webkit.ts
test/js/web/intl/intl.test.ts

_{Comment @coderabbitai help to get the list of available commands.}

robobun · 2026-07-26T08:15:21Z

^{Updated 7:09 AM PT - Jul 26th, 2026}

❌ @robobun, your commit b71ad1a has 1 failures in Build #82299 (All Failures):

📦 Binary size — 12 over 0.50 MB

target	this build	canary: main #79916
target	this build	size	Δ
❌ `bun-darwin-aarch64`	58.13 MB	57.58 MB	+564.9 KB
❌ `bun-darwin-x64`	63.48 MB	62.95 MB	+544.5 KB
❌ `bun-linux-aarch64`	70.98 MB	70.42 MB	+576.0 KB
❌ `bun-linux-x64`	72.47 MB	71.95 MB	+528.0 KB
❌ `bun-linux-aarch64-musl`	64.88 MB	64.32 MB	+576.0 KB
❌ `bun-linux-x64-musl`	66.98 MB	66.45 MB	+544.0 KB
❌ `bun-linux-aarch64-android`	78.47 MB	77.97 MB	+512.0 KB
❌ `bun-linux-x64-android`	80.62 MB	80.10 MB	+529.2 KB
❌ `bun-freebsd-x64`	83.07 MB	82.56 MB	+528.0 KB
❌ `bun-freebsd-aarch64`	84.84 MB	84.31 MB	+544.0 KB
❌ `bun-windows-x64`	80.26 MB	79.70 MB	+570.5 KB
❌ `bun-windows-aarch64`	70.86 MB	70.34 MB	+533.5 KB

Add [skip size check] to the commit message if this increase is intentional.

🧪 To try this PR locally:

bunx bun-pr 35900

That installs a local version of the PR into your bun-35900 executable, so you can run:

bun-35900 --bun

claude

Beyond the inline nit, I checked that the non-sha WEBKIT_VERSION format doesn't break the build scripts — prebuiltUrl() and prebuiltDestDir() both have explicit startsWith("autobuild-") branches that handle preview tags, so tarball URL and cache-dir keying resolve correctly. The pin to an unmerged preview tarball is a coordination call for a maintainer.

Extended reasoning...

The only inline finding is a nit about the 22/23-variant boundary sitting within a few bytes of an undocumented ICU threshold that may differ on macOS's system libicucore. Separately I verified the autobuild-preview-pr-350-* version string is handled by the existing startsWith("autobuild-") cases in prebuiltUrl (skips double-prefixing the release tag) and prebuiltDestDir (uses the full tag rather than a 16-char truncation), so the version bump itself won't break tarball resolution. Whether to land main pointed at a preview WebKit PR before oven-sh/WebKit#350 merges is a process decision I'm leaving to a human.

…23 variants Windows ICU 73.2 grows its canonicalization buffer and accepts arbitrarily long structurally-valid tags, so the 23-variant tag constructs there and the hard-coded RangeError assertion showed 'no throw' for every entry. The capacity is an ICU-internal detail (Linux/macOS fail at 23, Windows never does); probe for it via getCanonicalLocales and skip the RangeError assertion on builds without a limit. A Linux-only guard ensures the probe never silently degrades to a skip on the platform where the bug reproduces.

claude

The revised test in b71ad1a addresses the platform-fragility concern well — probing for the threshold and guarding against silent skip on Linux is the right shape. I'm deferring because WEBKIT_VERSION is being pinned to a preview tarball of an unmerged companion PR (oven-sh/WebKit#350); whether to land main against a preview build vs. wait for #350 to merge is a maintainer call, and the actual C++ fix isn't reviewable from this repo.

What was reviewed:

Probe loop finds the first N in [20..200] where getCanonicalLocales throws, then asserts all seven Intl paths agree on RangeError for that tag — no hardcoded boundary.
Linux-only expect(overflowTag).toBeDefined() guard prevents the RangeError test from silently becoming a skip on the platform where the bug reproduces.
"Still constructs" test now uses 5 variants (42 bytes), well clear of any ICU threshold.

Extended reasoning...

Overview

Two files: scripts/build/deps/webkit.ts bumps WEBKIT_VERSION from a main-branch sha to autobuild-preview-pr-350-db4518f8 (a preview tarball of oven-sh/WebKit#350, branched from the previous pin). test/js/web/intl/intl.test.ts adds an Intl.Locale describe block with three tests covering the RangeError-vs-TypeError fix. The actual behavioral change lives in JavaScriptCore's IntlLocale::initializeLocale in the companion WebKit PR and is not visible in this diff.

Security risks

None. The change swaps one thrown error class for another on an ICU buffer-overflow path that was already an error; no new inputs are accepted, no validation is loosened. The WebKit tarball is fetched from the same oven-sh/WebKit releases mechanism as before.

Level of scrutiny

WebKit version bumps warrant maintainer review by default — they replace the entire JS engine binary. Here the author states (and the binary-size comment corroborates) that the delta is a 320-byte change to libJavaScriptCore.a with all other libs byte-identical, which is consistent with a two-line throwTypeError→throwRangeError swap. But the pin is to an unmerged preview tarball with an explicit "revert to the merged-main sha once #350 lands" TODO in the comment. Whether Bun main should temporarily depend on a preview WebKit build, or whether this PR should wait for oven-sh/WebKit#350 to merge first, is a release-process decision I shouldn't make.

Other factors

My earlier inline concern about the hardcoded 22/23-variant boundary turned out to be real (Windows ICU 73.2 has no fixed limit and the original test failed there). The author's fix in b71ad1a is solid: the probe discovers the platform's actual threshold, the RangeError assertion skips where no limit exists, and a separate Linux-only test asserts the probe does find a limit so the main assertion can't quietly go vacuous on bundled ICU. The "still constructs" case moved to 5 variants. The probe loop runs at describe-body time (module load), which is fine — it's ~180 cheap sync calls. No remaining test-quality concerns.

robobun assigned Jarred-Sumner Jul 26, 2026

github-actions Bot added the claude label Jul 26, 2026

claude Bot reviewed Jul 26, 2026

View reviewed changes

Comment thread test/js/web/intl/intl.test.ts Outdated

claude Bot reviewed Jul 26, 2026

View reviewed changes

Navigation Menu

Uh oh!

Intl.Locale: throw RangeError for structurally valid tags that exceed ICU capacity - #35900

Intl.Locale: throw RangeError for structurally valid tags that exceed ICU capacity#35900
robobun wants to merge 2 commits into
mainfrom
farm/3a44b927/intl-locale-rangeerror

robobun commented Jul 26, 2026 •

edited

Loading

Uh oh!

robobun commented Jul 26, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 26, 2026 •

edited

Loading

Review limit reached

Uh oh!

robobun commented Jul 26, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

robobun commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Cause

Fix

Verification

Uh oh!

robobun commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

robobun commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Jul 26, 2026 •

edited

Loading

robobun commented Jul 26, 2026 •

edited

Loading

coderabbitai Bot commented Jul 26, 2026 •

edited

Loading

robobun commented Jul 26, 2026 •

edited

Loading