Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/jsc/bindings/DOMURL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,13 @@ static String applyIDNADeltaToURLAuthority(const String& urlString, StringView s
return {};

auto mappedHost = Bun::applyUnicode16IDNADelta(hostView.toString());
// A host of only ignored-class code points maps to the empty string, which is
// domain-to-ASCII failure (node throws ERR_INVALID_URL). Splicing an empty host
// back in would instead let the special-authority-ignore-slashes state promote
// the first path segment to the host (http://\u180E/a -> http:///a -> host "a").
// Leave the input untouched so the parser rejects the original code points.
Comment thread
robobun marked this conversation as resolved.
Outdated
if (mappedHost.isEmpty())
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
return {};
StringBuilder builder;
builder.append(view.left(hostStart));
builder.append(mappedHost);
Expand Down
7 changes: 7 additions & 0 deletions src/jsc/bindings/URLDecomposition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ void URLDecomposition::setHost(StringView value)
auto hostSpan = hostEnd == notFound ? value : value.left(hostEnd);
if (Bun::containsUnicode16IDNADeltaSource(hostSpan)) {
auto mappedHost = Bun::applyUnicode16IDNADelta(hostSpan.toString());
// A non-empty host span mapping to empty is domain-to-ASCII failure
// (setter no-ops, even for file: where a literal "" is assignable).
Comment thread
robobun marked this conversation as resolved.
Outdated
if (mappedHost.isEmpty())
return;
Comment thread
robobun marked this conversation as resolved.
mappedValue = hostEnd == notFound ? mappedHost : makeString(mappedHost, value.substring(hostEnd));
value = mappedValue;
}
Expand Down Expand Up @@ -175,6 +179,9 @@ void URLDecomposition::setHostname(StringView host)
String mappedHost;
if (fullURL.hasSpecialScheme() && !host.startsWith('[') && Bun::containsUnicode16IDNADeltaSource(host)) {
mappedHost = Bun::applyUnicode16IDNADelta(host.toString());
// See setHost: mapping a non-empty hostname to empty is failure, not "".
if (mappedHost.isEmpty())
return;
host = mappedHost;
}
if (host.isEmpty() && !fullURL.protocolIsFile() && fullURL.hasSpecialScheme())
Expand Down
39 changes: 39 additions & 0 deletions test/js/web/url/url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,45 @@
expect(h2.host).toBe("xn--foo-7ka:81");
});

it("rejects special-scheme hosts made only of IDNA-ignored code points", () => {
// U+180E and U+206A..U+206F map to nothing under UTS #46, so these hosts
// map to the empty string: domain-to-ASCII failure (node throws
// ERR_INVALID_URL), never "promote the first path segment to the host".
const inputs = [
"http://\u180E/evil.example/x",
"https://\u206A\u206F/other.example/p",
"ws://\u180E/h/p",
"file://\u180E/some/dir/f",
"http://\u180E\u206B:8080/x",
"http://user@\u180E/x",
];
for (const input of inputs) {
expect(() => new URL(input)).toThrow();
expect(URL.canParse(input)).toBe(false);
expect(URL.parse(input)).toBeNull();
const u = new URL("http://ok.example/");
expect(() => (u.href = input)).toThrow();
}
// Scheme-relative input and an all-ignored base reach the same host span.
expect(() => new URL("//\u180E/evil.example/", "http://good.example/")).toThrow();
expect(URL.canParse("//\u180E/evil.example/", "http://good.example/")).toBe(false);
expect(() => new URL("/x", "http://\u206A/base.example/")).toThrow();

Check warning on line 186 in test/js/web/url/url.test.ts

View check run for this annotation

Claude / Claude Code Review

Bare toThrow() without asserting error code

The bare `.toThrow()` calls at lines 177, 181, 184, and 186 should assert the specific error code — e.g. `toThrow(expect.objectContaining({ code: "ERR_INVALID_URL" }))` — matching the pattern already used in the "URL throws" test at the top of this file. REVIEW.md: "specific error class/code/message (never bare `toThrow()`)".
Comment thread
robobun marked this conversation as resolved.
Outdated
// Mixed hosts still strip the ignored code point rather than failing.
expect(new URL("http://a\u180Eb/").href).toBe("http://ab/");
expect(new URL("file://a\u180Eb/x").host).toBe("ab");
// Setters: a non-empty host that maps to empty is a failed host parse and
// no-ops, even for file: where assigning a literal "" clears the host.
const f1 = new URL("file://server/share");
f1.host = "\u180E";
expect(f1.href).toBe("file://server/share");
const f2 = new URL("file://server/share");
f2.hostname = "\u180E";
expect(f2.href).toBe("file://server/share");
const f3 = new URL("file://server/share");
f3.host = "";
expect(f3.href).toBe("file:///share");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

it("prints", () => {
// URL.prototype carries [Symbol.for("nodejs.util.inspect.custom")], so
// Bun.inspect matches node's util.inspect output.
Expand Down