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

lib: use safe methods from primordials #27096

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

'use strict';

const { ObjectPrototype } = primordials;

const assert = require('internal/assert');
const Stream = require('stream');
const internalUtil = require('internal/util');
Expand Down Expand Up @@ -53,8 +55,6 @@ const { CRLF, debug } = common;

const kIsCorked = Symbol('isCorked');

const hasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty);

const RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
const RE_TE_CHUNKED = common.chunkExpression;

Expand Down Expand Up @@ -310,7 +310,7 @@ function _storeHeader(firstLine, headers) {
}
} else {
for (const key in headers) {
if (hasOwnProperty(headers, key)) {
if (ObjectPrototype.hasOwnProperty(headers, key)) {
processHeader(this, state, key, headers[key], true);
}
}
Expand Down
6 changes: 2 additions & 4 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const { Reflect } = primordials;
const { FunctionPrototype, Reflect } = primordials;

const {
ERR_ASYNC_TYPE,
Expand Down Expand Up @@ -77,8 +77,6 @@ const { kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve,
kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId,
kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants;

const FunctionBind = Function.call.bind(Function.prototype.bind);

// Used in AsyncHook and AsyncResource.
const async_id_symbol = Symbol('asyncId');
const trigger_async_id_symbol = Symbol('triggerAsyncId');
Expand Down Expand Up @@ -181,7 +179,7 @@ function emitHook(symbol, asyncId) {
}

function emitHookFactory(symbol, name) {
const fn = FunctionBind(emitHook, undefined, symbol);
const fn = FunctionPrototype.bind(emitHook, undefined, symbol);

// Set the name property of the function as it looks good in the stack trace.
Object.defineProperty(fn, 'name', {
Expand Down
7 changes: 3 additions & 4 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ NativeModule.prototype.compileForPublicLoader = function(needToProxify) {
};

const getOwn = (target, property, receiver) => {
return Reflect.apply(ObjectPrototype.hasOwnProperty, target, [property]) ?
return ObjectPrototype.hasOwnProperty(target, property) ?
Reflect.get(target, property, receiver) :
undefined;
};
Expand All @@ -238,8 +238,7 @@ NativeModule.prototype.proxifyExports = function() {

const update = (property, value) => {
if (this.reflect !== undefined &&
Reflect.apply(ObjectPrototype.hasOwnProperty,
this.reflect.exports, [property]))
ObjectPrototype.hasOwnProperty(this.reflect.exports, [property]))
targos marked this conversation as resolved.
Show resolved Hide resolved
this.reflect.exports[property].set(value);
};

Expand All @@ -253,7 +252,7 @@ NativeModule.prototype.proxifyExports = function() {
!Reflect.has(handler, 'get')) {
handler.get = (target, prop, receiver) => {
const value = Reflect.get(target, prop, receiver);
if (Reflect.apply(ObjectPrototype.hasOwnProperty, target, [prop]))
if (ObjectPrototype.hasOwnProperty, target, prop)
targos marked this conversation as resolved.
Show resolved Hide resolved
update(prop, value);
return value;
};
Expand Down
33 changes: 32 additions & 1 deletion lib/internal/bootstrap/primordials.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@
// `primordials.Object` where `primordials` is a lexical variable passed
// by the native module compiler.

const ReflectApply = Reflect.apply;

// This function is borrowed from the function with the same name on V8 Extras'
// `utils` object. V8 implements Reflect.apply very efficiently in conjunction
// with the spread syntax, such that no additional special case is needed for
// function calls w/o arguments.
// Refs: https://github.com/v8/v8/blob/d6ead37d265d7215cf9c5f768f279e21bd170212/src/js/prologue.js#L152-L156
function uncurryThis(func) {
return (thisArg, ...args) => ReflectApply(func, thisArg, args);
}

primordials.uncurryThis = uncurryThis;

function copyProps(src, dest) {
for (const key of Reflect.ownKeys(src)) {
if (!Reflect.getOwnPropertyDescriptor(dest, key)) {
Expand All @@ -23,6 +36,18 @@ function copyProps(src, dest) {
}
}

function copyPrototype(src, dest) {
for (const key of Reflect.ownKeys(src)) {
if (!Reflect.getOwnPropertyDescriptor(dest, key)) {
const desc = Reflect.getOwnPropertyDescriptor(src, key);
if (typeof desc.value === 'function') {
desc.value = uncurryThis(desc.value);
Copy link
Member

Choose a reason for hiding this comment

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

I was talking about this, as this affect all prototype methods, not just the Function ones.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is already what was done manually in all the files I changed (either with call.bind or uncurryThis). This is centralizing the uncurrying to this file. If we want to use the methods safely, we have to call them without going through the prototype.

}
Reflect.defineProperty(dest, key, desc);
}
}
}

function makeSafe(unsafe, safe) {
copyProps(unsafe.prototype, safe.prototype);
copyProps(unsafe, safe);
Expand Down Expand Up @@ -64,17 +89,23 @@ primordials.SafePromise = makeSafe(
// Create copies of intrinsic objects
[
'Array',
'BigInt',
'Boolean',
'Date',
'Error',
'Function',
'Map',
'Number',
'Object',
'RegExp',
'Set',
'String',
'Symbol',
].forEach((name) => {
const target = primordials[name] = Object.create(null);
copyProps(global[name], target);
const proto = primordials[name + 'Prototype'] = Object.create(null);
copyProps(global[name].prototype, proto);
copyPrototype(global[name].prototype, proto);
});

Object.setPrototypeOf(primordials, null);
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/cli_table.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
'use strict';

const { Math } = primordials;
const { Math, ObjectPrototype } = primordials;

const { Buffer } = require('buffer');
const { removeColors } = require('internal/util');
const HasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty);

// The use of Unicode characters below is the only non-comment use of non-ASCII
// Unicode characters in Node.js built-in modules. If they are ever removed or
Expand Down Expand Up @@ -61,7 +60,8 @@ const table = (head, columns) => {
for (var j = 0; j < longestColumn; j++) {
if (rows[j] === undefined)
rows[j] = [];
const value = rows[j][i] = HasOwnProperty(column, j) ? column[j] : '';
const value = rows[j][i] =
ObjectPrototype.hasOwnProperty(column, j) ? column[j] : '';
const width = columnWidths[i] || 0;
const counted = countSymbols(value);
columnWidths[i] = Math.max(width, counted);
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/console/constructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// The Console constructor is not actually used to construct the global
// console. It's exported for backwards compatibility.

const { Reflect } = primordials;
const { ObjectPrototype, Reflect } = primordials;

const { trace } = internalBinding('trace_events');
const {
Expand Down Expand Up @@ -36,7 +36,6 @@ const {
keys: ObjectKeys,
values: ObjectValues,
} = Object;
const hasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty);

const {
isArray: ArrayIsArray,
Expand Down Expand Up @@ -493,7 +492,8 @@ const consoleMethods = {
for (const key of keys) {
if (map[key] === undefined)
map[key] = [];
if ((primitive && properties) || !hasOwnProperty(item, key))
if ((primitive && properties) ||
!ObjectPrototype.hasOwnProperty(item, key))
map[key][i] = '';
else
map[key][i] = _inspect(item[key]);
Expand Down
37 changes: 14 additions & 23 deletions lib/internal/error-serdes.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,17 @@

const Buffer = require('buffer').Buffer;
const {
SafeSet,
ArrayPrototype,
FunctionPrototype,
Object,
ObjectPrototype,
FunctionPrototype,
ArrayPrototype
SafeSet,
} = primordials;

const kSerializedError = 0;
const kSerializedObject = 1;
const kInspectedError = 2;

const GetPrototypeOf = Object.getPrototypeOf;
const GetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
const GetOwnPropertyNames = Object.getOwnPropertyNames;
const DefineProperty = Object.defineProperty;
const Assign = Object.assign;
const ObjectPrototypeToString =
FunctionPrototype.call.bind(ObjectPrototype.toString);
const ForEach = FunctionPrototype.call.bind(ArrayPrototype.forEach);
const Call = FunctionPrototype.call.bind(FunctionPrototype.call);

const errors = {
Error, TypeError, RangeError, URIError, SyntaxError, ReferenceError, EvalError
};
Expand All @@ -32,17 +22,18 @@ function TryGetAllProperties(object, target = object) {
const all = Object.create(null);
if (object === null)
return all;
Assign(all, TryGetAllProperties(GetPrototypeOf(object), target));
const keys = GetOwnPropertyNames(object);
ForEach(keys, (key) => {
Object.assign(all,
TryGetAllProperties(Object.getPrototypeOf(object), target));
const keys = Object.getOwnPropertyNames(object);
ArrayPrototype.forEach(keys, (key) => {
let descriptor;
try {
descriptor = GetOwnPropertyDescriptor(object, key);
descriptor = Object.getOwnPropertyDescriptor(object, key);
} catch { return; }
const getter = descriptor.get;
if (getter && key !== '__proto__') {
try {
descriptor.value = Call(getter, target);
descriptor.value = FunctionPrototype.call(getter, target);
} catch {}
}
if ('value' in descriptor && typeof descriptor.value !== 'function') {
Expand All @@ -59,10 +50,10 @@ function GetConstructors(object) {

for (var current = object;
current !== null;
current = GetPrototypeOf(current)) {
const desc = GetOwnPropertyDescriptor(current, 'constructor');
current = Object.getPrototypeOf(current)) {
const desc = Object.getOwnPropertyDescriptor(current, 'constructor');
if (desc && desc.value) {
DefineProperty(constructors, constructors.length, {
Object.defineProperty(constructors, constructors.length, {
value: desc.value, enumerable: true
});
}
Expand All @@ -72,7 +63,7 @@ function GetConstructors(object) {
}

function GetName(object) {
const desc = GetOwnPropertyDescriptor(object, 'name');
const desc = Object.getOwnPropertyDescriptor(object, 'name');
return desc && desc.value;
}

Expand All @@ -89,7 +80,7 @@ function serializeError(error) {
if (!serialize) serialize = require('v8').serialize;
try {
if (typeof error === 'object' &&
ObjectPrototypeToString(error) === '[object Error]') {
ObjectPrototype.toString(error) === '[object Error]') {
const constructors = GetConstructors(error);
for (var i = 0; i < constructors.length; i++) {
const name = GetName(constructors[i]);
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/modules/esm/create_dynamic_module.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
'use strict';

const { ArrayPrototype } = primordials;

const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
const debug = require('util').debuglog('esm');
const ArrayJoin = Function.call.bind(Array.prototype.join);
const ArrayMap = Function.call.bind(Array.prototype.map);

const createDynamicModule = (exports, url = '', evaluate) => {
debug('creating ESM facade for %s with exports: %j', url, exports);
const names = ArrayMap(exports, (name) => `${name}`);
const names = ArrayPrototype.map(exports, (name) => `${name}`);

const source = `
${ArrayJoin(ArrayMap(names, (name) =>
${ArrayPrototype.join(ArrayPrototype.map(names, (name) =>
`let $${name};
export { $${name} as ${name} };
import.meta.exports.${name} = {
Expand Down
12 changes: 7 additions & 5 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const { FunctionPrototype } = primordials;

const {
ERR_INVALID_RETURN_PROPERTY,
ERR_INVALID_RETURN_PROPERTY_VALUE,
Expand All @@ -18,8 +20,6 @@ const createDynamicModule = require(
const { translators } = require('internal/modules/esm/translators');
const { ModuleWrap } = internalBinding('module_wrap');

const FunctionBind = Function.call.bind(Function.prototype.bind);

const debug = require('internal/util/debuglog').debuglog('esm');

const {
Expand Down Expand Up @@ -132,9 +132,11 @@ class Loader {
hook({ resolve, dynamicInstantiate }) {
// Use .bind() to avoid giving access to the Loader instance when called.
if (resolve !== undefined)
this._resolve = FunctionBind(resolve, null);
if (dynamicInstantiate !== undefined)
this._dynamicInstantiate = FunctionBind(dynamicInstantiate, null);
this._resolve = FunctionPrototype.bind(resolve, null);
if (dynamicInstantiate !== undefined) {
this._dynamicInstantiate =
FunctionPrototype.bind(dynamicInstantiate, null);
}
}

async getModuleJob(specifier, parentURL) {
Expand Down
16 changes: 9 additions & 7 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
'use strict';

const {
SafeMap,
StringPrototype,
JSON
} = primordials;

const { NativeModule } = require('internal/bootstrap/loaders');
const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
const {
Expand All @@ -11,10 +17,6 @@ const internalURLModule = require('internal/url');
const createDynamicModule = require(
'internal/modules/esm/create_dynamic_module');
const fs = require('fs');
const {
SafeMap,
JSON
} = primordials;
const { fileURLToPath, URL } = require('url');
const { debuglog } = require('internal/util/debuglog');
const { promisify } = require('internal/util');
Expand All @@ -23,7 +25,6 @@ const {
ERR_UNKNOWN_BUILTIN_MODULE
} = require('internal/errors').codes;
const readFileAsync = promisify(fs.readFile);
const StringReplace = Function.call.bind(String.prototype.replace);
const JsonParse = JSON.parse;

const debug = debuglog('esm');
Expand Down Expand Up @@ -67,7 +68,8 @@ translators.set('commonjs', async function commonjsStrategy(url, isMain) {
return cached;
}
const module = CJSModule._cache[
isWindows ? StringReplace(pathname, winSepRegEx, '\\') : pathname];
isWindows ? StringPrototype.replace(pathname, winSepRegEx, '\\') : pathname
];
if (module && module.loaded) {
const exports = module.exports;
return createDynamicModule(['default'], url, (reflect) => {
Expand Down Expand Up @@ -110,7 +112,7 @@ translators.set('json', async function jsonStrategy(url) {
debug(`Loading JSONModule ${url}`);
const pathname = fileURLToPath(url);
const modulePath = isWindows ?
StringReplace(pathname, winSepRegEx, '\\') : pathname;
StringPrototype.replace(pathname, winSepRegEx, '\\') : pathname;
let module = CJSModule._cache[modulePath];
if (module && module.loaded) {
const exports = module.exports;
Expand Down
Loading