Skip to content

Commit

Permalink
url: make the context non-enumerable
Browse files Browse the repository at this point in the history
At the moment we expose the context as a normal property on the
prototype chain of URL or take them from the base URL
which makes them enumerable and considered
by assert libraries even though the context carries path-dependent
information that do not affect the equivalence of these objects.
This patch fixes it in a minimal manner by marking the context
non-enumerable as making it full private would require more
refactoring and can be done in a bigger patch later.

PR-URL: #24218
Refs: #24211
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
  • Loading branch information
joyeecheung authored and Trott committed Nov 9, 2018
1 parent 457cde3 commit 5e1bf05
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
15 changes: 13 additions & 2 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,14 @@ function onParseError(flags, input) {
// Reused by URL constructor and URL#href setter.
function parse(url, input, base) {
const base_context = base ? base[context] : undefined;
url[context] = new URLContext();
// In the URL#href setter
if (!url[context]) {
Object.defineProperty(url, context, {
enumerable: false,
configurable: false,
value: new URLContext()
});
}
_parse(input.trim(), -1, base_context, undefined,
onParseComplete.bind(url), onParseError);
}
Expand Down Expand Up @@ -1437,7 +1444,11 @@ function toPathIfFileURL(fileURLOrPath) {
}

function NativeURL(ctx) {
this[context] = ctx;
Object.defineProperty(this, context, {
enumerable: false,
configurable: false,
value: ctx
});
}
NativeURL.prototype = URL.prototype;

Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-whatwg-url-custom-no-enumerable-context.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';
// This tests that the context of URL objects are not
// enumerable and thus considered by assert libraries.
// See https://github.com/nodejs/node/issues/24211

// Tests below are not from WPT.

require('../common');
const assert = require('assert');

assert.deepStrictEqual(
new URL('./foo', 'https://example.com/'),
new URL('https://example.com/foo')
);

0 comments on commit 5e1bf05

Please sign in to comment.