From f5b8e7b06882000ce9df758f5f3cb57af5ce01db Mon Sep 17 00:00:00 2001 From: Antoine du Hamel 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 Reviewed-By: Rich Trott --- 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); } };