Skip to content

Commit f5b8e7b

Browse files
aduh95ruyadorno
authored andcommitted
http2: refactor to avoid unsafe array iteration
PR-URL: #36700 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent c2ec15a commit f5b8e7b

File tree

2 files changed

+20
-12
lines changed

2 files changed

+20
-12
lines changed

lib/internal/http2/compat.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const {
1212
ReflectApply,
1313
ReflectGetPrototypeOf,
1414
StringPrototypeIncludes,
15+
SafeArrayIterator,
1516
StringPrototypeToLowerCase,
1617
StringPrototypeTrim,
1718
Symbol,
@@ -148,7 +149,8 @@ function onStreamTrailers(trailers, flags, rawTrailers) {
148149
const request = this[kRequest];
149150
if (request !== undefined) {
150151
ObjectAssign(request[kTrailers], trailers);
151-
ArrayPrototypePush(request[kRawTrailers], ...rawTrailers);
152+
ArrayPrototypePush(request[kRawTrailers],
153+
...new SafeArrayIterator(rawTrailers));
152154
}
153155
}
154156

lib/internal/http2/core.js

+17-11
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const {
77
ArrayIsArray,
88
ArrayPrototypeForEach,
99
ArrayPrototypePush,
10+
ArrayPrototypeUnshift,
1011
FunctionPrototypeBind,
1112
FunctionPrototypeCall,
1213
MathMin,
@@ -20,6 +21,7 @@ const {
2021
ReflectApply,
2122
ReflectGetPrototypeOf,
2223
RegExpPrototypeTest,
24+
SafeArrayIterator,
2325
SafeMap,
2426
SafeSet,
2527
StringPrototypeSlice,
@@ -187,21 +189,22 @@ let debug = require('internal/util/debuglog').debuglog('http2', (fn) => {
187189
// this seems pretty fast, though.
188190
function debugStream(id, sessionType, message, ...args) {
189191
debug('Http2Stream %s [Http2Session %s]: ' + message,
190-
id, sessionName(sessionType), ...args);
192+
id, sessionName(sessionType), ...new SafeArrayIterator(args));
191193
}
192194

193195
function debugStreamObj(stream, message, ...args) {
194196
const session = stream[kSession];
195197
const type = session ? session[kType] : undefined;
196-
debugStream(stream[kID], type, message, ...args);
198+
debugStream(stream[kID], type, message, ...new SafeArrayIterator(args));
197199
}
198200

199201
function debugSession(sessionType, message, ...args) {
200-
debug('Http2Session %s: ' + message, sessionName(sessionType), ...args);
202+
debug('Http2Session %s: ' + message, sessionName(sessionType),
203+
...new SafeArrayIterator(args));
201204
}
202205

203206
function debugSessionObj(session, message, ...args) {
204-
debugSession(session[kType], message, ...args);
207+
debugSession(session[kType], message, ...new SafeArrayIterator(args));
205208
}
206209

207210
const kMaxFrameSize = (2 ** 24) - 1;
@@ -317,7 +320,7 @@ const SESSION_FLAGS_DESTROYED = 0x4;
317320

318321
// Top level to avoid creating a closure
319322
function emit(self, ...args) {
320-
self.emit(...args);
323+
ReflectApply(self.emit, self, args);
321324
}
322325

323326
// Called when a new block of headers has been received for a given
@@ -1020,7 +1023,7 @@ function setupHandle(socket, type, options) {
10201023

10211024
if (type === NGHTTP2_SESSION_SERVER &&
10221025
ArrayIsArray(options.origins)) {
1023-
this.origin(...options.origins);
1026+
ReflectApply(this.origin, this, options.origins);
10241027
}
10251028

10261029
process.nextTick(emit, this, 'connect', this, socket);
@@ -1495,7 +1498,7 @@ class Http2Session extends EventEmitter {
14951498
[EventEmitter.captureRejectionSymbol](err, event, ...args) {
14961499
switch (event) {
14971500
case 'stream':
1498-
const [stream] = args;
1501+
const stream = args[0];
14991502
stream.destroy(err);
15001503
break;
15011504
default:
@@ -1663,7 +1666,9 @@ class ClientHttp2Session extends Http2Session {
16631666
this[kUpdateTimer]();
16641667

16651668
if (headers !== null && headers !== undefined) {
1666-
for (const header of ObjectKeys(headers)) {
1669+
const keys = ObjectKeys(headers);
1670+
for (let i = 0; i < keys.length; i++) {
1671+
const header = keys[i];
16671672
if (header[0] === ':') {
16681673
assertValidPseudoHeader(header);
16691674
} else if (header && !checkIsHttpToken(header))
@@ -3095,7 +3100,7 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function(
30953100
case 'stream':
30963101
// TODO(mcollina): we might want to match this with what we do on
30973102
// the compat side.
3098-
const [stream] = args;
3103+
const { 0: stream } = args;
30993104
if (stream.sentHeaders) {
31003105
stream.destroy(err);
31013106
} else {
@@ -3104,7 +3109,7 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function(
31043109
}
31053110
break;
31063111
case 'request':
3107-
const [, res] = args;
3112+
const { 1: res } = args;
31083113
if (!res.headersSent && !res.finished) {
31093114
// Don't leak headers.
31103115
for (const name of res.getHeaderNames()) {
@@ -3117,8 +3122,9 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function(
31173122
}
31183123
break;
31193124
default:
3125+
ArrayPrototypeUnshift(args, err, event);
31203126
ReflectApply(net.Server.prototype[EventEmitter.captureRejectionSymbol],
3121-
this, [err, event, ...args]);
3127+
this, args);
31223128
}
31233129
};
31243130

0 commit comments

Comments
 (0)