From 175ed520c1db6cb5a386068643f733eb984e1205 Mon Sep 17 00:00:00 2001 From: Brian White Date: Sun, 18 Dec 2016 08:48:40 -0500 Subject: [PATCH 1/8] http: reuse existing headers array for raw values PR-URL: https://github.com/nodejs/node/pull/6533 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/_http_incoming.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index 5c04ab9dfe3331..d02f19424c0442 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -103,21 +103,17 @@ IncomingMessage.prototype.destroy = function destroy(error) { IncomingMessage.prototype._addHeaderLines = _addHeaderLines; function _addHeaderLines(headers, n) { if (headers && headers.length) { - var raw, dest; + var dest; if (this.complete) { - raw = this.rawTrailers; + this.rawTrailers = headers; dest = this.trailers; } else { - raw = this.rawHeaders; + this.rawHeaders = headers; dest = this.headers; } for (var i = 0; i < n; i += 2) { - var k = headers[i]; - var v = headers[i + 1]; - raw.push(k); - raw.push(v); - this._addHeaderLine(k, v, dest); + this._addHeaderLine(headers[i], headers[i + 1], dest); } } } From 14c76f8671c326d7d4b750fd2cc33b63de7e553e Mon Sep 17 00:00:00 2001 From: Brian White Date: Sun, 18 Dec 2016 08:56:52 -0500 Subject: [PATCH 2/8] http: misc cleanup and minor optimizations PR-URL: https://github.com/nodejs/node/pull/6533 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/_http_common.js | 1 - lib/_http_outgoing.js | 8 +++----- lib/_http_server.js | 9 +++++---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/_http_common.js b/lib/_http_common.js index 46408077316207..b0767fd43ca1b8 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -198,7 +198,6 @@ function freeParser(parser, req, socket) { parser[kOnExecute] = null; if (parsers.free(parser) === false) parser.close(); - parser = null; } if (req) { req.parser = null; diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index b85e3c54074d82..8d134ecc501083 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -380,8 +380,7 @@ OutgoingMessage.prototype.getHeader = function getHeader(name) { if (!this._headers) return; - var key = name.toLowerCase(); - return this._headers[key]; + return this._headers[name.toLowerCase()]; }; @@ -585,7 +584,6 @@ OutgoingMessage.prototype.end = function end(data, encoding, callback) { if (this.connection && data) this.connection.cork(); - var ret; if (data) { // Normal body write. this.write(data, encoding); @@ -598,6 +596,7 @@ OutgoingMessage.prototype.end = function end(data, encoding, callback) { this.emit('finish'); }; + var ret; if (this._hasBody && this.chunkedEncoding) { ret = this._send('0\r\n' + this._trailer + '\r\n', 'latin1', finish); } else { @@ -677,8 +676,7 @@ OutgoingMessage.prototype._flushOutput = function _flushOutput(socket) { var outputCallbacks = this.outputCallbacks; socket.cork(); for (var i = 0; i < outputLength; i++) { - ret = socket.write(output[i], outputEncodings[i], - outputCallbacks[i]); + ret = socket.write(output[i], outputEncodings[i], outputCallbacks[i]); } socket.uncork(); diff --git a/lib/_http_server.js b/lib/_http_server.js index c5f92d52d54d9b..b32e484b4bcfa3 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -88,6 +88,8 @@ function ServerResponse(req) { if (req.method === 'HEAD') this._hasBody = false; this.sendDate = true; + this._sent100 = false; + this._expect_continue = false; if (req.httpVersionMajor < 1 || req.httpVersionMinor < 1) { this.useChunkedEncodingByDefault = chunkExpression.test(req.headers.te); @@ -195,8 +197,7 @@ function writeHead(statusCode, reason, obj) { if (common._checkInvalidHeaderChar(this.statusMessage)) throw new Error('Invalid character in statusMessage.'); - var statusLine = 'HTTP/1.1 ' + statusCode.toString() + ' ' + - this.statusMessage + CRLF; + var statusLine = 'HTTP/1.1 ' + statusCode + ' ' + this.statusMessage + CRLF; if (statusCode === 204 || statusCode === 304 || (100 <= statusCode && statusCode <= 199)) { @@ -232,7 +233,7 @@ function Server(requestListener) { net.Server.call(this, { allowHalfOpen: true }); if (requestListener) { - this.addListener('request', requestListener); + this.on('request', requestListener); } /* eslint-disable max-len */ @@ -242,7 +243,7 @@ function Server(requestListener) { /* eslint-enable max-len */ this.httpAllowHalfOpen = false; - this.addListener('connection', connectionListener); + this.on('connection', connectionListener); this.timeout = 2 * 60 * 1000; From 832271c88f7950678595de1a329f4033e937a24e Mon Sep 17 00:00:00 2001 From: Brian White Date: Sun, 18 Dec 2016 21:20:29 -0500 Subject: [PATCH 3/8] http: refactor server connection handling PR-URL: https://github.com/nodejs/node/pull/6533 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/_http_server.js | 440 +++++++++++++++++++++++--------------------- 1 file changed, 226 insertions(+), 214 deletions(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index b32e484b4bcfa3..21113fc37a9f5b 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -264,40 +264,6 @@ exports.Server = Server; function connectionListener(socket) { - var self = this; - var outgoing = []; - var incoming = []; - var outgoingData = 0; - - function updateOutgoingData(delta) { - // `outgoingData` is an approximate amount of bytes queued through all - // inactive responses. If more data than the high watermark is queued - we - // need to pause TCP socket/HTTP parser, and wait until the data will be - // sent to the client. - outgoingData += delta; - if (socket._paused && outgoingData < socket._writableState.highWaterMark) - return socketOnDrain(); - } - - function abortIncoming() { - while (incoming.length) { - var req = incoming.shift(); - req.emit('aborted'); - req.emit('close'); - } - // abort socket._httpMessage ? - } - - function serverSocketCloseListener() { - debug('server socket close'); - // mark this parser as reusable - if (this.parser) { - freeParser(this.parser, null, this); - } - - abortIncoming(); - } - debug('SERVER new http connection'); httpSocketSetup(socket); @@ -305,18 +271,9 @@ function connectionListener(socket) { // If the user has added a listener to the server, // request, or response, then it's their responsibility. // otherwise, destroy on timeout by default - if (self.timeout) - socket.setTimeout(self.timeout); - socket.on('timeout', function() { - var req = socket.parser && socket.parser.incoming; - var reqTimeout = req && !req.complete && req.emit('timeout', socket); - var res = socket._httpMessage; - var resTimeout = res && res.emit('timeout', socket); - var serverTimeout = self.emit('timeout', socket); - - if (!reqTimeout && !resTimeout && !serverTimeout) - socket.destroy(); - }); + if (this.timeout) + socket.setTimeout(this.timeout); + socket.on('timeout', socketOnTimeout.bind(undefined, this, socket)); var parser = parsers.alloc(); parser.reinitialize(HTTPParser.REQUEST); @@ -332,17 +289,34 @@ function connectionListener(socket) { parser.maxHeaderPairs = 2000; } - socket.addListener('error', socketOnError); - socket.addListener('close', serverSocketCloseListener); - parser.onIncoming = parserOnIncoming; - socket.on('end', socketOnEnd); - socket.on('data', socketOnData); + var state = { + onData: null, + onError: null, + onEnd: null, + onClose: null, + outgoing: [], + incoming: [], + // `outgoingData` is an approximate amount of bytes queued through all + // inactive responses. If more data than the high watermark is queued - we + // need to pause TCP socket/HTTP parser, and wait until the data will be + // sent to the client. + outgoingData: 0 + }; + state.onData = socketOnData.bind(undefined, this, socket, parser, state); + state.onError = socketOnError.bind(undefined, this, socket, state); + state.onEnd = socketOnEnd.bind(undefined, this, socket, parser, state); + state.onClose = socketOnClose.bind(undefined, socket, state); + socket.on('data', state.onData); + socket.on('error', state.onError); + socket.on('end', state.onEnd); + socket.on('close', state.onClose); + parser.onIncoming = parserOnIncoming.bind(undefined, this, socket, state); // We are consuming socket, so it won't get any actual data socket.on('resume', onSocketResume); socket.on('pause', onSocketPause); - socket.on('drain', socketOnDrain); + socket.on('drain', socketOnDrain.bind(undefined, socket, state)); // Override on to unconsume on `data`, `readable` listeners socket.on = socketOnWrap; @@ -352,205 +326,243 @@ function connectionListener(socket) { parser._consumed = true; parser.consume(external); } - external = null; - parser[kOnExecute] = onParserExecute; + parser[kOnExecute] = + onParserExecute.bind(undefined, this, socket, parser, state); - // TODO(isaacs): Move all these functions out of here - function socketOnError(e) { - // Ignore further errors - this.removeListener('error', socketOnError); - this.on('error', () => {}); + socket._paused = false; +} +exports._connectionListener = connectionListener; - if (!self.emit('clientError', e, this)) - this.destroy(e); +function updateOutgoingData(socket, state, delta) { + state.outgoingData += delta; + if (socket._paused && + state.outgoingData < socket._writableState.highWaterMark) { + return socketOnDrain(socket, state); } +} - function socketOnData(d) { - assert(!socket._paused); - debug('SERVER socketOnData %d', d.length); - var ret = parser.execute(d); +function socketOnDrain(socket, state) { + var needPause = state.outgoingData > socket._writableState.highWaterMark; - onParserExecuteCommon(ret, d); + // If we previously paused, then start reading again. + if (socket._paused && !needPause) { + socket._paused = false; + if (socket.parser) + socket.parser.resume(); + socket.resume(); } +} + +function socketOnTimeout(server, socket) { + var req = socket.parser && socket.parser.incoming; + var reqTimeout = req && !req.complete && req.emit('timeout', socket); + var res = socket._httpMessage; + var resTimeout = res && res.emit('timeout', socket); + var serverTimeout = server.emit('timeout', socket); + + if (!reqTimeout && !resTimeout && !serverTimeout) + socket.destroy(); +} - function onParserExecute(ret, d) { - socket._unrefTimer(); - debug('SERVER socketOnParserExecute %d', ret); - onParserExecuteCommon(ret, undefined); +function socketOnClose(socket, state) { + debug('server socket close'); + // mark this parser as reusable + if (socket.parser) { + freeParser(socket.parser, null, socket); } - function onParserExecuteCommon(ret, d) { - if (ret instanceof Error) { - debug('parse error'); - socketOnError.call(socket, ret); - } else if (parser.incoming && parser.incoming.upgrade) { - // Upgrade or CONNECT - var bytesParsed = ret; - var req = parser.incoming; - debug('SERVER upgrade or connect', req.method); - - if (!d) - d = parser.getCurrentBuffer(); - - socket.removeListener('data', socketOnData); - socket.removeListener('end', socketOnEnd); - socket.removeListener('close', serverSocketCloseListener); - unconsume(parser, socket); - parser.finish(); - freeParser(parser, req, null); - parser = null; - - var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade'; - if (self.listenerCount(eventName) > 0) { - debug('SERVER have listener for %s', eventName); - var bodyHead = d.slice(bytesParsed, d.length); - - // TODO(isaacs): Need a way to reset a stream to fresh state - // IE, not flowing, and not explicitly paused. - socket._readableState.flowing = null; - self.emit(eventName, req, socket, bodyHead); - } else { - // Got upgrade header or CONNECT method, but have no handler. - socket.destroy(); - } - } + abortIncoming(state.incoming); +} - if (socket._paused && socket.parser) { - // onIncoming paused the socket, we should pause the parser as well - debug('pause parser'); - socket.parser.pause(); - } +function abortIncoming(incoming) { + while (incoming.length) { + var req = incoming.shift(); + req.emit('aborted'); + req.emit('close'); } + // abort socket._httpMessage ? +} - function socketOnEnd() { - var socket = this; - var ret = parser.finish(); +function socketOnEnd(server, socket, parser, state) { + var ret = parser.finish(); - if (ret instanceof Error) { - debug('parse error'); - socketOnError.call(socket, ret); - return; - } + if (ret instanceof Error) { + debug('parse error'); + state.onError(ret); + return; + } - if (!self.httpAllowHalfOpen) { - abortIncoming(); - if (socket.writable) socket.end(); - } else if (outgoing.length) { - outgoing[outgoing.length - 1]._last = true; - } else if (socket._httpMessage) { - socket._httpMessage._last = true; - } else { - if (socket.writable) socket.end(); - } + if (!server.httpAllowHalfOpen) { + abortIncoming(state.incoming); + if (socket.writable) socket.end(); + } else if (state.outgoing.length) { + state.outgoing[state.outgoing.length - 1]._last = true; + } else if (socket._httpMessage) { + socket._httpMessage._last = true; + } else { + if (socket.writable) socket.end(); } +} +function socketOnData(server, socket, parser, state, d) { + assert(!socket._paused); + debug('SERVER socketOnData %d', d.length); + var ret = parser.execute(d); - // The following callback is issued after the headers have been read on a - // new message. In this callback we setup the response object and pass it - // to the user. + onParserExecuteCommon(server, socket, parser, state, ret, d); +} - socket._paused = false; - function socketOnDrain() { - var needPause = outgoingData > socket._writableState.highWaterMark; - - // If we previously paused, then start reading again. - if (socket._paused && !needPause) { - socket._paused = false; - if (socket.parser) - socket.parser.resume(); - socket.resume(); +function onParserExecute(server, socket, parser, state, ret, d) { + socket._unrefTimer(); + debug('SERVER socketOnParserExecute %d', ret); + onParserExecuteCommon(server, socket, parser, state, ret, undefined); +} + +function socketOnError(server, socket, state, e) { + // Ignore further errors + socket.removeListener('error', state.onError); + socket.on('error', () => {}); + + if (!server.emit('clientError', e, socket)) + socket.destroy(e); +} + +function onParserExecuteCommon(server, socket, parser, state, ret, d) { + if (ret instanceof Error) { + debug('parse error'); + state.onError(ret); + } else if (parser.incoming && parser.incoming.upgrade) { + // Upgrade or CONNECT + var bytesParsed = ret; + var req = parser.incoming; + debug('SERVER upgrade or connect', req.method); + + if (!d) + d = parser.getCurrentBuffer(); + + socket.removeListener('data', state.onData); + socket.removeListener('end', state.onEnd); + socket.removeListener('close', state.onClose); + unconsume(parser, socket); + parser.finish(); + freeParser(parser, req, null); + parser = null; + + var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade'; + if (server.listenerCount(eventName) > 0) { + debug('SERVER have listener for %s', eventName); + var bodyHead = d.slice(bytesParsed, d.length); + + // TODO(isaacs): Need a way to reset a stream to fresh state + // IE, not flowing, and not explicitly paused. + socket._readableState.flowing = null; + server.emit(eventName, req, socket, bodyHead); + } else { + // Got upgrade header or CONNECT method, but have no handler. + socket.destroy(); } } - function parserOnIncoming(req, shouldKeepAlive) { - incoming.push(req); - - // If the writable end isn't consuming, then stop reading - // so that we don't become overwhelmed by a flood of - // pipelined requests that may never be resolved. - if (!socket._paused) { - var needPause = socket._writableState.needDrain || - outgoingData >= socket._writableState.highWaterMark; - if (needPause) { - socket._paused = true; - // We also need to pause the parser, but don't do that until after - // the call to execute, because we may still be processing the last - // chunk. - socket.pause(); - } - } + if (socket._paused && socket.parser) { + // onIncoming paused the socket, we should pause the parser as well + debug('pause parser'); + socket.parser.pause(); + } +} - var res = new ServerResponse(req); - res._onPendingData = updateOutgoingData; +function resOnFinish(req, res, socket, state) { + // Usually the first incoming element should be our request. it may + // be that in the case abortIncoming() was called that the incoming + // array will be empty. + assert(state.incoming.length === 0 || state.incoming[0] === req); - res.shouldKeepAlive = shouldKeepAlive; - DTRACE_HTTP_SERVER_REQUEST(req, socket); - LTTNG_HTTP_SERVER_REQUEST(req, socket); - COUNTER_HTTP_SERVER_REQUEST(); + state.incoming.shift(); - if (socket._httpMessage) { - // There are already pending outgoing res, append. - outgoing.push(res); - } else { - res.assignSocket(socket); + // if the user never called req.read(), and didn't pipe() or + // .resume() or .on('data'), then we call req._dump() so that the + // bytes will be pulled off the wire. + if (!req._consuming && !req._readableState.resumeScheduled) + req._dump(); + + res.detachSocket(socket); + + if (res._last) { + socket.destroySoon(); + } else { + // start sending the next message + var m = state.outgoing.shift(); + if (m) { + m.assignSocket(socket); } + } +} - // When we're finished writing the response, check if this is the last - // response, if so destroy the socket. - res.on('finish', resOnFinish); - function resOnFinish() { - // Usually the first incoming element should be our request. it may - // be that in the case abortIncoming() was called that the incoming - // array will be empty. - assert(incoming.length === 0 || incoming[0] === req); +// The following callback is issued after the headers have been read on a +// new message. In this callback we setup the response object and pass it +// to the user. +function parserOnIncoming(server, socket, state, req, keepAlive) { + state.incoming.push(req); + + // If the writable end isn't consuming, then stop reading + // so that we don't become overwhelmed by a flood of + // pipelined requests that may never be resolved. + if (!socket._paused) { + var needPause = socket._writableState.needDrain || + state.outgoingData >= socket._writableState.highWaterMark; + if (needPause) { + socket._paused = true; + // We also need to pause the parser, but don't do that until after + // the call to execute, because we may still be processing the last + // chunk. + socket.pause(); + } + } - incoming.shift(); + var res = new ServerResponse(req); + res._onPendingData = updateOutgoingData.bind(undefined, socket, state); - // if the user never called req.read(), and didn't pipe() or - // .resume() or .on('data'), then we call req._dump() so that the - // bytes will be pulled off the wire. - if (!req._consuming && !req._readableState.resumeScheduled) - req._dump(); + res.shouldKeepAlive = keepAlive; + DTRACE_HTTP_SERVER_REQUEST(req, socket); + LTTNG_HTTP_SERVER_REQUEST(req, socket); + COUNTER_HTTP_SERVER_REQUEST(); - res.detachSocket(socket); + if (socket._httpMessage) { + // There are already pending outgoing res, append. + state.outgoing.push(res); + } else { + res.assignSocket(socket); + } - if (res._last) { - socket.destroySoon(); - } else { - // start sending the next message - var m = outgoing.shift(); - if (m) { - m.assignSocket(socket); - } - } - } + // When we're finished writing the response, check if this is the last + // response, if so destroy the socket. + var finish = + resOnFinish.bind(undefined, req, res, socket, state); + res.on('finish', finish); + + if (req.headers.expect !== undefined && + (req.httpVersionMajor === 1 && req.httpVersionMinor === 1)) { + if (continueExpression.test(req.headers.expect)) { + res._expect_continue = true; - if (req.headers.expect !== undefined && - (req.httpVersionMajor === 1 && req.httpVersionMinor === 1)) { - if (continueExpression.test(req.headers.expect)) { - res._expect_continue = true; - - if (self.listenerCount('checkContinue') > 0) { - self.emit('checkContinue', req, res); - } else { - res.writeContinue(); - self.emit('request', req, res); - } + if (server.listenerCount('checkContinue') > 0) { + server.emit('checkContinue', req, res); } else { - if (self.listenerCount('checkExpectation') > 0) { - self.emit('checkExpectation', req, res); - } else { - res.writeHead(417); - res.end(); - } + res.writeContinue(); + server.emit('request', req, res); } } else { - self.emit('request', req, res); + if (server.listenerCount('checkExpectation') > 0) { + server.emit('checkExpectation', req, res); + } else { + res.writeHead(417); + res.end(); + } } - return false; // Not a HEAD response. (Not even a response!) + } else { + server.emit('request', req, res); } + return false; // Not a HEAD response. (Not even a response!) } -exports._connectionListener = connectionListener; function onSocketResume() { // It may seem that the socket is resumed, but this is an enemy's trick to From a54972c195f708afbd985d981b804834817da92b Mon Sep 17 00:00:00 2001 From: Brian White Date: Sun, 18 Dec 2016 21:21:57 -0500 Subject: [PATCH 4/8] http: improve validation performance The new table-based lookups perform significantly better for the common cases (checking latin1 characters). PR-URL: https://github.com/nodejs/node/pull/6533 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/_http_common.js | 100 ++++++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 41 deletions(-) diff --git a/lib/_http_common.js b/lib/_http_common.js index b0767fd43ca1b8..2ce523fe6298bd 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -246,44 +246,44 @@ exports.httpSocketSetup = httpSocketSetup; * so take care when making changes to the implementation so that the source * code size does not exceed v8's default max_inlined_source_size setting. **/ -function isValidTokenChar(ch) { - if (ch >= 94 && ch <= 122) - return true; - if (ch >= 65 && ch <= 90) - return true; - if (ch === 45) - return true; - if (ch >= 48 && ch <= 57) - return true; - if (ch === 34 || ch === 40 || ch === 41 || ch === 44) +var validTokens = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0 - 15 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16 - 31 + 0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 0, // 32 - 47 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 48 - 63 + 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 64 - 79 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, // 80 - 95 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 96 - 111 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0, // 112 - 127 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 128 ... + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 // ... 255 +]; +function checkIsHttpToken(val) { + if (typeof val !== 'string' || val.length === 0) + return false; + if (!validTokens[val.charCodeAt(0)]) return false; - if (ch >= 33 && ch <= 46) + if (val.length < 2) return true; - if (ch === 124 || ch === 126) + if (!validTokens[val.charCodeAt(1)]) + return false; + if (val.length < 3) return true; - return false; -} -function checkIsHttpToken(val) { - if (typeof val !== 'string' || val.length === 0) + if (!validTokens[val.charCodeAt(2)]) return false; - if (!isValidTokenChar(val.charCodeAt(0))) + if (val.length < 4) + return true; + if (!validTokens[val.charCodeAt(3)]) return false; - const len = val.length; - if (len > 1) { - if (!isValidTokenChar(val.charCodeAt(1))) + for (var i = 4; i < val.length; ++i) { + if (!validTokens[val.charCodeAt(i)]) return false; - if (len > 2) { - if (!isValidTokenChar(val.charCodeAt(2))) - return false; - if (len > 3) { - if (!isValidTokenChar(val.charCodeAt(3))) - return false; - for (var i = 4; i < len; i++) { - if (!isValidTokenChar(val.charCodeAt(i))) - return false; - } - } - } } return true; } @@ -299,26 +299,44 @@ exports._checkIsHttpToken = checkIsHttpToken; * so take care when making changes to the implementation so that the source * code size does not exceed v8's default max_inlined_source_size setting. **/ +var validHdrChars = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, // 0 - 15 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16 - 31 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 32 - 47 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 48 - 63 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 64 - 79 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 80 - 95 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 96 - 111 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, // 112 - 127 + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 128 ... + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 // ... 255 +]; function checkInvalidHeaderChar(val) { val += ''; if (val.length < 1) return false; - var c = val.charCodeAt(0); - if ((c <= 31 && c !== 9) || c > 255 || c === 127) + if (!validHdrChars[val.charCodeAt(0)]) return true; if (val.length < 2) return false; - c = val.charCodeAt(1); - if ((c <= 31 && c !== 9) || c > 255 || c === 127) + if (!validHdrChars[val.charCodeAt(1)]) return true; if (val.length < 3) return false; - c = val.charCodeAt(2); - if ((c <= 31 && c !== 9) || c > 255 || c === 127) + if (!validHdrChars[val.charCodeAt(2)]) + return true; + if (val.length < 4) + return false; + if (!validHdrChars[val.charCodeAt(3)]) return true; - for (var i = 3; i < val.length; ++i) { - c = val.charCodeAt(i); - if ((c <= 31 && c !== 9) || c > 255 || c === 127) + for (var i = 4; i < val.length; ++i) { + if (!validHdrChars[val.charCodeAt(i)]) return true; } return false; From b6ea857c7dabf65c2826c269bb4c8a94fecf40ca Mon Sep 17 00:00:00 2001 From: Brian White Date: Sun, 18 Dec 2016 21:25:31 -0500 Subject: [PATCH 5/8] lib: avoid recompilation of anonymous functions Since at least V8 5.4, using function.bind() is now fast enough to use to avoid recompiling/reoptimizing the same anonymous functions. These changes especially impact http servers. PR-URL: https://github.com/nodejs/node/pull/6533 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/_http_outgoing.js | 7 ++++--- lib/_stream_writable.js | 35 +++++++++++++++++------------------ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 8d134ecc501083..5a9e34bedd7903 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -545,6 +545,9 @@ OutgoingMessage.prototype.addTrailers = function addTrailers(headers) { const crlf_buf = Buffer.from('\r\n'); +function onFinish(outmsg) { + outmsg.emit('finish'); +} OutgoingMessage.prototype.end = function end(data, encoding, callback) { if (typeof data === 'function') { @@ -592,9 +595,7 @@ OutgoingMessage.prototype.end = function end(data, encoding, callback) { if (typeof callback === 'function') this.once('finish', callback); - const finish = () => { - this.emit('finish'); - }; + var finish = onFinish.bind(undefined, this); var ret; if (this._hasBody && this.chunkedEncoding) { diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 06499fc947fe30..b20fe8d2ea91ed 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -86,9 +86,7 @@ function WritableState(options, stream) { this.bufferProcessing = false; // the callback that's passed to _write(chunk,cb) - this.onwrite = function(er) { - onwrite(stream, er); - }; + this.onwrite = onwrite.bind(undefined, stream); // the callback that the user supplies to write(chunk,encoding,cb) this.writecb = null; @@ -538,20 +536,21 @@ function endWritable(stream, state, cb) { function CorkedRequest(state) { this.next = null; this.entry = null; + this.finish = onCorkedFinish.bind(undefined, this, state); +} - this.finish = (err) => { - var entry = this.entry; - this.entry = null; - while (entry) { - var cb = entry.callback; - state.pendingcb--; - cb(err); - entry = entry.next; - } - if (state.corkedRequestsFree) { - state.corkedRequestsFree.next = this; - } else { - state.corkedRequestsFree = this; - } - }; +function onCorkedFinish(corkReq, state, err) { + var entry = corkReq.entry; + corkReq.entry = null; + while (entry) { + var cb = entry.callback; + state.pendingcb--; + cb(err); + entry = entry.next; + } + if (state.corkedRequestsFree) { + state.corkedRequestsFree.next = corkReq; + } else { + state.corkedRequestsFree = corkReq; + } } From e8b809a7a0c8eeb41bd1904bafeba0884cf4c265 Mon Sep 17 00:00:00 2001 From: Brian White Date: Tue, 20 Dec 2016 02:48:18 -0500 Subject: [PATCH 6/8] http: extract validation functions PR-URL: https://github.com/nodejs/node/pull/6533 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/_http_outgoing.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 5a9e34bedd7903..1e612316f4c7ee 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -7,6 +7,8 @@ const util = require('util'); const internalUtil = require('internal/util'); const Buffer = require('buffer').Buffer; const common = require('_http_common'); +const checkIsHttpToken = common._checkIsHttpToken; +const checkInvalidHeaderChar = common._checkInvalidHeaderChar; const CRLF = common.CRLF; const trfrEncChunkExpression = common.chunkExpression; @@ -312,11 +314,11 @@ function _storeHeader(firstLine, headers) { } function storeHeader(self, state, field, value) { - if (!common._checkIsHttpToken(field)) { + if (!checkIsHttpToken(field)) { throw new TypeError( 'Header name must be a valid HTTP Token ["' + field + '"]'); } - if (common._checkInvalidHeaderChar(value) === true) { + if (checkInvalidHeaderChar(value)) { debug('Header "%s" contains invalid characters', field); throw new TypeError('The header content contains invalid characters'); } @@ -350,14 +352,14 @@ function storeHeader(self, state, field, value) { OutgoingMessage.prototype.setHeader = function setHeader(name, value) { - if (!common._checkIsHttpToken(name)) + if (!checkIsHttpToken(name)) throw new TypeError( 'Header name must be a valid HTTP Token ["' + name + '"]'); if (value === undefined) throw new Error('"value" required in setHeader("' + name + '", value)'); if (this._header) throw new Error('Can\'t set headers after they are sent.'); - if (common._checkInvalidHeaderChar(value) === true) { + if (checkInvalidHeaderChar(value)) { debug('Header "%s" contains invalid characters', name); throw new TypeError('The header content contains invalid characters'); } @@ -530,11 +532,11 @@ OutgoingMessage.prototype.addTrailers = function addTrailers(headers) { field = key; value = headers[key]; } - if (!common._checkIsHttpToken(field)) { + if (!checkIsHttpToken(field)) { throw new TypeError( 'Trailer name must be a valid HTTP Token ["' + field + '"]'); } - if (common._checkInvalidHeaderChar(value) === true) { + if (checkInvalidHeaderChar(value)) { debug('Trailer "%s" contains invalid characters', field); throw new TypeError('The trailer content contains invalid characters'); } From af74e72a9f163659865f835d313f5e532956e077 Mon Sep 17 00:00:00 2001 From: Brian White Date: Tue, 20 Dec 2016 02:50:34 -0500 Subject: [PATCH 7/8] http: simplify boolean checks PR-URL: https://github.com/nodejs/node/pull/6533 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/_http_outgoing.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 1e612316f4c7ee..262b1d9f87e4d8 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -239,7 +239,7 @@ function _storeHeader(firstLine, headers) { this.upgrading = true; // Date header - if (this.sendDate === true && state.sentDateHeader === false) { + if (this.sendDate && !state.sentDateHeader) { state.messageHeader += 'Date: ' + utcDate() + CRLF; } @@ -255,8 +255,7 @@ function _storeHeader(firstLine, headers) { // of creating security liabilities, so suppress the zero chunk and force // the connection to close. var statusCode = this.statusCode; - if ((statusCode === 204 || statusCode === 304) && - this.chunkedEncoding === true) { + if ((statusCode === 204 || statusCode === 304) && this.chunkedEncoding) { debug(statusCode + ' response should not use chunked encoding,' + ' closing connection.'); this.chunkedEncoding = false; @@ -267,7 +266,7 @@ function _storeHeader(firstLine, headers) { if (this._removedHeader.connection) { this._last = true; this.shouldKeepAlive = false; - } else if (state.sentConnectionHeader === false) { + } else if (!state.sentConnectionHeader) { var shouldSendKeepAlive = this.shouldKeepAlive && (state.sentContentLengthHeader || this.useChunkedEncodingByDefault || @@ -280,8 +279,7 @@ function _storeHeader(firstLine, headers) { } } - if (state.sentContentLengthHeader === false && - state.sentTransferEncodingHeader === false) { + if (!state.sentContentLengthHeader && !state.sentTransferEncodingHeader) { if (!this._hasBody) { // Make sure we don't end the 0\r\n\r\n at the end of the message. this.chunkedEncoding = false; From 4d7531de247339692aced0f9c5643d458e80fc6e Mon Sep 17 00:00:00 2001 From: Brian White Date: Tue, 20 Dec 2016 02:53:47 -0500 Subject: [PATCH 8/8] http: optimize headers iteration This commit uses instanceof instead of Array.isArray() for faster type checking and avoids calling Object.keys() when the headers are stored as a 2D array instead of a plain object. PR-URL: https://github.com/nodejs/node/pull/6533 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/_http_outgoing.js | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 262b1d9f87e4d8..67a8d6f8bb8233 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -209,23 +209,31 @@ function _storeHeader(firstLine, headers) { messageHeader: firstLine }; - if (headers) { - var keys = Object.keys(headers); - var isArray = Array.isArray(headers); - var field, value; - - for (var i = 0, l = keys.length; i < l; i++) { - var key = keys[i]; - if (isArray) { - field = headers[key][0]; - value = headers[key][1]; + var i; + var j; + var field; + var value; + if (headers instanceof Array) { + for (i = 0; i < headers.length; ++i) { + field = headers[i][0]; + value = headers[i][1]; + + if (value instanceof Array) { + for (j = 0; j < value.length; j++) { + storeHeader(this, state, field, value[j]); + } } else { - field = key; - value = headers[key]; + storeHeader(this, state, field, value); } + } + } else if (headers) { + var keys = Object.keys(headers); + for (i = 0; i < keys.length; ++i) { + field = keys[i]; + value = headers[field]; - if (Array.isArray(value)) { - for (var j = 0; j < value.length; j++) { + if (value instanceof Array) { + for (j = 0; j < value.length; j++) { storeHeader(this, state, field, value[j]); } } else {