Skip to content

Commit 0063448

Browse files
joyeecheungBridgeAR
authored andcommitted
url: make the context non-enumerable
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>
1 parent e46b8ed commit 0063448

File tree

2 files changed

+27
-2
lines changed

2 files changed

+27
-2
lines changed

lib/internal/url.js

+13-2
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,14 @@ function onParseError(flags, input) {
245245
// Reused by URL constructor and URL#href setter.
246246
function parse(url, input, base) {
247247
const base_context = base ? base[context] : undefined;
248-
url[context] = new URLContext();
248+
// In the URL#href setter
249+
if (!url[context]) {
250+
Object.defineProperty(url, context, {
251+
enumerable: false,
252+
configurable: false,
253+
value: new URLContext()
254+
});
255+
}
249256
_parse(input.trim(), -1, base_context, undefined,
250257
onParseComplete.bind(url), onParseError);
251258
}
@@ -1437,7 +1444,11 @@ function toPathIfFileURL(fileURLOrPath) {
14371444
}
14381445

14391446
function NativeURL(ctx) {
1440-
this[context] = ctx;
1447+
Object.defineProperty(this, context, {
1448+
enumerable: false,
1449+
configurable: false,
1450+
value: ctx
1451+
});
14411452
}
14421453
NativeURL.prototype = URL.prototype;
14431454

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
'use strict';
2+
// This tests that the context of URL objects are not
3+
// enumerable and thus considered by assert libraries.
4+
// See https://github.com/nodejs/node/issues/24211
5+
6+
// Tests below are not from WPT.
7+
8+
require('../common');
9+
const assert = require('assert');
10+
11+
assert.deepStrictEqual(
12+
new URL('./foo', 'https://example.com/'),
13+
new URL('https://example.com/foo')
14+
);

0 commit comments

Comments
 (0)