From 40c0cc15262fd9c83f1572a976bdd21b122ff73e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 17 Aug 2026 19:49:04 +0000 Subject: [PATCH 1/7] url: strip ASCII tab and newline in the pathname setter before the leading-slash check URL::setPath decides whether to prepend "/" (and the "/." guard for a host-less URL) from the raw value and only removes tab/LF/CR when it reparses, so assigning "\n/evil.com/y" to the pathname of "foo:/a" reparsed as "foo://evil.com/y" and made evil.com the host; on http URLs "\t/x" became "//x". The basic URL parser removes those characters before any state runs, so remove them in the setter first. --- src/jsc/bindings/URLDecomposition.cpp | 8 ++++-- src/jsc/bindings/URLDecomposition.h | 2 +- test/js/web/url/url.test.ts | 36 +++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/jsc/bindings/URLDecomposition.cpp b/src/jsc/bindings/URLDecomposition.cpp index a59c4747e242..0305a7d05da8 100644 --- a/src/jsc/bindings/URLDecomposition.cpp +++ b/src/jsc/bindings/URLDecomposition.cpp @@ -218,12 +218,16 @@ String URLDecomposition::pathname() const return fullURL().path().toString(); } -void URLDecomposition::setPathname(StringView value) +void URLDecomposition::setPathname(const String& value) { auto fullURL = this->fullURL(); if (fullURL.hasOpaquePath()) return; - fullURL.setPath(value); + // The basic URL parser removes ASCII tab and newline before anything else. + // URL::setPath only drops them in its reparse, after choosing the leading "/" + // (and the "/." guard) from the raw value, so "\n/a/b" reparsed as "//a/b". + String path = value.removeCharacters([](char16_t c) { return c == '\t' || c == '\n' || c == '\r'; }); + fullURL.setPath(path); setFullURL(fullURL); } diff --git a/src/jsc/bindings/URLDecomposition.h b/src/jsc/bindings/URLDecomposition.h index 47f88b5f41dd..7c0945816a62 100644 --- a/src/jsc/bindings/URLDecomposition.h +++ b/src/jsc/bindings/URLDecomposition.h @@ -60,7 +60,7 @@ class URLDecomposition { void setPort(StringView); WEBCORE_EXPORT String pathname() const; - void setPathname(StringView); + void setPathname(const String&); WEBCORE_EXPORT String search() const; void setSearch(const String&); diff --git a/test/js/web/url/url.test.ts b/test/js/web/url/url.test.ts index 3857ed3e9ac0..adfe256ca235 100755 --- a/test/js/web/url/url.test.ts +++ b/test/js/web/url/url.test.ts @@ -249,6 +249,42 @@ describe("url", () => { // A path whose first segment merely starts with "." gets no guard. expect(util.inspect(new URL("foo:/.foo"), { showHidden: true })).toContain("pathname_start: 4,"); }); + + it("pathname setter removes ASCII tab and newline before looking at the leading slash", () => { + // https://url.spec.whatwg.org/#concept-basic-url-parser removes tab, LF and CR from the value first, so + // "\n/x" is the same assignment as "/x". Expected values are Node's. On a host-less URL the unstripped + // value used to reparse as "//evil.com/y", which made evil.com the host. + const cases: [href: string, value: string, expected: { href: string; host: string; pathname: string }][] = [ + ["foo:/a", "\n/evil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], + ["foo:/a", "\t/evil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], + ["foo:/a", "\r/evil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], + ["foo:/a", "\t\n\r/evil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], + // Two leading slashes on a host-less URL get the /. guard instead of becoming an empty host. + ["foo:/a", "\n//evil.com/y", { href: "foo:/.//evil.com/y", host: "", pathname: "//evil.com/y" }], + ["foo:/a", "\tevil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], + ["foo:/.//p", "\t/x", { href: "foo:/x", host: "", pathname: "/x" }], + ["foo:/.//p", "\t//x", { href: "foo:/.//x", host: "", pathname: "//x" }], + ["foo://host/a", "\n/x/y", { href: "foo://host/x/y", host: "host", pathname: "/x/y" }], + // A value that strips down to "" empties the path of a non-special URL with a host. + ["foo://host/a", "\t", { href: "foo://host", host: "host", pathname: "" }], + ["http://h/", "\t/x", { href: "http://h/x", host: "h", pathname: "/x" }], + ["http://h/", "\n/x", { href: "http://h/x", host: "h", pathname: "/x" }], + ["http://h/", "\r/x", { href: "http://h/x", host: "h", pathname: "/x" }], + ["http://h/", "\t//x", { href: "http://h//x", host: "h", pathname: "//x" }], + ["http://h/", "\t\\x", { href: "http://h/x", host: "h", pathname: "/x" }], + ["http://h/", "\tx", { href: "http://h/x", host: "h", pathname: "/x" }], + ["http://h/", "\t/a\tb/\nc", { href: "http://h/ab/c", host: "h", pathname: "/ab/c" }], + ["http://h/", "/x\n", { href: "http://h/x", host: "h", pathname: "/x" }], + ["file:///a", "\t/x", { href: "file:///x", host: "", pathname: "/x" }], + ]; + const results = cases.map(([href, value]) => { + const url = new URL(href); + url.pathname = value; + return [href, value, { href: url.href, host: url.host, pathname: url.pathname }]; + }); + expect(results).toEqual(cases); + }); + it("works", () => { const inputs = [ [ From 348da53f26fdd6f3a91bbdd53843f00ee1120f09 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:15:26 +0000 Subject: [PATCH 2/7] ci: retrigger From 87dc303f042bdf59afc39af2549ddf99d4ca6eb2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:16:55 +0000 Subject: [PATCH 3/7] url: shorten the setPathname comment --- src/jsc/bindings/URLDecomposition.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/jsc/bindings/URLDecomposition.cpp b/src/jsc/bindings/URLDecomposition.cpp index 0305a7d05da8..0acc1dc49f6d 100644 --- a/src/jsc/bindings/URLDecomposition.cpp +++ b/src/jsc/bindings/URLDecomposition.cpp @@ -223,9 +223,7 @@ void URLDecomposition::setPathname(const String& value) auto fullURL = this->fullURL(); if (fullURL.hasOpaquePath()) return; - // The basic URL parser removes ASCII tab and newline before anything else. - // URL::setPath only drops them in its reparse, after choosing the leading "/" - // (and the "/." guard) from the raw value, so "\n/a/b" reparsed as "//a/b". + // URL::setPath picks the leading "/" before its reparse strips tab and newline. String path = value.removeCharacters([](char16_t c) { return c == '\t' || c == '\n' || c == '\r'; }); fullURL.setPath(path); setFullURL(fullURL); From 73e62de830ea499ab22c40ee51a496fe7ba257d4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:22:59 +0000 Subject: [PATCH 4/7] test: one test per row in the pathname setter table --- test/js/web/url/url.test.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/js/web/url/url.test.ts b/test/js/web/url/url.test.ts index adfe256ca235..8f9d4d2e8df3 100755 --- a/test/js/web/url/url.test.ts +++ b/test/js/web/url/url.test.ts @@ -250,11 +250,11 @@ describe("url", () => { expect(util.inspect(new URL("foo:/.foo"), { showHidden: true })).toContain("pathname_start: 4,"); }); - it("pathname setter removes ASCII tab and newline before looking at the leading slash", () => { + describe("pathname setter removes ASCII tab and newline before looking at the leading slash", () => { // https://url.spec.whatwg.org/#concept-basic-url-parser removes tab, LF and CR from the value first, so // "\n/x" is the same assignment as "/x". Expected values are Node's. On a host-less URL the unstripped // value used to reparse as "//evil.com/y", which made evil.com the host. - const cases: [href: string, value: string, expected: { href: string; host: string; pathname: string }][] = [ + test.each<[href: string, value: string, expected: { href: string; host: string; pathname: string }]>([ ["foo:/a", "\n/evil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], ["foo:/a", "\t/evil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], ["foo:/a", "\r/evil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], @@ -276,13 +276,11 @@ describe("url", () => { ["http://h/", "\t/a\tb/\nc", { href: "http://h/ab/c", host: "h", pathname: "/ab/c" }], ["http://h/", "/x\n", { href: "http://h/x", host: "h", pathname: "/x" }], ["file:///a", "\t/x", { href: "file:///x", host: "", pathname: "/x" }], - ]; - const results = cases.map(([href, value]) => { + ])("new URL(%p).pathname = %p", (href, value, expected) => { const url = new URL(href); url.pathname = value; - return [href, value, { href: url.href, host: url.host, pathname: url.pathname }]; + expect({ href: url.href, host: url.host, pathname: url.pathname }).toEqual(expected); }); - expect(results).toEqual(cases); }); it("works", () => { From e463e539f5856bcc748d424653feb9b64cb6cda5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:48:58 +0000 Subject: [PATCH 5/7] url: strip tab and newline in the host and hostname setters too setHost counted the port digits in the raw value, so a tab after the colon dropped or truncated the port, and a value that is only tab or newline skipped the empty-value no-op in both setters and reparsed http:///p with the first path segment as the host. --- src/jsc/bindings/URLDecomposition.cpp | 20 +++++++++++---- src/jsc/bindings/URLDecomposition.h | 4 +-- test/js/web/url/url.test.ts | 35 +++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/jsc/bindings/URLDecomposition.cpp b/src/jsc/bindings/URLDecomposition.cpp index 0acc1dc49f6d..691507a42cfa 100644 --- a/src/jsc/bindings/URLDecomposition.cpp +++ b/src/jsc/bindings/URLDecomposition.cpp @@ -38,6 +38,14 @@ static bool hasAcceptableHost(const WTF::URL& url) return Bun::hasValidPunycodeHost(url.host()) || !url.hasSpecialScheme(); } +// The basic URL parser removes these before any state runs. The host, hostname +// and pathname setters look at the value (empty check, port digits, leading "/") +// before WTF::URL reparses it, so they have to remove them up front too. +static String removeTabAndNewline(const String& value) +{ + return value.removeCharacters([](char16_t c) { return c == '\t' || c == '\n' || c == '\r'; }); +} + String URLDecomposition::origin() const { auto fullURL = this->fullURL(); @@ -113,9 +121,11 @@ static unsigned countASCIIDigits(StringView string) return length; } -void URLDecomposition::setHost(StringView value) +void URLDecomposition::setHost(const String& input) { auto fullURL = this->fullURL(); + String strippedInput = removeTabAndNewline(input); + StringView value = strippedInput; if (value.isEmpty() && !fullURL.protocolIsFile() && fullURL.hasSpecialScheme()) return; @@ -154,9 +164,10 @@ String URLDecomposition::hostname() const return fullURL().host().toString(); } -void URLDecomposition::setHostname(StringView host) +void URLDecomposition::setHostname(const String& input) { auto fullURL = this->fullURL(); + String host = removeTabAndNewline(input); if (host.isEmpty() && !fullURL.protocolIsFile() && fullURL.hasSpecialScheme()) return; if (fullURL.hasOpaquePath()) @@ -218,13 +229,12 @@ String URLDecomposition::pathname() const return fullURL().path().toString(); } -void URLDecomposition::setPathname(const String& value) +void URLDecomposition::setPathname(const String& input) { auto fullURL = this->fullURL(); if (fullURL.hasOpaquePath()) return; - // URL::setPath picks the leading "/" before its reparse strips tab and newline. - String path = value.removeCharacters([](char16_t c) { return c == '\t' || c == '\n' || c == '\r'; }); + String path = removeTabAndNewline(input); fullURL.setPath(path); setFullURL(fullURL); } diff --git a/src/jsc/bindings/URLDecomposition.h b/src/jsc/bindings/URLDecomposition.h index 7c0945816a62..428f1787a0ca 100644 --- a/src/jsc/bindings/URLDecomposition.h +++ b/src/jsc/bindings/URLDecomposition.h @@ -51,10 +51,10 @@ class URLDecomposition { void setPassword(StringView); WEBCORE_EXPORT String host() const; - void setHost(StringView); + void setHost(const String&); WEBCORE_EXPORT String hostname() const; - void setHostname(StringView); + void setHostname(const String&); WEBCORE_EXPORT String port() const; void setPort(StringView); diff --git a/test/js/web/url/url.test.ts b/test/js/web/url/url.test.ts index 8f9d4d2e8df3..5e195b2efc38 100755 --- a/test/js/web/url/url.test.ts +++ b/test/js/web/url/url.test.ts @@ -283,6 +283,41 @@ describe("url", () => { }); }); + describe("host and hostname setters remove ASCII tab and newline before looking at the value", () => { + // Same parser rule as the pathname setter above; expected values are Node's. The host setter used to count + // the port digits in the raw value, so a tab after the colon dropped or truncated the port, and a value + // that is only whitespace skipped the empty-value no-op and reparsed "http:///p" with "p" as the host. + test.each< + [ + href: string, + setter: "host" | "hostname", + value: string, + expected: { href: string; host: string; pathname: string }, + ] + >([ + ["http://h/p", "host", "x.com:\t81", { href: "http://x.com:81/p", host: "x.com:81", pathname: "/p" }], + ["http://h/p", "host", "x.com:8\n1", { href: "http://x.com:81/p", host: "x.com:81", pathname: "/p" }], + ["http://h/p", "host", "x.com:\r81\r", { href: "http://x.com:81/p", host: "x.com:81", pathname: "/p" }], + ["http://h/p", "host", "[::2]:\t81", { href: "http://[::2]:81/p", host: "[::2]:81", pathname: "/p" }], + // With a state override the port ends at the first non-digit, so only the "8" is taken. + ["http://h/p", "host", "x.com:\t8a", { href: "http://x.com:8/p", host: "x.com:8", pathname: "/p" }], + ["http://h/p", "host", "x.com:\t80", { href: "http://x.com/p", host: "x.com", pathname: "/p" }], + ["foo://h/p", "host", "x.com:\t81", { href: "foo://x.com:81/p", host: "x.com:81", pathname: "/p" }], + ["foo:/p", "host", "x.com:\n81", { href: "foo://x.com:81/p", host: "x.com:81", pathname: "/p" }], + // A file URL cannot take a port, so this assignment is a no-op rather than a host change. + ["file:///p", "host", "x.com:\t81", { href: "file:///p", host: "", pathname: "/p" }], + ["http://h/p", "host", "\t", { href: "http://h/p", host: "h", pathname: "/p" }], + ["http://h/p", "host", "\n", { href: "http://h/p", host: "h", pathname: "/p" }], + ["http://h/p", "hostname", "\r", { href: "http://h/p", host: "h", pathname: "/p" }], + ["http://h/p", "hostname", "\t\n\r", { href: "http://h/p", host: "h", pathname: "/p" }], + ["http://h/p", "hostname", "\tx.com\n", { href: "http://x.com/p", host: "x.com", pathname: "/p" }], + ])("new URL(%p).%s = %p", (href, setter, value, expected) => { + const url = new URL(href); + url[setter] = value; + expect({ href: url.href, host: url.host, pathname: url.pathname }).toEqual(expected); + }); + }); + it("works", () => { const inputs = [ [ From 4643085db065581f649b299f7c0db69569d06c5d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:52:24 +0000 Subject: [PATCH 6/7] url: one-line comment on removeTabAndNewline --- src/jsc/bindings/URLDecomposition.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/jsc/bindings/URLDecomposition.cpp b/src/jsc/bindings/URLDecomposition.cpp index 691507a42cfa..df36256530f4 100644 --- a/src/jsc/bindings/URLDecomposition.cpp +++ b/src/jsc/bindings/URLDecomposition.cpp @@ -38,9 +38,7 @@ static bool hasAcceptableHost(const WTF::URL& url) return Bun::hasValidPunycodeHost(url.host()) || !url.hasSpecialScheme(); } -// The basic URL parser removes these before any state runs. The host, hostname -// and pathname setters look at the value (empty check, port digits, leading "/") -// before WTF::URL reparses it, so they have to remove them up front too. +// The basic URL parser removes these first; the setters below inspect the value before WTF::URL reparses it. static String removeTabAndNewline(const String& value) { return value.removeCharacters([](char16_t c) { return c == '\t' || c == '\n' || c == '\r'; }); From db4f2ef9b5dfcab54da9fec7f17afea7aff0fc64 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:05:40 +0000 Subject: [PATCH 7/7] test: name the Node version the URL setter tables were checked against --- test/js/web/url/url.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/js/web/url/url.test.ts b/test/js/web/url/url.test.ts index 5e195b2efc38..cad231b3b251 100755 --- a/test/js/web/url/url.test.ts +++ b/test/js/web/url/url.test.ts @@ -252,7 +252,7 @@ describe("url", () => { describe("pathname setter removes ASCII tab and newline before looking at the leading slash", () => { // https://url.spec.whatwg.org/#concept-basic-url-parser removes tab, LF and CR from the value first, so - // "\n/x" is the same assignment as "/x". Expected values are Node's. On a host-less URL the unstripped + // "\n/x" is the same assignment as "/x". Expected values are Node 26's. On a host-less URL the unstripped // value used to reparse as "//evil.com/y", which made evil.com the host. test.each<[href: string, value: string, expected: { href: string; host: string; pathname: string }]>([ ["foo:/a", "\n/evil.com/y", { href: "foo:/evil.com/y", host: "", pathname: "/evil.com/y" }], @@ -284,7 +284,7 @@ describe("url", () => { }); describe("host and hostname setters remove ASCII tab and newline before looking at the value", () => { - // Same parser rule as the pathname setter above; expected values are Node's. The host setter used to count + // Same parser rule as the pathname setter above; expected values are Node 26's. The host setter used to count // the port digits in the raw value, so a tab after the colon dropped or truncated the port, and a value // that is only whitespace skipped the empty-value no-op and reparsed "http:///p" with "p" as the host. test.each<