http2: remove prototype primordials

Co-authored-by: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/53696
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
This commit is contained in:
Antoine du Hamel 2024-07-05 08:20:12 +02:00 committed by GitHub
parent 5a775b3b9e
commit 88beb76e5c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 105 additions and 125 deletions

View File

@ -4,8 +4,13 @@ The file `lib/internal/per_context/primordials.js` subclasses and stores the JS
built-ins that come from the VM so that Node.js built-in modules do not need to
later look these up from the global proxy, which can be mutated by users.
Usage of primordials should be preferred for any new code, but replacing current
code with primordials should be
For some area of the codebase, performance and code readability are deemed more
important than reliability against prototype pollution:
* `node:http2`
Usage of primordials should be preferred for new code in other areas, but
replacing current code with primordials should be
[done with care](#primordials-with-known-performance-issues). It is highly
recommended to ping the relevant team when reviewing a pull request that touches
one of the subsystems they "own".

View File

@ -5,6 +5,32 @@ import {
noRestrictedSyntaxCommonLib,
} from '../tools/eslint/eslint.config_utils.mjs';
const noRestrictedSyntax = [
'error',
...noRestrictedSyntaxCommonAll,
...noRestrictedSyntaxCommonLib,
{
selector: "CallExpression[callee.object.name='assert']:not([callee.property.name='ok']):not([callee.property.name='fail']):not([callee.property.name='ifError'])",
message: 'Please only use simple assertions in ./lib',
},
{
selector: 'NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError|NodeAggregateError)$/])',
message: 'Use an error exported by the internal/errors module.',
},
{
selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']",
message: "Please use `require('internal/errors').hideStackFrames()` instead.",
},
{
selector: "AssignmentExpression:matches([left.object.name='Error']):matches([left.name='prepareStackTrace'], [left.property.name='prepareStackTrace'])",
message: "Use 'overrideStackTrace' from 'lib/internal/errors.js' instead of 'Error.prepareStackTrace'.",
},
{
selector: "ThrowStatement > NewExpression[callee.name=/^ERR_[A-Z_]+$/] > ObjectExpression:first-child:not(:has([key.name='message']):has([key.name='code']):has([key.name='syscall']))",
message: 'The context passed into SystemError constructor must have .code, .syscall and .message.',
},
];
export default [
{
files: ['lib/**/*.js'],
@ -22,31 +48,7 @@ export default [
rules: {
'prefer-object-spread': 'error',
'no-buffer-constructor': 'error',
'no-restricted-syntax': [
'error',
...noRestrictedSyntaxCommonAll,
...noRestrictedSyntaxCommonLib,
{
selector: "CallExpression[callee.object.name='assert']:not([callee.property.name='ok']):not([callee.property.name='fail']):not([callee.property.name='ifError'])",
message: 'Please only use simple assertions in ./lib',
},
{
selector: 'NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError|NodeAggregateError)$/])',
message: 'Use an error exported by the internal/errors module.',
},
{
selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']",
message: "Please use `require('internal/errors').hideStackFrames()` instead.",
},
{
selector: "AssignmentExpression:matches([left.object.name='Error']):matches([left.name='prepareStackTrace'], [left.property.name='prepareStackTrace'])",
message: "Use 'overrideStackTrace' from 'lib/internal/errors.js' instead of 'Error.prepareStackTrace'.",
},
{
selector: "ThrowStatement > NewExpression[callee.name=/^ERR_[A-Z_]+$/] > ObjectExpression:first-child:not(:has([key.name='message']):has([key.name='code']):has([key.name='syscall']))",
message: 'The context passed into SystemError constructor must have .code, .syscall and .message.',
},
],
'no-restricted-syntax': noRestrictedSyntax,
'no-restricted-globals': [
'error',
{
@ -487,4 +489,19 @@ export default [
'node-core/set-proto-to-null-in-object': 'error',
},
},
{
files: [
'lib/internal/http2/*.js',
'lib/http2.js',
],
rules: {
'no-restricted-syntax': [
...noRestrictedSyntax,
{
selector: 'VariableDeclarator:has(.init[name="primordials"]) Identifier[name=/Prototype/]:not([name=/^(Object|Reflect)(Get|Set)PrototypeOf$/])',
message: 'We do not use prototype primordials in this file',
},
],
},
},
];

View File

@ -2,19 +2,13 @@
const {
ArrayIsArray,
ArrayPrototypePush,
Boolean,
FunctionPrototypeBind,
ObjectAssign,
ObjectHasOwn,
ObjectKeys,
ObjectPrototypeHasOwnProperty,
Proxy,
ReflectApply,
ReflectGetPrototypeOf,
SafeArrayIterator,
StringPrototypeIncludes,
StringPrototypeToLowerCase,
StringPrototypeTrim,
Symbol,
} = primordials;
@ -89,7 +83,7 @@ let statusConnectionHeaderWarned = false;
const assertValidHeader = hideStackFrames((name, value) => {
if (name === '' ||
typeof name !== 'string' ||
StringPrototypeIncludes(name, ' ')) {
name.includes(' ')) {
throw new ERR_INVALID_HTTP_TOKEN.HideStackFramesError('Header name', name);
}
if (isPseudoHeader(name)) {
@ -153,8 +147,7 @@ function onStreamTrailers(trailers, flags, rawTrailers) {
const request = this[kRequest];
if (request !== undefined) {
ObjectAssign(request[kTrailers], trailers);
ArrayPrototypePush(request[kRawTrailers],
...new SafeArrayIterator(rawTrailers));
request[kRawTrailers].push(...rawTrailers);
}
}
@ -216,7 +209,7 @@ const proxySocketHandler = {
case 'end':
case 'emit':
case 'destroy':
return FunctionPrototypeBind(stream[prop], stream);
return stream[prop].bind(stream);
case 'writable':
case 'destroyed':
return stream[prop];
@ -229,8 +222,8 @@ const proxySocketHandler = {
case 'setTimeout': {
const session = stream.session;
if (session !== undefined)
return FunctionPrototypeBind(session.setTimeout, session);
return FunctionPrototypeBind(stream.setTimeout, stream);
return session.setTimeout.bind(session);
return stream.setTimeout.bind(stream);
}
case 'write':
case 'read':
@ -242,7 +235,7 @@ const proxySocketHandler = {
stream.session[kSocket] : stream;
const value = ref[prop];
return typeof value === 'function' ?
FunctionPrototypeBind(value, ref) :
value.bind(ref) :
value;
}
}
@ -417,7 +410,7 @@ class Http2ServerRequest extends Readable {
set method(method) {
validateString(method, 'method');
if (StringPrototypeTrim(method) === '')
if (method.trim() === '')
throw new ERR_INVALID_ARG_VALUE('method', method);
this[kHeaders][HTTP2_HEADER_METHOD] = method;
@ -578,7 +571,7 @@ class Http2ServerResponse extends Stream {
setTrailer(name, value) {
validateString(name, 'name');
name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
name = name.trim().toLowerCase();
assertValidHeader(name, value);
this[kTrailers][name] = value;
}
@ -594,7 +587,7 @@ class Http2ServerResponse extends Stream {
getHeader(name) {
validateString(name, 'name');
name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
name = name.trim().toLowerCase();
return this[kHeaders][name];
}
@ -609,8 +602,8 @@ class Http2ServerResponse extends Stream {
hasHeader(name) {
validateString(name, 'name');
name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
return ObjectPrototypeHasOwnProperty(this[kHeaders], name);
name = name.trim().toLowerCase();
return ObjectHasOwn(this[kHeaders], name);
}
removeHeader(name) {
@ -618,7 +611,7 @@ class Http2ServerResponse extends Stream {
if (this[kStream].headersSent)
throw new ERR_HTTP2_HEADERS_SENT();
name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
name = name.trim().toLowerCase();
if (name === 'date') {
this[kState].sendDate = false;
@ -638,7 +631,7 @@ class Http2ServerResponse extends Stream {
}
[kSetHeader](name, value) {
name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
name = name.trim().toLowerCase();
assertValidHeader(name, value);
if (!isConnectionHeaderAllowed(name, value)) {
@ -662,7 +655,7 @@ class Http2ServerResponse extends Stream {
}
[kAppendHeader](name, value) {
name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
name = name.trim().toLowerCase();
assertValidHeader(name, value);
if (!isConnectionHeaderAllowed(name, value)) {

View File

@ -3,34 +3,24 @@
const {
ArrayFrom,
ArrayIsArray,
ArrayPrototypeForEach,
ArrayPrototypePush,
ArrayPrototypeUnshift,
FunctionPrototypeBind,
FunctionPrototypeCall,
MathMin,
Number,
ObjectAssign,
ObjectDefineProperty,
ObjectEntries,
ObjectHasOwn,
ObjectKeys,
ObjectPrototypeHasOwnProperty,
Promise,
PromisePrototypeThen,
Proxy,
ReflectApply,
ReflectGet,
ReflectGetPrototypeOf,
ReflectSet,
RegExpPrototypeExec,
SafeArrayIterator,
SafeMap,
SafeSet,
StringPrototypeSlice,
Symbol,
SymbolAsyncDispose,
SymbolDispose,
TypedArrayPrototypeGetLength,
Uint32Array,
Uint8Array,
} = primordials;
@ -205,22 +195,22 @@ function debugStream(id, sessionType, message, ...args) {
return;
}
debug('Http2Stream %s [Http2Session %s]: ' + message,
id, sessionName(sessionType), ...new SafeArrayIterator(args));
id, sessionName(sessionType), ...args);
}
function debugStreamObj(stream, message, ...args) {
const session = stream[kSession];
const type = session ? session[kType] : undefined;
debugStream(stream[kID], type, message, ...new SafeArrayIterator(args));
debugStream(stream[kID], type, message, ...args);
}
function debugSession(sessionType, message, ...args) {
debug('Http2Session %s: ' + message, sessionName(sessionType),
...new SafeArrayIterator(args));
...args);
}
function debugSessionObj(session, message, ...args) {
debugSession(session[kType], message, ...new SafeArrayIterator(args));
debugSession(session[kType], message, ...args);
}
const kMaxFrameSize = (2 ** 24) - 1;
@ -826,8 +816,7 @@ function submitSettings(settings, callback) {
debugSessionObj(this, 'submitting settings');
this[kUpdateTimer]();
updateSettingsBuffer(settings);
if (!this[kHandle].settings(FunctionPrototypeBind(settingsCallback,
this, callback))) {
if (!this[kHandle].settings(settingsCallback.bind(this, callback))) {
this.destroy(new ERR_HTTP2_MAX_PENDING_SETTINGS_ACK());
}
}
@ -869,7 +858,7 @@ const proxySocketHandler = {
case 'setTimeout':
case 'ref':
case 'unref':
return FunctionPrototypeBind(session[prop], session);
return session[prop].bind(session);
case 'destroy':
case 'emit':
case 'end':
@ -887,7 +876,7 @@ const proxySocketHandler = {
throw new ERR_HTTP2_SOCKET_UNBOUND();
const value = socket[prop];
return typeof value === 'function' ?
FunctionPrototypeBind(value, socket) :
value.bind(socket) :
value;
}
}
@ -998,7 +987,7 @@ const validateSettings = hideStackFrames((settings) => {
// Wrap a typed array in a proxy, and allow selectively copying the entries
// that have explicitly been set to another typed array.
function trackAssignmentsTypedArray(typedArray) {
const typedArrayLength = TypedArrayPrototypeGetLength(typedArray);
const typedArrayLength = typedArray.length;
const modifiedEntries = new Uint8Array(typedArrayLength);
function copyAssigned(target) {
@ -1183,8 +1172,7 @@ function closeSession(session, code, error) {
// Destroy the handle if it exists at this point.
if (handle !== undefined) {
handle.ondone = FunctionPrototypeBind(finishSessionClose,
null, session, error);
handle.ondone = finishSessionClose.bind(null, session, error);
handle.destroy(code, socket.destroyed);
} else {
finishSessionClose(session, error);
@ -1276,8 +1264,7 @@ class Http2Session extends EventEmitter {
if (typeof socket.disableRenegotiation === 'function')
socket.disableRenegotiation();
const setupFn = FunctionPrototypeBind(setupHandle, this,
socket, type, options);
const setupFn = setupHandle.bind(this, socket, type, options);
if (socket.connecting || socket.secureConnecting) {
const connectEvent =
socket instanceof tls.TLSSocket ? 'secureConnect' : 'connect';
@ -1512,8 +1499,7 @@ class Http2Session extends EventEmitter {
this[kState].pendingAck++;
const settingsFn = FunctionPrototypeBind(submitSettings, this,
{ ...settings }, callback);
const settingsFn = submitSettings.bind(this, { ...settings }, callback);
if (this.connecting) {
this.once('connect', settingsFn);
return;
@ -1535,9 +1521,7 @@ class Http2Session extends EventEmitter {
validateNumber(code, 'code');
validateNumber(lastStreamID, 'lastStreamID');
const goawayFn = FunctionPrototypeBind(submitGoaway,
this,
code, lastStreamID, opaqueData);
const goawayFn = submitGoaway.bind(this, code, lastStreamID, opaqueData);
if (this.connecting) {
this.once('connect', goawayFn);
return;
@ -1693,7 +1677,7 @@ class ServerHttp2Session extends Http2Session {
}
validateString(alt, 'alt');
if (RegExpPrototypeExec(kQuotedString, alt) === null)
if (!kQuotedString.test(alt))
throw new ERR_INVALID_CHAR('alt');
// Max length permitted for ALTSVC
@ -1786,8 +1770,7 @@ class ClientHttp2Session extends Http2Session {
if (getAuthority(headers) === undefined)
headers[HTTP2_HEADER_AUTHORITY] = this[kAuthority];
if (headers[HTTP2_HEADER_SCHEME] === undefined)
headers[HTTP2_HEADER_SCHEME] = StringPrototypeSlice(this[kProtocol],
0, -1);
headers[HTTP2_HEADER_SCHEME] = this[kProtocol].slice(0, -1);
if (headers[HTTP2_HEADER_PATH] === undefined)
headers[HTTP2_HEADER_PATH] = '/';
} else {
@ -1839,15 +1822,14 @@ class ClientHttp2Session extends Http2Session {
}
}
const onConnect = FunctionPrototypeBind(requestOnConnect,
stream, headersList, options);
const onConnect = requestOnConnect.bind(stream, headersList, options);
if (this.connecting) {
if (this[kPendingRequestCalls] !== null) {
ArrayPrototypePush(this[kPendingRequestCalls], onConnect);
this[kPendingRequestCalls].push(onConnect);
} else {
this[kPendingRequestCalls] = [onConnect];
this.once('connect', () => {
ArrayPrototypeForEach(this[kPendingRequestCalls], (f) => f());
this[kPendingRequestCalls].forEach((f) => f());
this[kPendingRequestCalls] = null;
});
}
@ -1951,7 +1933,7 @@ function closeStream(stream, code, rstStreamStatus = kSubmitRstStream) {
}
if (rstStreamStatus !== kNoRstStream) {
const finishFn = FunctionPrototypeBind(finishCloseStream, stream, code);
const finishFn = finishCloseStream.bind(stream, code);
if (!ending || stream.writableFinished || code !== NGHTTP2_NO_ERROR ||
rstStreamStatus === kForceRstStream)
finishFn();
@ -1961,7 +1943,7 @@ function closeStream(stream, code, rstStreamStatus = kSubmitRstStream) {
}
function finishCloseStream(code) {
const rstStreamFn = FunctionPrototypeBind(submitRstStream, this, code);
const rstStreamFn = submitRstStream.bind(this, code);
// If the handle has not yet been assigned, queue up the request to
// ensure that the RST_STREAM frame is sent after the stream ID has
// been determined.
@ -2145,8 +2127,7 @@ class Http2Stream extends Duplex {
if (this.pending) {
this.once(
'ready',
FunctionPrototypeBind(this[kWriteGeneric],
this, writev, data, encoding, cb),
this[kWriteGeneric].bind(this, writev, data, encoding, cb),
);
return;
}
@ -2238,7 +2219,7 @@ class Http2Stream extends Duplex {
this[kState].didRead = true;
}
if (!this.pending) {
FunctionPrototypeCall(streamOnResume, this);
streamOnResume.call(this);
} else {
this.once('ready', streamOnResume);
}
@ -2252,7 +2233,7 @@ class Http2Stream extends Duplex {
options = { ...options };
setAndValidatePriorityOptions(options);
const priorityFn = FunctionPrototypeBind(submitPriority, this, options);
const priorityFn = submitPriority.bind(this, options);
// If the handle has not yet been assigned, queue up the priority
// frame to be sent as soon as the ready event is emitted.
@ -2455,7 +2436,7 @@ function processHeaders(oldHeaders, options) {
if (oldHeaders !== null && oldHeaders !== undefined) {
// This loop is here for performance reason. Do not change.
for (const key in oldHeaders) {
if (ObjectPrototypeHasOwnProperty(oldHeaders, key)) {
if (ObjectHasOwn(oldHeaders, key)) {
headers[key] = oldHeaders[key];
}
}
@ -2493,8 +2474,7 @@ function processHeaders(oldHeaders, options) {
function onFileUnpipe() {
const stream = this.sink[kOwner];
if (stream.ownsFd)
PromisePrototypeThen(this.source.close(), undefined,
FunctionPrototypeBind(stream.destroy, stream));
this.source.close().catch(stream.destroy.bind(stream));
else
this.source.releaseFD();
}
@ -2677,9 +2657,7 @@ function afterOpen(session, options, headers, streamOptions, err, fd) {
state.fd = fd;
fs.fstat(fd,
FunctionPrototypeBind(doSendFileFD, this,
session, options, fd,
headers, streamOptions));
doSendFileFD.bind(this, session, options, fd, headers, streamOptions));
}
class ServerHttp2Stream extends Http2Stream {
@ -2882,9 +2860,7 @@ class ServerHttp2Stream extends Http2Stream {
if (options.statCheck !== undefined) {
fs.fstat(fd,
FunctionPrototypeBind(doSendFD, this,
session, options, fd,
headers, streamOptions));
doSendFD.bind(this, session, options, fd, headers, streamOptions));
return;
}
@ -2943,8 +2919,7 @@ class ServerHttp2Stream extends Http2Stream {
}
fs.open(path, 'r',
FunctionPrototypeBind(afterOpen, this,
session, options, headers, streamOptions));
afterOpen.bind(this, session, options, headers, streamOptions));
}
// Sends a block of informational headers. In theory, the HTTP/2 spec
@ -2980,7 +2955,7 @@ class ServerHttp2Stream extends Http2Stream {
if (!this[kInfoHeaders])
this[kInfoHeaders] = [headers];
else
ArrayPrototypePush(this[kInfoHeaders], headers);
this[kInfoHeaders].push(headers);
const ret = this[kHandle].info(headersList);
if (ret < 0)
@ -3066,7 +3041,7 @@ function connectionListener(socket) {
if (options.allowHTTP1 === true) {
socket.server[kIncomingMessage] = options.Http1IncomingMessage;
socket.server[kServerResponse] = options.Http1ServerResponse;
return FunctionPrototypeCall(httpConnectionListener, this, socket);
return httpConnectionListener.call(this, socket);
}
// Let event handler deal with the socket
debug('Unknown protocol from %s:%s',
@ -3164,7 +3139,7 @@ function initializeTLSOptions(options, servername) {
options = initializeOptions(options);
options.ALPNProtocols = ['h2'];
if (options.allowHTTP1 === true)
ArrayPrototypePush(options.ALPNProtocols, 'http/1.1');
options.ALPNProtocols.push('http/1.1');
if (servername !== undefined && !options.servername)
options.servername = servername;
return options;
@ -3249,7 +3224,7 @@ class Http2Server extends NETServer {
}
async [SymbolAsyncDispose]() {
return FunctionPrototypeCall(promisify(super.close), this);
return promisify(super.close).call(this);
}
}
@ -3284,7 +3259,7 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function(
break;
}
default:
ArrayPrototypeUnshift(args, err, event);
args.unshift(err, event);
ReflectApply(net.Server.prototype[EventEmitter.captureRejectionSymbol],
this, args);
}
@ -3293,11 +3268,8 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function(
function setupCompat(ev) {
if (ev === 'request') {
this.removeListener('newListener', setupCompat);
this.on('stream', FunctionPrototypeBind(onServerStream,
this,
this[kOptions].Http2ServerRequest,
this[kOptions].Http2ServerResponse),
);
this.on('stream', onServerStream.bind(this, this[kOptions].Http2ServerRequest,
this[kOptions].Http2ServerResponse));
}
}
@ -3344,7 +3316,7 @@ function connect(authority, options, listener) {
host = authority.hostname;
if (host[0] === '[')
host = StringPrototypeSlice(host, 1, -1);
host = host.slice(1, -1);
} else if (authority.host) {
host = authority.host;
}

View File

@ -2,9 +2,6 @@
const {
ArrayIsArray,
ArrayPrototypeIncludes,
ArrayPrototypeMap,
ArrayPrototypePush,
Error,
MathMax,
Number,
@ -13,8 +10,6 @@ const {
SafeSet,
String,
StringFromCharCode,
StringPrototypeIncludes,
StringPrototypeToLowerCase,
Symbol,
} = primordials;
@ -602,15 +597,13 @@ function mapToHeaders(map,
let value;
let isSingleValueHeader;
let err;
const neverIndex =
ArrayPrototypeMap(map[kSensitiveHeaders] || emptyArray,
StringPrototypeToLowerCase);
const neverIndex = (map[kSensitiveHeaders] || emptyArray).map((v) => v.toLowerCase());
for (i = 0; i < keys.length; ++i) {
key = keys[i];
value = map[key];
if (value === undefined || key === '')
continue;
key = StringPrototypeToLowerCase(key);
key = key.toLowerCase();
isSingleValueHeader = kSingleValueHeaders.has(key);
isArray = ArrayIsArray(value);
if (isArray) {
@ -633,7 +626,7 @@ function mapToHeaders(map,
throw new ERR_HTTP2_HEADER_SINGLE_VALUE(key);
singles.add(key);
}
const flags = ArrayPrototypeIncludes(neverIndex, key) ?
const flags = neverIndex.includes(key) ?
kNeverIndexFlag :
kNoHeaderFlags;
if (key[0] === ':') {
@ -644,7 +637,7 @@ function mapToHeaders(map,
count++;
continue;
}
if (StringPrototypeIncludes(key, ' ')) {
if (key.includes(' ')) {
throw new ERR_INVALID_HTTP_TOKEN('Header name', key);
}
if (isIllegalConnectionSpecificHeader(key, value)) {
@ -738,7 +731,7 @@ function toHeaderObject(headers, sensitiveHeaders) {
// fields with the same name. Since it cannot be combined into a
// single field-value, recipients ought to handle "Set-Cookie" as a
// special case while processing header fields."
ArrayPrototypePush(existing, value);
existing.push(value);
break;
default:
// https://tools.ietf.org/html/rfc7230#section-3.2.2