url: fix url spec compliance issues

lib/internal/url.js

@@ -143,7 +143,6 @@ class URLContext {
   href = '';
   origin = '';
   protocol = '';
-  host = '';
   hostname = '';
   pathname = '';
   search = '';
@@ -626,14 +625,13 @@ class URL {
     return constructHref(this[context], options);
   }
 
-  #onParseComplete = (href, origin, protocol, host, hostname, pathname,
+  #onParseComplete = (href, origin, protocol, hostname, pathname,
                       search, username, password, port, hash, hasHost,
                       hasOpaquePath) => {
     const ctx = this[context];
     ctx.href = href;
     ctx.origin = origin;
     ctx.protocol = protocol;
-    ctx.host = host;
     ctx.hostname = hostname;
     ctx.pathname = pathname;
     ctx.search = search;
@@ -716,7 +714,9 @@ class URL {
   get host() {
     if (!isURLThis(this))
       throw new ERR_INVALID_THIS('URL');
-    return this[context].host;
+    const port = this[context].port;
+    const suffix = port.length > 0 ? `:${port}` : '';
+    return this[context].hostname + suffix;
   }
 
   set host(value) {
@@ -864,6 +864,19 @@ function update(url, params) {
     ctx.search = '?' + serializedParams;
   } else {
     ctx.search = '';
+
+    // Potentially strip trailing spaces from an opaque path
+    if (ctx.hasOpaquePath && ctx.hash.length === 0) {
+      let length = ctx.pathname.length;
+      while (length > 0 && ctx.pathname.charCodeAt(length - 1) === 32) {
+        length--;
+      }
+
+      // No need to copy the whole string if there is no space at the end
+      if (length !== ctx.pathname.length) {
+        ctx.pathname = ctx.pathname.slice(0, length);
+      }
+    }
   }
   ctx.href = constructHref(ctx);
 }

src/node_url.cc

@@ -52,16 +52,15 @@ void SetArgs(Environment* env, Local<Value> argv[12], const ada::result& url) {
   argv[0] = Utf8String(isolate, url->get_href());
   argv[1] = Utf8String(isolate, url->get_origin());
   argv[2] = Utf8String(isolate, url->get_protocol());
-  argv[3] = Utf8String(isolate, url->get_host());
-  argv[4] = Utf8String(isolate, url->get_hostname());
-  argv[5] = Utf8String(isolate, url->get_pathname());
-  argv[6] = Utf8String(isolate, url->get_search());
-  argv[7] = Utf8String(isolate, url->get_username());
-  argv[8] = Utf8String(isolate, url->get_password());
-  argv[9] = Utf8String(isolate, url->get_port());
-  argv[10] = Utf8String(isolate, url->get_hash());
-  argv[11] = Boolean::New(isolate, url->host.has_value());
-  argv[12] = Boolean::New(isolate, url->has_opaque_path);
+  argv[3] = Utf8String(isolate, url->get_hostname());
+  argv[4] = Utf8String(isolate, url->get_pathname());
+  argv[5] = Utf8String(isolate, url->get_search());
+  argv[6] = Utf8String(isolate, url->get_username());
+  argv[7] = Utf8String(isolate, url->get_password());
+  argv[8] = Utf8String(isolate, url->get_port());
+  argv[9] = Utf8String(isolate, url->get_hash());
+  argv[10] = Boolean::New(isolate, url->host.has_value());
+  argv[11] = Boolean::New(isolate, url->has_opaque_path);
 }
 
 void Parse(const FunctionCallbackInfo<Value>& args) {
@@ -108,7 +107,6 @@ void Parse(const FunctionCallbackInfo<Value>& args) {
       undef,
       undef,
       undef,
-      undef,
   };
   SetArgs(env, argv, out);
   USE(success_callback_->Call(
@@ -259,7 +257,6 @@ void UpdateUrl(const FunctionCallbackInfo<Value>& args) {
       undef,
       undef,
       undef,
-      undef,
   };
   SetArgs(env, argv, out);
   USE(success_callback_->Call(

test/fixtures/wpt/README.md

@@ -28,7 +28,7 @@ Last update:
 - resource-timing: https://github.com/web-platform-tests/wpt/tree/22d38586d0/resource-timing
 - resources: https://github.com/web-platform-tests/wpt/tree/fbf1e7d247/resources
 - streams: https://github.com/web-platform-tests/wpt/tree/9e5ef42bd3/streams
-- url: https://github.com/web-platform-tests/wpt/tree/0a187bc169/url
+- url: https://github.com/web-platform-tests/wpt/tree/f1ade799d0/url
 - user-timing: https://github.com/web-platform-tests/wpt/tree/df24fb604e/user-timing
 - wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/d8dbe6990b/wasm/jsapi
 - wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi

test/fixtures/wpt/url/IdnaTestV2.window.js

@@ -0,0 +1,41 @@
+promise_test(() => fetch("resources/IdnaTestV2.json").then(res => res.json()).then(runTests), "Loading data…");
+
+// Performance impact of this seems negligible (performance.now() diff in WebKit went from 48 to 52)
+// and there was a preference to let more non-ASCII hit the parser.
+function encodeHostEndingCodePoints(input) {
+  let output = "";
+  for (const codePoint of input) {
+    if ([":", "/", "?", "#", "\\"].includes(codePoint)) {
+      output += encodeURIComponent(codePoint);
+    } else {
+      output += codePoint;
+    }
+  }
+  return output;
+}
+
+function runTests(idnaTests) {
+  for (const idnaTest of idnaTests) {
+    if (typeof idnaTest === "string") {
+      continue // skip comments
+    }
+    if (idnaTest.input === "") {
+      continue // cannot test empty string input through new URL()
+    }
+    // Percent-encode the input such that ? and equivalent code points do not end up counting as
+    // part of the URL, but are parsed through the host parser instead.
+    const encodedInput = encodeHostEndingCodePoints(idnaTest.input);
+
+    test(() => {
+      if (idnaTest.output === null) {
+        assert_throws_js(TypeError, () => new URL(`https://${encodedInput}/x`));
+      } else {
+        const url = new URL(`https://${encodedInput}/x`);
+        assert_equals(url.host, idnaTest.output);
+        assert_equals(url.hostname, idnaTest.output);
+        assert_equals(url.pathname, "/x");
+        assert_equals(url.href, `https://${idnaTest.output}/x`);
+      }
+    }, `ToASCII("${idnaTest.input}")${idnaTest.comment ? " " + idnaTest.comment : ""}`);
+  }
+}

test/fixtures/wpt/url/a-element-xhtml.xhtml

@@ -3,8 +3,13 @@
 <html xmlns="http://www.w3.org/1999/xhtml">
   <head>
     <title>URL Test</title>
+    <meta name="variant" content="?include=file"/>
+    <meta name="variant" content="?include=javascript"/>
+    <meta name="variant" content="?include=mailto"/>
+    <meta name="variant" content="?exclude=(file|javascript|mailto)"/>
     <script src="/resources/testharness.js"></script>
     <script src="/resources/testharnessreport.js"></script>
+    <script src="/common/subset-tests-by-key.js"></script>
     <base id="base"/>
   </head>
   <body>

test/fixtures/wpt/url/a-element.html

@@ -1,7 +1,12 @@
 <!doctype html>
 <meta charset=utf-8>
+<meta name="variant" content="?include=file">
+<meta name="variant" content="?include=javascript">
+<meta name="variant" content="?include=mailto">
+<meta name="variant" content="?exclude=(file|javascript|mailto)">
 <script src=/resources/testharness.js></script>
 <script src=/resources/testharnessreport.js></script>
+<script src="/common/subset-tests-by-key.js"></script>
 <base id=base>
 <div id=log></div>
 <script src=resources/a-element.js></script>

test/fixtures/wpt/url/historical.any.js

@@ -28,4 +28,12 @@ test(function() {
   assert_equals(URL.domainToUnicode, undefined);
 }, "URL.domainToUnicode should be undefined");
 
+test(() => {
+  assert_throws_dom("DataCloneError", () => self.structuredClone(new URL("about:blank")));
+}, "URL: no structured serialize/deserialize support");
+
+test(() => {
+  assert_throws_dom("DataCloneError", () => self.structuredClone(new URLSearchParams()));
+}, "URLSearchParams: no structured serialize/deserialize support");
+
 done();