Skip to content

Commit c390b53

Browse files
anonriganonrig
committed
url: use private properties for brand check
PR-URL: nodejs#46904 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 758474e commit c390b53

File tree

6 files changed

+79
-122
lines changed

6 files changed

+79
-122
lines changed

‎lib/internal/modules/cjs/loader.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ const { BuiltinModule } = require('internal/bootstrap/loaders');
7979
const {
8080
maybeCacheSourceMap,
8181
} = require('internal/source_map/source_map_cache');
82-
const { pathToFileURL, fileURLToPath, isURLInstance } = require('internal/url');
82+
const { pathToFileURL, fileURLToPath, isURL } = require('internal/url');
8383
const {
8484
deprecate,
8585
emitExperimentalWarning,
@@ -1363,7 +1363,7 @@ const createRequireError = 'must be a file URL object, file URL string, or ' +
13631363
function createRequire(filename) {
13641364
let filepath;
13651365

1366-
if (isURLInstance(filename) ||
1366+
if (isURL(filename) ||
13671367
(typeof filename === 'string' && !path.isAbsolute(filename))) {
13681368
try {
13691369
filepath = fileURLToPath(filename);

‎lib/internal/modules/esm/loader.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const {
3131
ERR_INVALID_RETURN_VALUE,
3232
ERR_UNKNOWN_MODULE_FORMAT,
3333
} = require('internal/errors').codes;
34-
const { pathToFileURL, isURLInstance, URL } = require('internal/url');
34+
const { pathToFileURL, isURL, URL } = require('internal/url');
3535
const { emitExperimentalWarning } = require('internal/util');
3636
const {
3737
isAnyArrayBuffer,
@@ -792,7 +792,7 @@ class ESMLoader {
792792
if (
793793
!isMain &&
794794
typeof parentURL !== 'string' &&
795-
!isURLInstance(parentURL)
795+
!isURL(parentURL)
796796
) {
797797
throw new ERR_INVALID_ARG_TYPE(
798798
'parentURL',

‎lib/internal/url.js

Lines changed: 63 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const {
77
ArrayPrototypePush,
88
ArrayPrototypeReduce,
99
ArrayPrototypeSlice,
10-
FunctionPrototypeBind,
10+
Boolean,
1111
Int8Array,
1212
IteratorPrototype,
1313
Number,
@@ -17,7 +17,6 @@ const {
1717
ObjectGetOwnPropertySymbols,
1818
ObjectGetPrototypeOf,
1919
ObjectKeys,
20-
ObjectPrototypeHasOwnProperty,
2120
ReflectGetOwnPropertyDescriptor,
2221
ReflectOwnKeys,
2322
RegExpPrototypeSymbolReplace,
@@ -537,16 +536,27 @@ ObjectDefineProperties(URLSearchParams.prototype, {
537536
},
538537
});
539538

540-
function isURLThis(self) {
541-
return self != null && ObjectPrototypeHasOwnProperty(self, context);
539+
/**
540+
* Checks if a value has the shape of a WHATWG URL object.
541+
*
542+
* Using a symbol or instanceof would not be able to recognize URL objects
543+
* coming from other implementations (e.g. in Electron), so instead we are
544+
* checking some well known properties for a lack of a better test.
545+
*
546+
* @param {*} self
547+
* @returns {self is URL}
548+
*/
549+
function isURL(self) {
550+
return Boolean(self?.href && self.origin);
542551
}
543552

544553
class URL {
554+
#context = new URLContext();
555+
#searchParams;
556+
545557
constructor(input, base = undefined) {
546558
// toUSVString is not needed.
547559
input = `${input}`;
548-
this[context] = new URLContext();
549-
this.#onParseComplete = FunctionPrototypeBind(this.#onParseComplete, this);
550560

551561
if (base !== undefined) {
552562
base = `${base}`;
@@ -562,11 +572,6 @@ class URL {
562572
}
563573

564574
[inspect.custom](depth, opts) {
565-
if (this == null ||
566-
ObjectGetPrototypeOf(this[context]) !== URLContext.prototype) {
567-
throw new ERR_INVALID_THIS('URL');
568-
}
569-
570575
if (typeof depth === 'number' && depth < 0)
571576
return this;
572577

@@ -587,181 +592,133 @@ class URL {
587592
obj.hash = this.hash;
588593

589594
if (opts.showHidden) {
590-
obj[context] = this[context];
595+
obj[context] = this.#context;
591596
}
592597

593598
return `${constructor.name} ${inspect(obj, opts)}`;
594599
}
595600

596601
#onParseComplete = (href, origin, protocol, hostname, pathname,
597602
search, username, password, port, hash) => {
598-
const ctx = this[context];
599-
ctx.href = href;
600-
ctx.origin = origin;
601-
ctx.protocol = protocol;
602-
ctx.hostname = hostname;
603-
ctx.pathname = pathname;
604-
ctx.search = search;
605-
ctx.username = username;
606-
ctx.password = password;
607-
ctx.port = port;
608-
ctx.hash = hash;
609-
if (!this[searchParams]) { // Invoked from URL constructor
610-
this[searchParams] = new URLSearchParams();
611-
this[searchParams][context] = this;
603+
this.#context.href = href;
604+
this.#context.origin = origin;
605+
this.#context.protocol = protocol;
606+
this.#context.hostname = hostname;
607+
this.#context.pathname = pathname;
608+
this.#context.search = search;
609+
this.#context.username = username;
610+
this.#context.password = password;
611+
this.#context.port = port;
612+
this.#context.hash = hash;
613+
if (this.#searchParams) {
614+
this.#searchParams[searchParams] = parseParams(search);
612615
}
613-
initSearchParams(this[searchParams], ctx.search);
614616
};
615617

616618
toString() {
617-
if (!isURLThis(this))
618-
throw new ERR_INVALID_THIS('URL');
619-
return this[context].href;
619+
return this.#context.href;
620620
}
621621

622622
get href() {
623-
if (!isURLThis(this))
624-
throw new ERR_INVALID_THIS('URL');
625-
return this[context].href;
623+
return this.#context.href;
626624
}
627625

628626
set href(value) {
629-
if (!isURLThis(this))
630-
throw new ERR_INVALID_THIS('URL');
631-
const valid = updateUrl(this[context].href, updateActions.kHref, `${value}`, this.#onParseComplete);
627+
const valid = updateUrl(this.#context.href, updateActions.kHref, `${value}`, this.#onParseComplete);
632628
if (!valid) { throw ERR_INVALID_URL(`${value}`); }
633629
}
634630

635631
// readonly
636632
get origin() {
637-
if (!isURLThis(this))
638-
throw new ERR_INVALID_THIS('URL');
639-
return this[context].origin;
633+
return this.#context.origin;
640634
}
641635

642636
get protocol() {
643-
if (!isURLThis(this))
644-
throw new ERR_INVALID_THIS('URL');
645-
return this[context].protocol;
637+
return this.#context.protocol;
646638
}
647639

648640
set protocol(value) {
649-
if (!isURLThis(this))
650-
throw new ERR_INVALID_THIS('URL');
651-
updateUrl(this[context].href, updateActions.kProtocol, `${value}`, this.#onParseComplete);
641+
updateUrl(this.#context.href, updateActions.kProtocol, `${value}`, this.#onParseComplete);
652642
}
653643

654644
get username() {
655-
if (!isURLThis(this))
656-
throw new ERR_INVALID_THIS('URL');
657-
return this[context].username;
645+
return this.#context.username;
658646
}
659647

660648
set username(value) {
661-
if (!isURLThis(this))
662-
throw new ERR_INVALID_THIS('URL');
663-
updateUrl(this[context].href, updateActions.kUsername, `${value}`, this.#onParseComplete);
649+
updateUrl(this.#context.href, updateActions.kUsername, `${value}`, this.#onParseComplete);
664650
}
665651

666652
get password() {
667-
if (!isURLThis(this))
668-
throw new ERR_INVALID_THIS('URL');
669-
return this[context].password;
653+
return this.#context.password;
670654
}
671655

672656
set password(value) {
673-
if (!isURLThis(this))
674-
throw new ERR_INVALID_THIS('URL');
675-
updateUrl(this[context].href, updateActions.kPassword, `${value}`, this.#onParseComplete);
657+
updateUrl(this.#context.href, updateActions.kPassword, `${value}`, this.#onParseComplete);
676658
}
677659

678660
get host() {
679-
if (!isURLThis(this))
680-
throw new ERR_INVALID_THIS('URL');
681-
const port = this[context].port;
661+
const port = this.#context.port;
682662
const suffix = port.length > 0 ? `:${port}` : '';
683-
return this[context].hostname + suffix;
663+
return this.#context.hostname + suffix;
684664
}
685665

686666
set host(value) {
687-
if (!isURLThis(this))
688-
throw new ERR_INVALID_THIS('URL');
689-
updateUrl(this[context].href, updateActions.kHost, `${value}`, this.#onParseComplete);
667+
updateUrl(this.#context.href, updateActions.kHost, `${value}`, this.#onParseComplete);
690668
}
691669

692670
get hostname() {
693-
if (!isURLThis(this))
694-
throw new ERR_INVALID_THIS('URL');
695-
return this[context].hostname;
671+
return this.#context.hostname;
696672
}
697673

698674
set hostname(value) {
699-
if (!isURLThis(this))
700-
throw new ERR_INVALID_THIS('URL');
701-
updateUrl(this[context].href, updateActions.kHostname, `${value}`, this.#onParseComplete);
675+
updateUrl(this.#context.href, updateActions.kHostname, `${value}`, this.#onParseComplete);
702676
}
703677

704678
get port() {
705-
if (!isURLThis(this))
706-
throw new ERR_INVALID_THIS('URL');
707-
return this[context].port;
679+
return this.#context.port;
708680
}
709681

710682
set port(value) {
711-
if (!isURLThis(this))
712-
throw new ERR_INVALID_THIS('URL');
713-
updateUrl(this[context].href, updateActions.kPort, `${value}`, this.#onParseComplete);
683+
updateUrl(this.#context.href, updateActions.kPort, `${value}`, this.#onParseComplete);
714684
}
715685

716686
get pathname() {
717-
if (!isURLThis(this))
718-
throw new ERR_INVALID_THIS('URL');
719-
return this[context].pathname;
687+
return this.#context.pathname;
720688
}
721689

722690
set pathname(value) {
723-
if (!isURLThis(this))
724-
throw new ERR_INVALID_THIS('URL');
725-
updateUrl(this[context].href, updateActions.kPathname, `${value}`, this.#onParseComplete);
691+
updateUrl(this.#context.href, updateActions.kPathname, `${value}`, this.#onParseComplete);
726692
}
727693

728694
get search() {
729-
if (!isURLThis(this))
730-
throw new ERR_INVALID_THIS('URL');
731-
return this[context].search;
695+
return this.#context.search;
732696
}
733697

734-
set search(search) {
735-
if (!isURLThis(this))
736-
throw new ERR_INVALID_THIS('URL');
737-
search = toUSVString(search);
738-
updateUrl(this[context].href, updateActions.kSearch, search, this.#onParseComplete);
739-
initSearchParams(this[searchParams], this[context].search);
698+
set search(value) {
699+
updateUrl(this.#context.href, updateActions.kSearch, toUSVString(value), this.#onParseComplete);
740700
}
741701

742702
// readonly
743703
get searchParams() {
744-
if (!isURLThis(this))
745-
throw new ERR_INVALID_THIS('URL');
746-
return this[searchParams];
704+
// Create URLSearchParams on demand to greatly improve the URL performance.
705+
if (this.#searchParams == null) {
706+
this.#searchParams = new URLSearchParams(this.#context.search);
707+
this.#searchParams[context] = this;
708+
}
709+
return this.#searchParams;
747710
}
748711

749712
get hash() {
750-
if (!isURLThis(this))
751-
throw new ERR_INVALID_THIS('URL');
752-
return this[context].hash;
713+
return this.#context.hash;
753714
}
754715

755716
set hash(value) {
756-
if (!isURLThis(this))
757-
throw new ERR_INVALID_THIS('URL');
758-
updateUrl(this[context].href, updateActions.kHash, `${value}`, this.#onParseComplete);
717+
updateUrl(this.#context.href, updateActions.kHash, `${value}`, this.#onParseComplete);
759718
}
760719

761720
toJSON() {
762-
if (!isURLThis(this))
763-
throw new ERR_INVALID_THIS('URL');
764-
return this[context].href;
721+
return this.#context.href;
765722
}
766723

767724
static createObjectURL(obj) {
@@ -1219,7 +1176,7 @@ function getPathFromURLPosix(url) {
12191176
function fileURLToPath(path) {
12201177
if (typeof path === 'string')
12211178
path = new URL(path);
1222-
else if (!isURLInstance(path))
1179+
else if (!isURL(path))
12231180
throw new ERR_INVALID_ARG_TYPE('path', ['string', 'URL'], path);
12241181
if (path.protocol !== 'file:')
12251182
throw new ERR_INVALID_URL_SCHEME('file');
@@ -1295,12 +1252,8 @@ function pathToFileURL(filepath) {
12951252
return outURL;
12961253
}
12971254

1298-
function isURLInstance(fileURLOrPath) {
1299-
return fileURLOrPath != null && fileURLOrPath.href && fileURLOrPath.origin;
1300-
}
1301-
13021255
function toPathIfFileURL(fileURLOrPath) {
1303-
if (!isURLInstance(fileURLOrPath))
1256+
if (!isURL(fileURLOrPath))
13041257
return fileURLOrPath;
13051258
return fileURLToPath(fileURLOrPath);
13061259
}
@@ -1310,12 +1263,12 @@ module.exports = {
13101263
fileURLToPath,
13111264
pathToFileURL,
13121265
toPathIfFileURL,
1313-
isURLInstance,
13141266
URL,
13151267
URLSearchParams,
13161268
domainToASCII,
13171269
domainToUnicode,
13181270
urlToHttpOptions,
13191271
searchParamsSymbol: searchParams,
13201272
encodeStr,
1273+
isURL,
13211274
};

‎lib/internal/worker.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const {
5555
WritableWorkerStdio,
5656
} = workerIo;
5757
const { deserializeError } = require('internal/error_serdes');
58-
const { fileURLToPath, isURLInstance, pathToFileURL } = require('internal/url');
58+
const { fileURLToPath, isURL, pathToFileURL } = require('internal/url');
5959
const { kEmptyObject } = require('internal/util');
6060
const { validateArray, validateString } = require('internal/validators');
6161

@@ -145,13 +145,13 @@ class Worker extends EventEmitter {
145145
}
146146
url = null;
147147
doEval = 'classic';
148-
} else if (isURLInstance(filename) && filename.protocol === 'data:') {
148+
} else if (isURL(filename) && filename.protocol === 'data:') {
149149
url = null;
150150
doEval = 'module';
151151
filename = `import ${JSONStringify(`${filename}`)}`;
152152
} else {
153153
doEval = false;
154-
if (isURLInstance(filename)) {
154+
if (isURL(filename)) {
155155
url = filename;
156156
filename = fileURLToPath(filename);
157157
} else if (typeof filename !== 'string') {

‎test/parallel/test-whatwg-url-custom-inspect.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ assert.strictEqual(
6161

6262
assert.strictEqual(
6363
util.inspect({ a: url }, { depth: 0 }),
64-
'{ a: [URL] }');
64+
'{ a: URL {} }');
6565

6666
class MyURL extends URL {}
6767
assert(util.inspect(new MyURL(url.href)).startsWith('MyURL {'));

0 commit comments

Comments
 (0)