From 514f464a600f258fbd56415340e0f4f8a85afde0 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 17 Nov 2020 00:49:27 +0100 Subject: [PATCH] http2: refactor to use more primordials PR-URL: https://github.com/nodejs/node/pull/36142 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Stephen Belanger Reviewed-By: Ricky Zhou <0x19951125@gmail.com> Reviewed-By: Rich Trott --- lib/internal/http2/compat.js | 31 ++++++---- lib/internal/http2/core.js | 111 +++++++++++++++++++++-------------- lib/internal/http2/util.js | 27 +++++---- 3 files changed, 103 insertions(+), 66 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index e3cc240da1e1c6..a0251ffbafecd2 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -2,12 +2,17 @@ const { ArrayIsArray, + ArrayPrototypePush, Boolean, + FunctionPrototypeBind, ObjectAssign, ObjectCreate, ObjectKeys, ObjectPrototypeHasOwnProperty, + ReflectApply, ReflectGetPrototypeOf, + StringPrototypeToLowerCase, + StringPrototypeTrim, Symbol, } = primordials; @@ -138,7 +143,7 @@ function onStreamTrailers(trailers, flags, rawTrailers) { const request = this[kRequest]; if (request !== undefined) { ObjectAssign(request[kTrailers], trailers); - request[kRawTrailers].push(...rawTrailers); + ArrayPrototypePush(request[kRawTrailers], ...rawTrailers); } } @@ -200,7 +205,7 @@ const proxySocketHandler = { case 'end': case 'emit': case 'destroy': - return stream[prop].bind(stream); + return FunctionPrototypeBind(stream[prop], stream); case 'writable': case 'destroyed': return stream[prop]; @@ -212,8 +217,8 @@ const proxySocketHandler = { case 'setTimeout': const session = stream.session; if (session !== undefined) - return session.setTimeout.bind(session); - return stream.setTimeout.bind(stream); + return FunctionPrototypeBind(session.setTimeout, session); + return FunctionPrototypeBind(stream.setTimeout, stream); case 'write': case 'read': case 'pause': @@ -223,7 +228,9 @@ const proxySocketHandler = { const ref = stream.session !== undefined ? stream.session[kSocket] : stream; const value = ref[prop]; - return typeof value === 'function' ? value.bind(ref) : value; + return typeof value === 'function' ? + FunctionPrototypeBind(value, ref) : + value; } }, getPrototypeOf(stream) { @@ -394,7 +401,7 @@ class Http2ServerRequest extends Readable { set method(method) { validateString(method, 'method'); - if (method.trim() === '') + if (StringPrototypeTrim(method) === '') throw new ERR_INVALID_ARG_VALUE('method', method); this[kHeaders][HTTP2_HEADER_METHOD] = method; @@ -554,7 +561,7 @@ class Http2ServerResponse extends Stream { setTrailer(name, value) { validateString(name, 'name'); - name = name.trim().toLowerCase(); + name = StringPrototypeToLowerCase(StringPrototypeTrim(name)); assertValidHeader(name, value); this[kTrailers][name] = value; } @@ -570,7 +577,7 @@ class Http2ServerResponse extends Stream { getHeader(name) { validateString(name, 'name'); - name = name.trim().toLowerCase(); + name = StringPrototypeToLowerCase(StringPrototypeTrim(name)); return this[kHeaders][name]; } @@ -585,7 +592,7 @@ class Http2ServerResponse extends Stream { hasHeader(name) { validateString(name, 'name'); - name = name.trim().toLowerCase(); + name = StringPrototypeToLowerCase(StringPrototypeTrim(name)); return ObjectPrototypeHasOwnProperty(this[kHeaders], name); } @@ -594,7 +601,7 @@ class Http2ServerResponse extends Stream { if (this[kStream].headersSent) throw new ERR_HTTP2_HEADERS_SENT(); - name = name.trim().toLowerCase(); + name = StringPrototypeToLowerCase(StringPrototypeTrim(name)); if (name === 'date') { this[kState].sendDate = false; @@ -614,7 +621,7 @@ class Http2ServerResponse extends Stream { } [kSetHeader](name, value) { - name = name.trim().toLowerCase(); + name = StringPrototypeToLowerCase(StringPrototypeTrim(name)); assertValidHeader(name, value); if (!isConnectionHeaderAllowed(name, value)) { @@ -755,7 +762,7 @@ class Http2ServerResponse extends Stream { this.writeHead(this[kState].statusCode); if (this[kState].closed || stream.destroyed) - onStreamCloseResponse.call(stream); + ReflectApply(onStreamCloseResponse, stream, []); else stream.end(); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 84d0cd5d948b6b..2ea8eacd0c10a2 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -5,7 +5,9 @@ const { ArrayFrom, ArrayIsArray, - Map, + ArrayPrototypeForEach, + ArrayPrototypePush, + FunctionPrototypeBind, MathMin, ObjectAssign, ObjectCreate, @@ -13,10 +15,15 @@ const { ObjectDefineProperty, ObjectPrototypeHasOwnProperty, Promise, + PromisePrototypeCatch, ReflectApply, ReflectGetPrototypeOf, - Set, + RegExpPrototypeTest, + SafeMap, + SafeSet, + StringPrototypeSlice, Symbol, + TypedArrayPrototypeSet, Uint32Array, Uint8Array, } = primordials; @@ -34,10 +41,10 @@ const EventEmitter = require('events'); const fs = require('fs'); const http = require('http'); const { readUInt16BE, readUInt32BE } = require('internal/buffer'); +const { URL } = require('internal/url'); const net = require('net'); const { Duplex } = require('stream'); const tls = require('tls'); -const { URL } = require('url'); const { setImmediate } = require('timers'); const { @@ -393,7 +400,7 @@ function onSessionHeaders(handle, id, cat, flags, headers, sensitiveHeaders) { function tryClose(fd) { // Try to close the file descriptor. If closing fails, assert because // an error really should not happen at this point. - fs.close(fd, (err) => assert.ifError(err)); + fs.close(fd, assert.ifError); } // Called when the Http2Stream has finished sending data and is ready for @@ -601,7 +608,7 @@ function initOriginSet(session) { let originSet = session[kState].originSet; if (originSet === undefined) { const socket = session[kSocket]; - session[kState].originSet = originSet = new Set(); + session[kState].originSet = originSet = new SafeSet(); if (socket.servername != null) { let originString = `https://${socket.servername}`; if (socket.remotePort != null) @@ -789,7 +796,8 @@ function submitSettings(settings, callback) { debugSessionObj(this, 'submitting settings'); this[kUpdateTimer](); updateSettingsBuffer(settings); - if (!this[kHandle].settings(settingsCallback.bind(this, callback))) { + if (!this[kHandle].settings(FunctionPrototypeBind(settingsCallback, + this, callback))) { this.destroy(new ERR_HTTP2_MAX_PENDING_SETTINGS_ACK()); } } @@ -831,7 +839,7 @@ const proxySocketHandler = { case 'setTimeout': case 'ref': case 'unref': - return session[prop].bind(session); + return FunctionPrototypeBind(session[prop], session); case 'destroy': case 'emit': case 'end': @@ -848,7 +856,9 @@ const proxySocketHandler = { if (socket === undefined) throw new ERR_HTTP2_SOCKET_UNBOUND(); const value = socket[prop]; - return typeof value === 'function' ? value.bind(socket) : value; + return typeof value === 'function' ? + FunctionPrototypeBind(value, socket) : + value; } }, getPrototypeOf(session) { @@ -973,7 +983,7 @@ function setupHandle(socket, type, options) { this[kHandle] = handle; if (this[kNativeFields]) - handle.fields.set(this[kNativeFields]); + TypedArrayPrototypeSet(handle.fields, this[kNativeFields]); else this[kNativeFields] = handle.fields; @@ -1086,7 +1096,8 @@ function closeSession(session, code, error) { // Destroy the handle if it exists at this point. if (handle !== undefined) { - handle.ondone = finishSessionClose.bind(null, session, error); + handle.ondone = FunctionPrototypeBind(finishSessionClose, + null, session, error); handle.destroy(code, socket.destroyed); } else { finishSessionClose(session, error); @@ -1154,8 +1165,8 @@ class Http2Session extends EventEmitter { flags: SESSION_FLAGS_PENDING, goawayCode: null, goawayLastStreamID: null, - streams: new Map(), - pendingStreams: new Set(), + streams: new SafeMap(), + pendingStreams: new SafeSet(), pendingAck: 0, shutdownWritableCalled: false, writeQueueSize: 0, @@ -1178,7 +1189,8 @@ class Http2Session extends EventEmitter { if (typeof socket.disableRenegotiation === 'function') socket.disableRenegotiation(); - const setupFn = setupHandle.bind(this, socket, type, options); + const setupFn = FunctionPrototypeBind(setupHandle, this, + socket, type, options); if (socket.connecting || socket.secureConnecting) { const connectEvent = socket instanceof tls.TLSSocket ? 'secureConnect' : 'connect'; @@ -1398,7 +1410,8 @@ class Http2Session extends EventEmitter { this[kState].pendingAck++; - const settingsFn = submitSettings.bind(this, { ...settings }, callback); + const settingsFn = FunctionPrototypeBind(submitSettings, this, + { ...settings }, callback); if (this.connecting) { this.once('connect', settingsFn); return; @@ -1422,7 +1435,9 @@ class Http2Session extends EventEmitter { validateNumber(code, 'code'); validateNumber(lastStreamID, 'lastStreamID'); - const goawayFn = submitGoaway.bind(this, code, lastStreamID, opaqueData); + const goawayFn = FunctionPrototypeBind(submitGoaway, + this, + code, lastStreamID, opaqueData); if (this.connecting) { this.once('connect', goawayFn); return; @@ -1577,7 +1592,7 @@ class ServerHttp2Session extends Http2Session { } validateString(alt, 'alt'); - if (!kQuotedString.test(alt)) + if (!RegExpPrototypeTest(kQuotedString, alt)) throw new ERR_INVALID_CHAR('alt'); // Max length permitted for ALTSVC @@ -1668,7 +1683,8 @@ class ClientHttp2Session extends Http2Session { if (getAuthority(headers) === undefined) headers[HTTP2_HEADER_AUTHORITY] = this[kAuthority]; if (headers[HTTP2_HEADER_SCHEME] === undefined) - headers[HTTP2_HEADER_SCHEME] = this[kProtocol].slice(0, -1); + headers[HTTP2_HEADER_SCHEME] = StringPrototypeSlice(this[kProtocol], + 0, -1); if (headers[HTTP2_HEADER_PATH] === undefined) headers[HTTP2_HEADER_PATH] = '/'; } else { @@ -1705,14 +1721,15 @@ class ClientHttp2Session extends Http2Session { if (options.waitForTrailers) stream[kState].flags |= STREAM_FLAGS_HAS_TRAILERS; - const onConnect = requestOnConnect.bind(stream, headersList, options); + const onConnect = FunctionPrototypeBind(requestOnConnect, + stream, headersList, options); if (this.connecting) { if (this[kPendingRequestCalls] !== null) { - this[kPendingRequestCalls].push(onConnect); + ArrayPrototypePush(this[kPendingRequestCalls], onConnect); } else { this[kPendingRequestCalls] = [onConnect]; this.once('connect', () => { - this[kPendingRequestCalls].forEach((f) => f()); + ArrayPrototypeForEach(this[kPendingRequestCalls], (f) => f()); this[kPendingRequestCalls] = null; }); } @@ -1767,7 +1784,7 @@ function shutdownWritable(callback) { req.handle = handle; const err = handle.shutdown(req); if (err === 1) // synchronous finish - return afterShutdown.call(req, 0); + return ReflectApply(afterShutdown, req, [0]); } function finishSendTrailers(stream, headersList) { @@ -1816,7 +1833,7 @@ function closeStream(stream, code, rstStreamStatus = kSubmitRstStream) { } if (rstStreamStatus !== kNoRstStream) { - const finishFn = finishCloseStream.bind(stream, code); + const finishFn = FunctionPrototypeBind(finishCloseStream, stream, code); if (!ending || stream.writableFinished || code !== NGHTTP2_NO_ERROR || rstStreamStatus === kForceRstStream) finishFn(); @@ -1826,7 +1843,7 @@ function closeStream(stream, code, rstStreamStatus = kSubmitRstStream) { } function finishCloseStream(code) { - const rstStreamFn = submitRstStream.bind(this, code); + const rstStreamFn = FunctionPrototypeBind(submitRstStream, 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. @@ -2010,7 +2027,8 @@ class Http2Stream extends Duplex { if (this.pending) { this.once( 'ready', - this[kWriteGeneric].bind(this, writev, data, encoding, cb) + FunctionPrototypeBind(this[kWriteGeneric], + this, writev, data, encoding, cb) ); return; } @@ -2089,7 +2107,7 @@ class Http2Stream extends Duplex { return; } debugStreamObj(this, 'shutting down writable on _final'); - shutdownWritable.call(this, cb); + ReflectApply(shutdownWritable, this, [cb]); } _read(nread) { @@ -2102,7 +2120,7 @@ class Http2Stream extends Duplex { this[kState].didRead = true; } if (!this.pending) { - streamOnResume.call(this); + ReflectApply(streamOnResume, this, []); } else { this.once('ready', streamOnResume); } @@ -2116,7 +2134,7 @@ class Http2Stream extends Duplex { options = { ...options }; setAndValidatePriorityOptions(options); - const priorityFn = submitPriority.bind(this, options); + const priorityFn = FunctionPrototypeBind(submitPriority, 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. @@ -2346,7 +2364,8 @@ function processHeaders(oldHeaders, options) { function onFileUnpipe() { const stream = this.sink[kOwner]; if (stream.ownsFd) - this.source.close().catch(stream.destroy.bind(stream)); + PromisePrototypeCatch(this.source.close(), + FunctionPrototypeBind(stream.destroy, stream)); else this.source.releaseFD(); } @@ -2431,7 +2450,8 @@ function doSendFD(session, options, fd, headers, streamOptions, err, stat) { // response is canceled. The user code may also send a separate type // of response so check again for the HEADERS_SENT flag if ((typeof options.statCheck === 'function' && - options.statCheck.call(this, stat, headers, statOptions) === false) || + ReflectApply(options.statCheck, this, + [stat, headers, statOptions]) === false) || (this[kState].flags & STREAM_FLAGS_HEADERS_SENT)) { return; } @@ -2490,7 +2510,7 @@ function doSendFileFD(session, options, fd, headers, streamOptions, err, stat) { // response is canceled. The user code may also send a separate type // of response so check again for the HEADERS_SENT flag if ((typeof options.statCheck === 'function' && - options.statCheck.call(this, stat, headers) === false) || + ReflectApply(options.statCheck, this, [stat, headers]) === false) || (this[kState].flags & STREAM_FLAGS_HEADERS_SENT)) { tryClose(fd); return; @@ -2528,8 +2548,9 @@ function afterOpen(session, options, headers, streamOptions, err, fd) { state.fd = fd; fs.fstat(fd, - doSendFileFD.bind(this, session, options, fd, - headers, streamOptions)); + FunctionPrototypeBind(doSendFileFD, this, + session, options, fd, + headers, streamOptions)); } class ServerHttp2Stream extends Http2Stream { @@ -2732,8 +2753,9 @@ class ServerHttp2Stream extends Http2Stream { if (options.statCheck !== undefined) { fs.fstat(fd, - doSendFD.bind(this, session, options, fd, - headers, streamOptions)); + FunctionPrototypeBind(doSendFD, this, + session, options, fd, + headers, streamOptions)); return; } @@ -2792,7 +2814,8 @@ class ServerHttp2Stream extends Http2Stream { } fs.open(path, 'r', - afterOpen.bind(this, session, options, headers, streamOptions)); + FunctionPrototypeBind(afterOpen, this, + session, options, headers, streamOptions)); } // Sends a block of informational headers. In theory, the HTTP/2 spec @@ -2828,7 +2851,7 @@ class ServerHttp2Stream extends Http2Stream { if (!this[kInfoHeaders]) this[kInfoHeaders] = [headers]; else - this[kInfoHeaders].push(headers); + ArrayPrototypePush(this[kInfoHeaders], headers); const ret = this[kHandle].info(headersList); if (ret < 0) @@ -2982,7 +3005,7 @@ function initializeTLSOptions(options, servername) { options = initializeOptions(options); options.ALPNProtocols = ['h2']; if (options.allowHTTP1 === true) - options.ALPNProtocols.push('http/1.1'); + ArrayPrototypePush(options.ALPNProtocols, 'http/1.1'); if (servername !== undefined && options.servername === undefined) options.servername = servername; return options; @@ -3079,18 +3102,18 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function( } break; default: - net.Server.prototype[EventEmitter.captureRejectionSymbol] - .call(this, err, event, ...args); + ReflectApply(net.Server.prototype[EventEmitter.captureRejectionSymbol], + this, [err, event, ...args]); } }; function setupCompat(ev) { if (ev === 'request') { this.removeListener('newListener', setupCompat); - this.on('stream', onServerStream.bind( - this, - this[kOptions].Http2ServerRequest, - this[kOptions].Http2ServerResponse) + this.on('stream', FunctionPrototypeBind(onServerStream, + this, + this[kOptions].Http2ServerRequest, + this[kOptions].Http2ServerResponse) ); } } @@ -3131,7 +3154,7 @@ function connect(authority, options, listener) { host = authority.hostname; if (host[0] === '[') - host = host.slice(1, -1); + host = StringPrototypeSlice(host, 1, -1); } else if (authority.host) { host = authority.host; } diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index b3fdf420c8c2b5..dadfb68e8b5289 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -2,14 +2,18 @@ const { ArrayIsArray, + ArrayPrototypeIncludes, + ArrayPrototypeMap, + ArrayPrototypePush, Error, MathMax, Number, ObjectCreate, ObjectKeys, - Set, + SafeSet, String, StringFromCharCode, + StringPrototypeIncludes, StringPrototypeToLowerCase, Symbol, } = primordials; @@ -98,7 +102,7 @@ const { // This set is defined strictly by the HTTP/2 specification. Only // :-prefixed headers defined by that specification may be added to // this set. -const kValidPseudoHeaders = new Set([ +const kValidPseudoHeaders = new SafeSet([ HTTP2_HEADER_STATUS, HTTP2_HEADER_METHOD, HTTP2_HEADER_AUTHORITY, @@ -109,7 +113,7 @@ const kValidPseudoHeaders = new Set([ // This set contains headers that are permitted to have only a single // value. Multiple instances must not be specified. -const kSingleValueHeaders = new Set([ +const kSingleValueHeaders = new SafeSet([ HTTP2_HEADER_STATUS, HTTP2_HEADER_METHOD, HTTP2_HEADER_AUTHORITY, @@ -156,7 +160,7 @@ const kSingleValueHeaders = new Set([ // meaning to the request payload. By default, unless the user explicitly // overrides the endStream option on the request method, the endStream // option will be defaulted to true when these methods are used. -const kNoPayloadMethods = new Set([ +const kNoPayloadMethods = new SafeSet([ HTTP2_METHOD_DELETE, HTTP2_METHOD_GET, HTTP2_METHOD_HEAD @@ -468,7 +472,7 @@ function mapToHeaders(map, let ret = ''; let count = 0; const keys = ObjectKeys(map); - const singles = new Set(); + const singles = new SafeSet(); let i, j; let isArray; let key; @@ -476,13 +480,14 @@ function mapToHeaders(map, let isSingleValueHeader; let err; const neverIndex = - (map[kSensitiveHeaders] || emptyArray).map(StringPrototypeToLowerCase); + ArrayPrototypeMap(map[kSensitiveHeaders] || emptyArray, + StringPrototypeToLowerCase); for (i = 0; i < keys.length; ++i) { key = keys[i]; value = map[key]; if (value === undefined || key === '') continue; - key = key.toLowerCase(); + key = StringPrototypeToLowerCase(key); isSingleValueHeader = kSingleValueHeaders.has(key); isArray = ArrayIsArray(value); if (isArray) { @@ -505,7 +510,9 @@ function mapToHeaders(map, throw new ERR_HTTP2_HEADER_SINGLE_VALUE(key); singles.add(key); } - const flags = neverIndex.includes(key) ? kNeverIndexFlag : kNoHeaderFlags; + const flags = ArrayPrototypeIncludes(neverIndex, key) ? + kNeverIndexFlag : + kNoHeaderFlags; if (key[0] === ':') { err = assertValuePseudoHeader(key); if (err !== undefined) @@ -514,7 +521,7 @@ function mapToHeaders(map, count++; continue; } - if (key.indexOf(' ') >= 0) { + if (StringPrototypeIncludes(key, ' ')) { throw new ERR_INVALID_HTTP_TOKEN('Header name', key); } if (isIllegalConnectionSpecificHeader(key, value)) { @@ -595,7 +602,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." - existing.push(value); + ArrayPrototypePush(existing, value); break; default: // https://tools.ietf.org/html/rfc7230#section-3.2.2