diff --git a/doc/contributing/primordials.md b/doc/contributing/primordials.md index 4ca383c41cb..2dfe183b89a 100644 --- a/doc/contributing/primordials.md +++ b/doc/contributing/primordials.md @@ -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". diff --git a/lib/eslint.config_partial.mjs b/lib/eslint.config_partial.mjs index 6e9f5df25ba..b0abd1b026e 100644 --- a/lib/eslint.config_partial.mjs +++ b/lib/eslint.config_partial.mjs @@ -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', + }, + ], + }, + }, ]; diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 631d79ddfbf..24370094866 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -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)) { diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 9d3f9b84b7d..4dc4eaf547e 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -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; } diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 46e0218ac64..77267a52fdc 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -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