Skip to content

Commit

Permalink
url: check forEach callback is a function
Browse files Browse the repository at this point in the history
The Web IDL spec mandates such a check.

Also make error messages consistent with rest of Node.js and add
additional tests for forEach().

PR-URL: #10905
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
TimothyGu authored and italoacasas committed Jan 30, 2017
1 parent 8778fca commit 95faa55
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 10 deletions.
18 changes: 8 additions & 10 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -672,8 +672,7 @@ class URLSearchParams {
throw new TypeError('Value of `this` is not a URLSearchParams');
}
if (arguments.length < 2) {
throw new TypeError(
'Both `name` and `value` arguments need to be specified');
throw new TypeError('"name" and "value" arguments must be specified');
}

name = String(name);
Expand All @@ -687,7 +686,7 @@ class URLSearchParams {
throw new TypeError('Value of `this` is not a URLSearchParams');
}
if (arguments.length < 1) {
throw new TypeError('The `name` argument needs to be specified');
throw new TypeError('"name" argument must be specified');
}

const list = this[searchParams];
Expand All @@ -708,8 +707,7 @@ class URLSearchParams {
throw new TypeError('Value of `this` is not a URLSearchParams');
}
if (arguments.length < 2) {
throw new TypeError(
'Both `name` and `value` arguments need to be specified');
throw new TypeError('"name" and "value" arguments must be specified');
}

const list = this[searchParams];
Expand Down Expand Up @@ -749,7 +747,7 @@ class URLSearchParams {
throw new TypeError('Value of `this` is not a URLSearchParams');
}
if (arguments.length < 1) {
throw new TypeError('The `name` argument needs to be specified');
throw new TypeError('"name" argument must be specified');
}

const list = this[searchParams];
Expand All @@ -767,7 +765,7 @@ class URLSearchParams {
throw new TypeError('Value of `this` is not a URLSearchParams');
}
if (arguments.length < 1) {
throw new TypeError('The `name` argument needs to be specified');
throw new TypeError('"name" argument must be specified');
}

const list = this[searchParams];
Expand All @@ -786,7 +784,7 @@ class URLSearchParams {
throw new TypeError('Value of `this` is not a URLSearchParams');
}
if (arguments.length < 1) {
throw new TypeError('The `name` argument needs to be specified');
throw new TypeError('"name" argument must be specified');
}

const list = this[searchParams];
Expand Down Expand Up @@ -814,8 +812,8 @@ class URLSearchParams {
if (!this || !(this instanceof URLSearchParams)) {
throw new TypeError('Value of `this` is not a URLSearchParams');
}
if (arguments.length < 1) {
throw new TypeError('The `callback` argument needs to be specified');
if (typeof callback !== 'function') {
throw new TypeError('"callback" argument must be a function');
}

let list = this[searchParams];
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-whatwg-url-searchparams.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ n = 0;
for (val of sp.values()) {
assert.strictEqual(val, String(values[n++]));
}
n = 0;
sp.forEach(function(val, key, obj) {
assert.strictEqual(this, undefined);
assert.strictEqual(key, 'a');
assert.strictEqual(val, String(values[n++]));
assert.strictEqual(obj, sp);
});
sp.forEach(function() {
assert.strictEqual(this, m);
}, m);
assert.throws(() => sp.forEach(),
/^TypeError: "callback" argument must be a function$/);
assert.throws(() => sp.forEach(1),
/^TypeError: "callback" argument must be a function$/);

m.search = '?a=a&b=b';
assert.strictEqual(sp.toString(), 'a=a&b=b');

0 comments on commit 95faa55

Please sign in to comment.