diff --git a/scripts/build/deps/webkit.ts b/scripts/build/deps/webkit.ts index 0b1c956da743..b96239fe3620 100644 --- a/scripts/build/deps/webkit.ts +++ b/scripts/build/deps/webkit.ts @@ -3,7 +3,7 @@ * for local mode. Override via `--webkit-version=` to test a branch. * From https://github.com/oven-sh/WebKit releases. */ -export const WEBKIT_VERSION = "f0f60fd2324817dae9656d8bf2fcae25ceaccc37"; +export const WEBKIT_VERSION = "0cbb4a194653231955187f9d8a2990d4b4a55266"; /** * WebKit (JavaScriptCore) — the JS engine. diff --git a/src/jsc/bindings/DOMURL.cpp b/src/jsc/bindings/DOMURL.cpp index 8a8151bedf2d..f853f796fbd0 100644 --- a/src/jsc/bindings/DOMURL.cpp +++ b/src/jsc/bindings/DOMURL.cpp @@ -33,13 +33,39 @@ namespace WebCore { // The WHATWG parser (WebKit) fast-paths all-ASCII hosts without validating // xn-- labels; Node's ada rejects invalid punycode in special-scheme hosts. -static bool hasValidParsedHost(const URL& url) +// `input` is the string the host was parsed from (a base URL's host was checked when the base was parsed). +static bool hasValidParsedHost(const URL& url, const String& input) { - // Cheap accept first: hosts without an invalid xn-- label are always fine. - if (Bun::hasValidPunycodeHost(url.host())) + auto host = url.host(); + if (host.length() < 4 || !host.contains("xn--"_s)) return true; // Non-special schemes have opaque hosts and skip IDNA entirely. - return !url.hasSpecialScheme(); + if (!url.hasSpecialScheme()) + return true; + // An xn-- label that ICU produced from a Unicode host is valid by construction; only one that was literally in the + // input needs checking. If this input supplied the host, it did so from its authority: after the scheme and any + // slashes, up to the next slash, '?' or '#'. Tabs and newlines are removed anywhere and percent-encoding is decoded + // in hosts, so either could hide a literal label. + StringView view(input); + if (view.find([](char16_t character) { return character == '\t' || character == '\n' || character == '\r'; }) != notFound) + return Bun::hasValidPunycodeHost(host); + unsigned start = 0; + while (start < view.length() && view[start] <= ' ') + ++start; + if (start < view.length() && isASCIIAlpha(view[start])) { + unsigned schemeEnd = start + 1; + while (schemeEnd < view.length() && (isASCIIAlphanumeric(view[schemeEnd]) || view[schemeEnd] == '+' || view[schemeEnd] == '-' || view[schemeEnd] == '.')) + ++schemeEnd; + if (schemeEnd < view.length() && view[schemeEnd] == ':') + start = schemeEnd + 1; + } + while (start < view.length() && (view[start] == '/' || view[start] == '\\')) + ++start; + auto authority = view.substring(start); + authority = authority.left(std::min(authority.find([](char16_t character) { return character == '/' || character == '\\' || character == '?' || character == '#'; }), authority.length())); + if (authority.find('%') == notFound && !authority.containsIgnoringASCIICase("xn--"_s)) + return true; + return Bun::hasValidPunycodeHost(host); } inline DOMURL::DOMURL(URL&& completeURL) @@ -56,7 +82,7 @@ inline DOMURL::DOMURL(URL&& completeURL) ExceptionOr> DOMURL::create(const String& url) { URL completeURL { url }; - if (!completeURL.isValid() || !hasValidParsedHost(completeURL)) + if (!completeURL.isValid() || !hasValidParsedHost(completeURL, url)) return Exception { InvalidURLError, url }; return adoptRef(*new DOMURL(WTF::move(completeURL))); } @@ -65,7 +91,7 @@ ExceptionOr> DOMURL::create(const String& url, const URL& base, cons { ASSERT(base.isValid() || base.isNull()); URL completeURL { base, url }; - if (!completeURL.isValid() || !hasValidParsedHost(completeURL)) + if (!completeURL.isValid() || !hasValidParsedHost(completeURL, url)) return Exception { InvalidURLError, url, baseInput }; return adoptRef(*new DOMURL(WTF::move(completeURL))); } @@ -73,7 +99,7 @@ ExceptionOr> DOMURL::create(const String& url, const URL& base, cons ExceptionOr> DOMURL::create(const String& url, const String& base) { URL baseURL { base }; - if (!base.isNull() && (!baseURL.isValid() || !hasValidParsedHost(baseURL))) + if (!base.isNull() && (!baseURL.isValid() || !hasValidParsedHost(baseURL, base))) return Exception { InvalidURLError, url, base }; return create(url, baseURL, base); } @@ -83,10 +109,10 @@ DOMURL::~DOMURL() = default; static URL parseInternal(const String& url, const String& base) { URL baseURL { base }; - if (!base.isNull() && (!baseURL.isValid() || !hasValidParsedHost(baseURL))) + if (!base.isNull() && (!baseURL.isValid() || !hasValidParsedHost(baseURL, base))) return {}; URL result { baseURL, url }; - if (result.isValid() && !hasValidParsedHost(result)) + if (result.isValid() && !hasValidParsedHost(result, url)) return {}; return result; } @@ -107,7 +133,7 @@ bool DOMURL::canParse(const String& url, const String& base) ExceptionOr DOMURL::setHref(const String& url) { URL completeURL { URL {}, url }; - if (!completeURL.isValid() || !hasValidParsedHost(completeURL)) + if (!completeURL.isValid() || !hasValidParsedHost(completeURL, url)) return Exception { InvalidURLError, url }; m_url = WTF::move(completeURL); m_searchParamsDirty = false; diff --git a/test/js/web/url/url-wpt-constructor.test.ts b/test/js/web/url/url-wpt-constructor.test.ts new file mode 100644 index 000000000000..96bccc2ff489 --- /dev/null +++ b/test/js/web/url/url-wpt-constructor.test.ts @@ -0,0 +1,65 @@ +// WPT url/url-constructor.any.js over the vendored urltestdata.json: every non-failure entry must produce the expected +// href and components, every failure entry must throw. +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +type Entry = { + input: string; + base?: string | null; + href?: string; + failure?: boolean; + origin?: string; + protocol?: string; + username?: string; + password?: string; + host?: string; + hostname?: string; + port?: string; + pathname?: string; + search?: string; + hash?: string; +}; + +const fixture = join(import.meta.dir, "../../node/test/fixtures/wpt/url/resources/urltestdata.json"); +const entries = (JSON.parse(readFileSync(fixture, "utf8")) as (Entry | string)[]).filter( + (entry): entry is Entry => typeof entry === "object", +); + +// url.origin for these does not match the spec yet (parsing does); tracked separately from the parser. +const knownOriginDeviations = new Set([ + "ftps:/example.com/", + "ftps:example.com/", + "blob:ftp://host/path", + "blob:ws://example.org/", + "blob:wss://example.org/", +]); + +describe("WPT url-constructor", () => { + test("fixture is present", () => { + expect(entries.length).toBeGreaterThan(800); + }); + + for (const entry of entries) { + const name = `${JSON.stringify(entry.input)}${entry.base != null ? ` against ${JSON.stringify(entry.base)}` : ""}`; + test(name, () => { + const construct = () => (entry.base != null ? new URL(entry.input, entry.base) : new URL(entry.input)); + if (entry.failure) { + expect(construct).toThrow(TypeError); + return; + } + const url = construct(); + expect(url.href).toBe(entry.href); + if (entry.origin !== undefined && !knownOriginDeviations.has(entry.input)) expect(url.origin).toBe(entry.origin); + expect(url.protocol).toBe(entry.protocol); + expect(url.username).toBe(entry.username); + expect(url.password).toBe(entry.password); + expect(url.host).toBe(entry.host); + expect(url.hostname).toBe(entry.hostname); + expect(url.port).toBe(entry.port); + expect(url.pathname).toBe(entry.pathname); + expect(url.search).toBe(entry.search); + expect(url.hash).toBe(entry.hash); + }); + } +}); diff --git a/test/js/web/url/url.test.ts b/test/js/web/url/url.test.ts index dc65093de0fc..3857ed3e9ac0 100755 --- a/test/js/web/url/url.test.ts +++ b/test/js/web/url/url.test.ts @@ -140,6 +140,41 @@ describe("url", () => { expect(hn.hostname).toBe("xn--s5a.com"); }); + it("rejects invalid punycode labels however they are spelled in the input (like Node)", () => { + for (const input of [ + "https://xn--a.com/", + "https://XN--a.com/", + "https://x%6E--a.com/", + "https://x\tn--a.com/", + "https://xn-\n-a/", + "https://xn-\r-a/", + " https://xn--a/", + "https:xn--a/", + "https:\\\\u:p@xn--a\\p", + ]) { + expect(() => new URL(input)).toThrow(TypeError); + expect(URL.canParse(input)).toBe(false); + expect(URL.parse(input)).toBe(null); + } + for (const [input, base] of [ + ["/p", "https://x%6E--a.com/"], + ["//xn--a/p", "https://example.com/"], + ["xn--a", "https://example.com/"], + ]) { + if (input === "xn--a") { + // A relative path never supplies a host. + expect(new URL(input, base).href).toBe("https://example.com/xn--a"); + continue; + } + expect(() => new URL(input, base)).toThrow(TypeError); + expect(URL.canParse(input, base)).toBe(false); + expect(URL.parse(input, base)).toBe(null); + } + expect(new URL("https://xn--ls8h.com/?q=%E3%81#xn--a").href).toBe("https://xn--ls8h.com/?q=%E3%81#xn--a"); + expect(new URL("https://\u{1F4A9}.com/p%20q?xn--a").hostname).toBe("xn--ls8h.com"); + expect(new URL("https://\u{1F4A9}.com/xn--a/%41").pathname).toBe("/xn--a/%41"); + }); + it("prints", () => { // URL.prototype carries [Symbol.for("nodejs.util.inspect.custom")], so // Bun.inspect matches node's util.inspect output.