From 0fd9bbbb2458c310d45e2d2b6e6c01c539d31f60 Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
Date: Tue, 29 Dec 2020 20:33:03 +0100
Subject: [PATCH] http2: refactor to avoid unsafe array iteration

PR-URL: https://github.com/nodejs/node/pull/36700
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 lib/internal/http2/compat.js |  4 +++-
 lib/internal/http2/core.js   | 28 +++++++++++++++++-----------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
index d116cb9d5ee925..44961fb87dd6a0 100644
--- a/lib/internal/http2/compat.js
+++ b/lib/internal/http2/compat.js
@@ -12,6 +12,7 @@ const {
   ReflectApply,
   ReflectGetPrototypeOf,
   StringPrototypeIncludes,
+  SafeArrayIterator,
   StringPrototypeToLowerCase,
   StringPrototypeTrim,
   Symbol,
@@ -148,7 +149,8 @@ function onStreamTrailers(trailers, flags, rawTrailers) {
   const request = this[kRequest];
   if (request !== undefined) {
     ObjectAssign(request[kTrailers], trailers);
-    ArrayPrototypePush(request[kRawTrailers], ...rawTrailers);
+    ArrayPrototypePush(request[kRawTrailers],
+                       ...new SafeArrayIterator(rawTrailers));
   }
 }
 
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 22ed8086dc9316..a4eab21c135b35 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -7,6 +7,7 @@ const {
   ArrayIsArray,
   ArrayPrototypeForEach,
   ArrayPrototypePush,
+  ArrayPrototypeUnshift,
   FunctionPrototypeBind,
   FunctionPrototypeCall,
   MathMin,
@@ -20,6 +21,7 @@ const {
   ReflectApply,
   ReflectGetPrototypeOf,
   RegExpPrototypeTest,
+  SafeArrayIterator,
   SafeMap,
   SafeSet,
   StringPrototypeSlice,
@@ -187,21 +189,22 @@ let debug = require('internal/util/debuglog').debuglog('http2', (fn) => {
 // this seems pretty fast, though.
 function debugStream(id, sessionType, message, ...args) {
   debug('Http2Stream %s [Http2Session %s]: ' + message,
-        id, sessionName(sessionType), ...args);
+        id, sessionName(sessionType), ...new SafeArrayIterator(args));
 }
 
 function debugStreamObj(stream, message, ...args) {
   const session = stream[kSession];
   const type = session ? session[kType] : undefined;
-  debugStream(stream[kID], type, message, ...args);
+  debugStream(stream[kID], type, message, ...new SafeArrayIterator(args));
 }
 
 function debugSession(sessionType, message, ...args) {
-  debug('Http2Session %s: ' + message, sessionName(sessionType), ...args);
+  debug('Http2Session %s: ' + message, sessionName(sessionType),
+        ...new SafeArrayIterator(args));
 }
 
 function debugSessionObj(session, message, ...args) {
-  debugSession(session[kType], message, ...args);
+  debugSession(session[kType], message, ...new SafeArrayIterator(args));
 }
 
 const kMaxFrameSize = (2 ** 24) - 1;
@@ -317,7 +320,7 @@ const SESSION_FLAGS_DESTROYED = 0x4;
 
 // Top level to avoid creating a closure
 function emit(self, ...args) {
-  self.emit(...args);
+  ReflectApply(self.emit, self, args);
 }
 
 // Called when a new block of headers has been received for a given
@@ -1020,7 +1023,7 @@ function setupHandle(socket, type, options) {
 
   if (type === NGHTTP2_SESSION_SERVER &&
       ArrayIsArray(options.origins)) {
-    this.origin(...options.origins);
+    ReflectApply(this.origin, this, options.origins);
   }
 
   process.nextTick(emit, this, 'connect', this, socket);
@@ -1495,7 +1498,7 @@ class Http2Session extends EventEmitter {
   [EventEmitter.captureRejectionSymbol](err, event, ...args) {
     switch (event) {
       case 'stream':
-        const [stream] = args;
+        const stream = args[0];
         stream.destroy(err);
         break;
       default:
@@ -1663,7 +1666,9 @@ class ClientHttp2Session extends Http2Session {
     this[kUpdateTimer]();
 
     if (headers !== null && headers !== undefined) {
-      for (const header of ObjectKeys(headers)) {
+      const keys = ObjectKeys(headers);
+      for (let i = 0; i < keys.length; i++) {
+        const header = keys[i];
         if (header[0] === ':') {
           assertValidPseudoHeader(header);
         } else if (header && !checkIsHttpToken(header))
@@ -3095,7 +3100,7 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function(
     case 'stream':
       // TODO(mcollina): we might want to match this with what we do on
       // the compat side.
-      const [stream] = args;
+      const { 0: stream } = args;
       if (stream.sentHeaders) {
         stream.destroy(err);
       } else {
@@ -3104,7 +3109,7 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function(
       }
       break;
     case 'request':
-      const [, res] = args;
+      const { 1: res } = args;
       if (!res.headersSent && !res.finished) {
         // Don't leak headers.
         for (const name of res.getHeaderNames()) {
@@ -3117,8 +3122,9 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function(
       }
       break;
     default:
+      ArrayPrototypeUnshift(args, err, event);
       ReflectApply(net.Server.prototype[EventEmitter.captureRejectionSymbol],
-                   this, [err, event, ...args]);
+                   this, args);
   }
 };