url: add value argument to has and delete methods

The change aims to add value argument to two methods of URLSearchParams
class i.e the has method and the delete method. For has method, if
value argument is provided, then use it to check for presence. For
delete method, if value argument provided, use it to delete.

Fixes: https://github.com/nodejs/node/issues/47883
PR-URL: https://github.com/nodejs/node/pull/47885
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>

benchmark/common.js

@@ -360,8 +360,9 @@ function getUrlData(withBase) {
   for (const item of data) {
     if (item.failure || !item.input) continue;
     if (withBase) {
-      result.push([item.input, item.base]);
-    } else if (item.base !== 'about:blank') {
+      // item.base might be null. It should be converted into `undefined`.
+      result.push([item.input, item.base ?? undefined]);
+    } else if (item.base !== null) {
       result.push(item.base);
     }
   }

doc/api/url.md

@@ -859,11 +859,22 @@ new URLSearchParams([
 
 Append a new name-value pair to the query string.
 
-#### `urlSearchParams.delete(name)`
+#### `urlSearchParams.delete(name[, value])`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47885
+    description: Add support for optional `value` argument.
+-->
 
 * `name` {string}
+* `value` {string}
 
-Remove all name-value pairs whose name is `name`.
+If `value` is provided, removes all name-value pairs
+where name is `name` and value is `value`..
+
+If `value` is not provided, removes all name-value pairs whose name is `name`.
 
 #### `urlSearchParams.entries()`
 
@@ -918,12 +929,27 @@ are no such pairs, `null` is returned.
 Returns the values of all name-value pairs whose name is `name`. If there are
 no such pairs, an empty array is returned.
 
-#### `urlSearchParams.has(name)`
+#### `urlSearchParams.has(name[, value])`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47885
+    description: Add support for optional `value` argument.
+-->
 
 * `name` {string}
+* `value` {string}
 * Returns: {boolean}
 
-Returns `true` if there is at least one name-value pair whose name is `name`.
+Checks if the `URLSearchParams` object contains key-value pair(s) based on
+`name` and an optional `value` argument.
+
+If `value` is provided, returns `true` when name-value pair with
+same `name` and `value` exists.
+
+If `value` is not provided, returns `true` if there is at least one name-value
+pair whose name is `name`.
 
 #### `urlSearchParams.keys()`
 

lib/internal/url.js

@@ -435,7 +435,7 @@ class URLSearchParams {
     }
   }
 
-  delete(name) {
+  delete(name, value = undefined) {
     if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
@@ -445,12 +445,23 @@ class URLSearchParams {
 
     const list = this.#searchParams;
     name = toUSVString(name);
-    for (let i = 0; i < list.length;) {
-      const cur = list[i];
-      if (cur === name) {
-        list.splice(i, 2);
-      } else {
-        i += 2;
+
+    if (value !== undefined) {
+      value = toUSVString(value);
+      for (let i = 0; i < list.length;) {
+        if (list[i] === name && list[i + 1] === value) {
+          list.splice(i, 2);
+        } else {
+          i += 2;
+        }
+      }
+    } else {
+      for (let i = 0; i < list.length;) {
+        if (list[i] === name) {
+          list.splice(i, 2);
+        } else {
+          i += 2;
+        }
       }
     }
     if (this.#context) {
@@ -495,7 +506,7 @@ class URLSearchParams {
     return values;
   }
 
-  has(name) {
+  has(name, value = undefined) {
     if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
@@ -505,11 +516,19 @@ class URLSearchParams {
 
     const list = this.#searchParams;
     name = toUSVString(name);
+
+    if (value !== undefined) {
+      value = toUSVString(value);
+    }
+
     for (let i = 0; i < list.length; i += 2) {
       if (list[i] === name) {
-        return true;
+        if (value === undefined || list[i + 1] === value) {
+          return true;
+        }
       }
     }
+
     return false;
   }
 

test/fixtures/wpt/README.md

@@ -27,7 +27,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/919874f84f/resources
 - streams: https://github.com/web-platform-tests/wpt/tree/51750bc8d7/streams
-- url: https://github.com/web-platform-tests/wpt/tree/7c5c3cc125/url
+- url: https://github.com/web-platform-tests/wpt/tree/c4726447f3/url
 - user-timing: https://github.com/web-platform-tests/wpt/tree/df24fb604e/user-timing
 - wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/cde25e7e3c/wasm/jsapi
 - wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi

test/fixtures/wpt/url/README.md

@@ -1,31 +1,29 @@
 ## urltestdata.json
 
-These tests are for browsers, but the data for
-`a-element.html`, `url-constructor.html`, `a-element-xhtml.xhtml`, and `failure.html`
-is in `resources/urltestdata.json` and can be re-used by non-browser implementations.
-This file contains a JSON array of comments as strings and test cases as objects.
-The keys for each test case are:
+`resources/urltestdata.json` contains URL parsing tests suitable for any URL parser implementation.
 
-* `base`: an absolute URL as a string whose [parsing] without a base of its own must succeed.
-  This key is always present,
-  and may have a value like `"about:blank"` when `input` is an absolute URL.
-* `input`: an URL as a string to be [parsed][parsing] with `base` as its base URL.
-* Either:
-  * `failure` with the value `true`, indicating that parsing `input` should return failure,
-  * or `href`, `origin`, `protocol`, `username`, `password`, `host`, `hostname`, `port`,
-    `pathname`, `search`, and `hash` with string values;
-    indicating that parsing `input` should return an URL record
-    and that the getters of each corresponding attribute in that URL’s [API]
-    should return the corresponding value.
+It's used as a source of tests by `a-element.html`, `failure.html`, `url-constructor.any.js`, and
+other test files in this directory.
 
-    The `origin` key may be missing.
-    In that case, the API’s `origin` attribute is not tested.
+The format of `resources/urltestdata.json` is a JSON array of comments as strings and test cases as
+objects. The keys for each test case are:
 
-In addition to testing that parsing `input` against `base` gives the result, a test harness for the
-`URL` constructor (or similar APIs) should additionally test the following pattern: if `failure` is
-true, parsing `about:blank` against `input` must give failure. This tests that the logic for
-converting base URLs into strings properly fails the whole parsing algorithm if the base URL cannot
-be parsed.
+* `input`: a string to be parsed as URL.
+* `base`: null or a serialized URL (i.e., does not fail parsing).
+* Then either
+
+  * `failure` whose value is `true`, indicating that parsing `input` relative to `base` returns
+    failure
+  * `relativeTo` whose value is "`non-opaque-path-base`" (input does parse against a non-null base
+    URL without an opaque path) or "`any-base`" (input parses against any non-null base URL), or is
+    omitted in its entirety (input never parses successfully)
+
+  or `href`, `origin`, `protocol`, `username`, `password`, `host`, `hostname`, `port`,
+  `pathname`, `search`, and `hash` with string values; indicating that parsing `input` should return
+  an URL record and that the getters of each corresponding attribute in that URL’s [API] should
+  return the corresponding value.
+
+  The `origin` key may be missing. In that case, the API’s `origin` attribute is not tested.
 
 ## setters_tests.json
 

test/fixtures/wpt/url/failure.html

@@ -9,14 +9,15 @@
 promise_test(() => fetch("resources/urltestdata.json").then(res => res.json()).then(runTests), "Loading data…")
 
 function runTests(testData) {
-  for(const test of testData) {
-    if (typeof test === "string" || !test.failure || test.base !== "about:blank") {
-      continue
+  for (const test of testData) {
+    if (typeof test === "string" || !test.failure || test.base !== null) {
+      continue;
     }
 
     const name = test.input + " should throw"
 
-    self.test(() => { // URL's constructor's first argument is tested by url-constructor.html
+    self.test(() => {
+      // URL's constructor's first argument is tested by url-constructor.html
       // If a URL fails to parse with any valid base, it must also fail to parse with no base, i.e.
       // when used as a base URL itself.
       assert_throws_js(TypeError, () => new URL("about:blank", test.input));
@@ -30,7 +31,7 @@ function runTests(testData) {
     // The following use cases resolve the URL input relative to the current
     // document's URL. If this test input could be construed as a valid URL
     // when resolved against a base URL, skip these cases.
-    if (!test.inputCanBeRelative) {
+    if (test.relativeTo === undefined) {
       self.test(() => {
         const client = new XMLHttpRequest()
         assert_throws_dom("SyntaxError", () => client.open("GET", test.input))

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

@@ -36,4 +36,11 @@ test(() => {
   assert_throws_dom("DataCloneError", () => self.structuredClone(new URLSearchParams()));
 }, "URLSearchParams: no structured serialize/deserialize support");
 
+test(() => {
+  const url = new URL("about:blank");
+  url.toString = () => { throw 1 };
+  assert_throws_exactly(1, () => new URL(url), "url argument");
+  assert_throws_exactly(1, () => new URL("about:blank", url), "base argument");
+}, "Constructor only takes strings");
+
 done();

test/fixtures/wpt/url/resources/a-element-origin.js

@@ -5,23 +5,28 @@ function setBase(base) {
 }
 
 function bURL(url, base) {
-  base = base || "about:blank"
-  setBase(base)
-  var a = document.createElement("a")
-  a.setAttribute("href", url)
-  return a
+  setBase(base);
+  const a = document.createElement("a");
+  a.setAttribute("href", url);
+  return a;
 }
 
-function runURLTests(urltests) {
-  for(var i = 0, l = urltests.length; i < l; i++) {
-    var expected = urltests[i]
-    if (typeof expected === "string" || !("origin" in expected)) continue
-    // skip without base because you cannot unset the baseURL of a document
-    if (expected.base === null) continue;
+function runURLTests(urlTests) {
+  for (const expected of urlTests) {
+    // Skip comments and tests without "origin" expectation
+    if (typeof expected === "string" || !("origin" in expected))
+      continue;
+
+    // Fragments are relative against "about:blank" (this might always be redundant due to requiring "origin" in expected)
+    if (expected.base === null && expected.input.startsWith("#"))
+      continue;
+
+    // We cannot use a null base for HTML tests
+    const base = expected.base === null ? "about:blank" : expected.base;
 
     test(function() {
-      var url = bURL(expected.input, expected.base)
+      var url = bURL(expected.input, base)
       assert_equals(url.origin, expected.origin, "origin")
-    }, "Parsing origin: <" + expected.input + "> against <" + expected.base + ">")
+    }, "Parsing origin: <" + expected.input + "> against <" + base + ">")
   }
 }

test/fixtures/wpt/url/resources/a-element.js

@@ -1,23 +1,28 @@
 promise_test(() => fetch("resources/urltestdata.json").then(res => res.json()).then(runURLTests), "Loading data…");
 
 function setBase(base) {
-  document.getElementById("base").href = base
+  document.getElementById("base").href = base;
 }
 
 function bURL(url, base) {
-  base = base || "about:blank"
-  setBase(base)
-  var a = document.createElement("a")
-  a.setAttribute("href", url)
-  return a
+  setBase(base);
+  const a = document.createElement("a");
+  a.setAttribute("href", url);
+  return a;
 }
 
-function runURLTests(urltests) {
-  for(var i = 0, l = urltests.length; i < l; i++) {
-    var expected = urltests[i]
-    if (typeof expected === "string") continue // skip comments
-    // skip without base because you cannot unset the baseURL of a document
-    if (expected.base === null) continue;
+function runURLTests(urlTests) {
+  for (const expected of urlTests) {
+    // Skip comments
+    if (typeof expected === "string")
+      continue;
+
+    // Fragments are relative against "about:blank"
+    if (expected.relativeTo === "any-base")
+      continue;
+
+    // We cannot use a null base for HTML tests
+    const base = expected.base === null ? "about:blank" : expected.base;
 
     function getKey(expected) {
       if (expected.protocol) {
@@ -30,7 +35,7 @@ function runURLTests(urltests) {
     }
 
     subsetTestByKey(getKey(expected), test, function() {
-      var url = bURL(expected.input, expected.base)
+      var url = bURL(expected.input, base)
       if(expected.failure) {
         if(url.protocol !== ':') {
           assert_unreached("Expected URL to fail parsing")
@@ -49,6 +54,6 @@ function runURLTests(urltests) {
       assert_equals(url.pathname, expected.pathname, "pathname")
       assert_equals(url.search, expected.search, "search")
       assert_equals(url.hash, expected.hash, "hash")
-    }, "Parsing: <" + expected.input + "> against <" + expected.base + ">")
+    }, "Parsing: <" + expected.input + "> against <" + base + ">")
   }
 }

test/fixtures/wpt/url/url-constructor.any.js

@@ -5,14 +5,13 @@
 // META: variant=?include=mailto
 // META: variant=?exclude=(file|javascript|mailto)
 
-function bURL(url, base) {
-  return base ? new URL(url, base) : new URL(url)
-}
+function runURLTests(urlTests) {
+  for (const expected of urlTests) {
+    // Skip comments
+    if (typeof expected === "string")
+      continue;
 
-function runURLTests(urltests) {
-  for(var i = 0, l = urltests.length; i < l; i++) {
-    var expected = urltests[i]
-    if (typeof expected === "string") continue // skip comments
+    const base = expected.base !== null ? expected.base : undefined;
 
     function getKey(expected) {
       if (expected.protocol) {
@@ -27,12 +26,12 @@ function runURLTests(urltests) {
     subsetTestByKey(getKey(expected), test, function() {
       if (expected.failure) {
         assert_throws_js(TypeError, function() {
-          bURL(expected.input, expected.base)
-        })
-        return
+          new URL(expected.input, base);
+        });
+        return;
       }
 
-      var url = bURL(expected.input, expected.base)
+      const url = new URL(expected.input, base);
       assert_equals(url.href, expected.href, "href")
       assert_equals(url.protocol, expected.protocol, "protocol")
       assert_equals(url.username, expected.username, "username")
@@ -47,7 +46,7 @@ function runURLTests(urltests) {
         assert_equals(url.searchParams.toString(), expected.searchParams, "searchParams")
       }
       assert_equals(url.hash, expected.hash, "hash")
-    }, "Parsing: <" + expected.input + "> against <" + expected.base + ">")
+    }, `Parsing: <${expected.input}> ${base ? "against <" + base + ">" : "without base"}`)
   }
 }
 

test/fixtures/wpt/url/url-origin.any.js

@@ -1,17 +1,16 @@
 promise_test(() => fetch("resources/urltestdata.json").then(res => res.json()).then(runURLTests), "Loading data…");
 
-function bURL(url, base) {
-  return base ? new URL(url, base) : new URL(url)
-}
+function runURLTests(urlTests) {
+  for (const expected of urlTests) {
+    // Skip comments and tests without "origin" expectation
+    if (typeof expected === "string" || !("origin" in expected))
+      continue;
 
-function runURLTests(urltests) {
-  for(var i = 0, l = urltests.length; i < l; i++) {
-    var expected = urltests[i]
-    if (typeof expected === "string" || !("origin" in expected)) continue
+    const base = expected.base !== null ? expected.base : undefined;
 
-    test(function() {
-      var url = bURL(expected.input, expected.base)
-      assert_equals(url.origin, expected.origin, "origin")
-    }, "Origin parsing: <" + expected.input + "> against <" + expected.base + ">")
+    test(() => {
+      const url = new URL(expected.input, base);
+      assert_equals(url.origin, expected.origin, "origin");
+    }, `Origin parsing: <${expected.input}> ${base ? "against <" + base + ">" : "without base"}`);
   }
 }

test/fixtures/wpt/url/urlsearchparams-delete.any.js

@@ -61,3 +61,12 @@ test(() => {
   assert_equals(url.pathname, 'space    ');
   assert_equals(url.href, 'data:space    #test');
 }, 'Changing the query of a URL with an opaque path can impact the path if the URL has no fragment');
+
+test(() => {
+  const params = new URLSearchParams();
+  params.append('a', 'b');
+  params.append('a', 'c');
+  params.append('a', 'd');
+  params.delete('a', 'c');
+  assert_equals(params.toString(), 'a=b&a=d');
+}, "Two-argument delete()");

test/fixtures/wpt/url/urlsearchparams-has.any.js

@@ -22,3 +22,16 @@ test(function() {
     params.delete('first');
     assert_false(params.has('first'), 'Search params object has no name "first"');
 }, 'has() following delete()');
+
+test(() => {
+  const params = new URLSearchParams("a=b&a=d&c&e&");
+  assert_true(params.has('a', 'b'));
+  assert_false(params.has('a', 'c'));
+  assert_true(params.has('a', 'd'));
+  assert_true(params.has('e', ''));
+  params.append('first', null);
+  assert_false(params.has('first', ''));
+  assert_true(params.has('first', 'null'));
+  params.delete('a', 'b');
+  assert_true(params.has('a', 'd'));
+}, "Two-argument has()");

test/fixtures/wpt/versions.json

@@ -68,7 +68,7 @@
     "path": "streams"
   },
   "url": {
-    "commit": "7c5c3cc125979b4768d414471e6ab655b473aae8",
+    "commit": "c4726447f3bad1c71244fb99c98738ff0354a034",
     "path": "url"
   },
   "user-timing": {