Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

url: refactor to use more primordials #45966

Merged
merged 11 commits into from
Jan 22, 2023
30 changes: 30 additions & 0 deletions benchmark/url/whatwgurl-to-and-from-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
'use strict';
const common = require('../common.js');
const { fileURLToPath, pathToFileURL } = require('node:url');
const isWindows = process.platform === 'win32';

const bench = common.createBenchmark(main, {
input: isWindows ? [
'file:///c/',
] : [
'file:///dev/null',
'file:///dev/null?key=param&bool',
'file:///dev/null?key=param&bool#hash',
],
method: isWindows ? [
'fileURLToPath',
] : [
'fileURLToPath',
'pathToFileURL',
],
n: [5e6],
});

function main({ n, input, method }) {
method = method === 'fileURLOrPath' ? fileURLToPath : pathToFileURL;
bench.start();
for (let i = 0; i < n; i++) {
method(input);
}
bench.end(n);
}
3 changes: 2 additions & 1 deletion lib/internal/freeze_intrinsics.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const {
Int32ArrayPrototype,
Int8Array,
Int8ArrayPrototype,
IteratorPrototype,
Map,
MapPrototype,
Number,
Expand Down Expand Up @@ -211,7 +212,7 @@ module.exports = function() {

// 27 Control Abstraction Objects
// 27.1 Iteration
ObjectGetPrototypeOf(ArrayIteratorPrototype), // 27.1.2 IteratorPrototype
IteratorPrototype, // 27.1.2 IteratorPrototype
// 27.1.3 AsyncIteratorPrototype
ObjectGetPrototypeOf(ObjectGetPrototypeOf(ObjectGetPrototypeOf(
(async function*() {})()
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/per_context/primordials.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ function copyPrototype(src, dest, prefix) {
copyPrototype(original.prototype, primordials, `${name}Prototype`);
});

primordials.IteratorPrototype = Reflect.getPrototypeOf(primordials.ArrayIteratorPrototype);

/* eslint-enable node-core/prefer-primordials */

const {
Expand Down
48 changes: 24 additions & 24 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const {
ArrayPrototypeSlice,
FunctionPrototypeBind,
Int8Array,
IteratorPrototype,
Number,
ObjectCreate,
ObjectDefineProperties,
Expand All @@ -19,11 +20,13 @@ const {
ReflectApply,
ReflectGetOwnPropertyDescriptor,
ReflectOwnKeys,
RegExpPrototypeSymbolReplace,
String,
StringPrototypeCharAt,
StringPrototypeCharCodeAt,
StringPrototypeCodePointAt,
StringPrototypeIncludes,
StringPrototypeReplace,
StringPrototypeReplaceAll,
StringPrototypeIndexOf,
StringPrototypeSlice,
StringPrototypeSplit,
StringPrototypeStartsWith,
Expand Down Expand Up @@ -113,6 +116,8 @@ const {
revokeDataObject,
} = internalBinding('blob');

const FORWARD_SLASH = /\//g;

const context = Symbol('context');
const cannotBeBase = Symbol('cannot-be-base');
const cannotHaveUsernamePasswordPort =
Expand All @@ -139,11 +144,6 @@ function lazyCryptoRandom() {
return cryptoRandom;
}

// https://tc39.github.io/ecma262/#sec-%iteratorprototype%-object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: should we add this url to primordials declaration too?

const IteratorPrototype = ObjectGetPrototypeOf(
ObjectGetPrototypeOf([][SymbolIterator]())
);

// Refs: https://html.spec.whatwg.org/multipage/browsers.html#concept-origin-opaque
const kOpaqueOrigin = 'null';

Expand Down Expand Up @@ -1377,7 +1377,7 @@ defineIDLClass(URLSearchParamsIteratorPrototype, 'URLSearchParams Iterator', {
},
[]
);
const breakLn = inspect(output, innerOpts).includes('\n');
const breakLn = StringPrototypeIncludes(inspect(output, innerOpts), '\n');
const outputStrs = ArrayPrototypeMap(output, (p) => inspect(p, innerOpts));
let outputStr;
if (breakLn) {
Expand Down Expand Up @@ -1435,7 +1435,7 @@ function getPathFromURLWin32(url) {
let pathname = url.pathname;
for (let n = 0; n < pathname.length; n++) {
if (pathname[n] === '%') {
const third = pathname.codePointAt(n + 2) | 0x20;
const third = StringPrototypeCodePointAt(pathname, n + 2) | 0x20;
if ((pathname[n + 1] === '2' && third === 102) || // 2f 2F /
(pathname[n + 1] === '5' && third === 99)) { // 5c 5C \
throw new ERR_INVALID_FILE_URL_PATH(
Expand All @@ -1444,7 +1444,7 @@ function getPathFromURLWin32(url) {
}
}
}
pathname = StringPrototypeReplaceAll(pathname, '/', '\\');
pathname = RegExpPrototypeSymbolReplace(FORWARD_SLASH, pathname, '\\');
pathname = decodeURIComponent(pathname);
if (hostname !== '') {
// If hostname is set, then we have a UNC path
Expand All @@ -1456,13 +1456,13 @@ function getPathFromURLWin32(url) {
return `\\\\${domainToUnicode(hostname)}${pathname}`;
}
// Otherwise, it's a local path that requires a drive letter
const letter = pathname.codePointAt(1) | 0x20;
const sep = pathname[2];
const letter = StringPrototypeCodePointAt(pathname, 1) | 0x20;
const sep = StringPrototypeCharAt(pathname, 2);
if (letter < CHAR_LOWERCASE_A || letter > CHAR_LOWERCASE_Z || // a..z A..Z
(sep !== ':')) {
throw new ERR_INVALID_FILE_URL_PATH('must be absolute');
}
return pathname.slice(1);
return StringPrototypeSlice(pathname, 1);
}

function getPathFromURLPosix(url) {
Expand All @@ -1472,7 +1472,7 @@ function getPathFromURLPosix(url) {
const pathname = url.pathname;
for (let n = 0; n < pathname.length; n++) {
if (pathname[n] === '%') {
const third = pathname.codePointAt(n + 2) | 0x20;
const third = StringPrototypeCodePointAt(pathname, n + 2) | 0x20;
if (pathname[n + 1] === '2' && third === 102) {
throw new ERR_INVALID_FILE_URL_PATH(
'must not include encoded / characters'
Expand Down Expand Up @@ -1512,42 +1512,42 @@ const tabRegEx = /\t/g;

function encodePathChars(filepath) {
if (StringPrototypeIncludes(filepath, '%'))
filepath = StringPrototypeReplace(filepath, percentRegEx, '%25');
filepath = RegExpPrototypeSymbolReplace(percentRegEx, filepath, '%25');
// In posix, backslash is a valid character in paths:
if (!isWindows && StringPrototypeIncludes(filepath, '\\'))
filepath = StringPrototypeReplace(filepath, backslashRegEx, '%5C');
filepath = RegExpPrototypeSymbolReplace(backslashRegEx, filepath, '%5C');
if (StringPrototypeIncludes(filepath, '\n'))
filepath = StringPrototypeReplace(filepath, newlineRegEx, '%0A');
filepath = RegExpPrototypeSymbolReplace(newlineRegEx, filepath, '%0A');
if (StringPrototypeIncludes(filepath, '\r'))
filepath = StringPrototypeReplace(filepath, carriageReturnRegEx, '%0D');
filepath = RegExpPrototypeSymbolReplace(carriageReturnRegEx, filepath, '%0D');
if (StringPrototypeIncludes(filepath, '\t'))
filepath = StringPrototypeReplace(filepath, tabRegEx, '%09');
filepath = RegExpPrototypeSymbolReplace(tabRegEx, filepath, '%09');
return filepath;
}

function pathToFileURL(filepath) {
const outURL = new URL('file://');
if (isWindows && StringPrototypeStartsWith(filepath, '\\\\')) {
// UNC path format: \\server\share\resource
const paths = StringPrototypeSplit(filepath, '\\');
if (paths.length <= 3) {
const hostnameEndIndex = StringPrototypeIndexOf('\\', 2);
anonrig marked this conversation as resolved.
Show resolved Hide resolved
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
if (hostnameEndIndex === -1) {
throw new ERR_INVALID_ARG_VALUE(
'filepath',
filepath,
'Missing UNC resource path'
);
}
const hostname = paths[2];
if (hostname.length === 0) {
if (hostnameEndIndex === 3) {
throw new ERR_INVALID_ARG_VALUE(
'filepath',
filepath,
'Empty UNC servername'
);
}
const hostname = StringPrototypeSlice(filepath, 2, hostnameEndIndex);
outURL.hostname = domainToASCII(hostname);
outURL.pathname = encodePathChars(
ArrayPrototypeJoin(ArrayPrototypeSlice(paths, 3), '/'));
RegExpPrototypeSymbolReplace(backslashRegEx, StringPrototypeSlice(filepath, hostnameEndIndex), '/'));
} else {
let resolved = path.resolve(filepath);
// path.resolve strips trailing slashes so we must add them back
Expand Down