diff --git a/src/jsc/bindings/URLDecomposition.cpp b/src/jsc/bindings/URLDecomposition.cpp index a59c4747e242..df36256530f4 100644 --- a/src/jsc/bindings/URLDecomposition.cpp +++ b/src/jsc/bindings/URLDecomposition.cpp @@ -38,6 +38,12 @@ static bool hasAcceptableHost(const WTF::URL& url) return Bun::hasValidPunycodeHost(url.host()) || !url.hasSpecialScheme(); } +// 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'; }); +} + String URLDecomposition::origin() const { auto fullURL = this->fullURL(); @@ -113,9 +119,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 +162,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,12 +227,13 @@ String URLDecomposition::pathname() const return fullURL().path().toString(); } -void URLDecomposition::setPathname(StringView value) +void URLDecomposition::setPathname(const String& input) { auto fullURL = this->fullURL(); if (fullURL.hasOpaquePath()) return; - fullURL.setPath(value); + String path = removeTabAndNewline(input); + fullURL.setPath(path); setFullURL(fullURL); } diff --git a/src/jsc/bindings/URLDecomposition.h b/src/jsc/bindings/URLDecomposition.h index 47f88b5f41dd..428f1787a0ca 100644 --- a/src/jsc/bindings/URLDecomposition.h +++ b/src/jsc/bindings/URLDecomposition.h @@ -51,16 +51,16 @@ 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); 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..cad231b3b251 100755 --- a/test/js/web/url/url.test.ts +++ b/test/js/web/url/url.test.ts @@ -249,6 +249,75 @@ 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,"); }); + + 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 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" }], + ["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" }], + ])("new URL(%p).pathname = %p", (href, value, expected) => { + const url = new URL(href); + url.pathname = value; + expect({ href: url.href, host: url.host, pathname: url.pathname }).toEqual(expected); + }); + }); + + 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 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< + [ + 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 = [ [