http2: remove prototype primordials

Co-authored-by: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: #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>

Author: 2 people

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".

lib/internal/http2/compat.js

@@ -2,19 +2,13 @@
 
 const {
   ArrayIsArray,
-  ArrayPrototypePush,
   Boolean,
-  FunctionPrototypeBind,
   ObjectAssign,
+  ObjectHasOwn,
   ObjectKeys,
-  ObjectPrototypeHasOwnProperty,
   Proxy,
   ReflectApply,
   ReflectGetPrototypeOf,
-  StringPrototypeIncludes,
-  SafeArrayIterator,
-  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,16 +602,16 @@ 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) {
     validateString(name, 'name');
     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)) {

lib/internal/http2/core.js

@@ -3,34 +3,24 @@
 const {
   ArrayFrom,
   ArrayIsArray,
-  ArrayPrototypeForEach,
-  ArrayPrototypePush,
-  ArrayPrototypeUnshift,
-  FunctionPrototypeBind,
-  FunctionPrototypeCall,
   MathMin,
   Number,
   ObjectAssign,
-  ObjectKeys,
   ObjectDefineProperty,
   ObjectEntries,
-  ObjectPrototypeHasOwnProperty,
+  ObjectHasOwn,
+  ObjectKeys,
   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;
   }

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