Skip to content

Commit

Permalink
http: refactor outgoing headers processing
Browse files Browse the repository at this point in the history 
Use a shared function, for..in instead of Object.keys, do less work in
`setHeader` and instead defer some of it until later, and other minor
changes to improve clarity, as well as a slight boost in performance.

PR-URL: #20250
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@apapirovski @MylesBorins
apapirovski authored and MylesBorins committed May 4, 2018
1 parent e0d2bc5 commit f0b2b26
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 103 deletions.
36 changes: 36 additions & 0 deletions benchmark/http/headers.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';

const common = require('../common.js');
const http = require('http');

const bench = common.createBenchmark(main, {
duplicates: [1, 100],
n: [10, 1000],
});

function main({ duplicates, n }) {
const headers = {
'Connection': 'keep-alive',
'Transfer-Encoding': 'chunked',
};

for (var i = 0; i < n / duplicates; i++) {
headers[`foo${i}`] = [];
for (var j = 0; j < duplicates; j++) {
headers[`foo${i}`].push(`some header value ${i}`);
}
}

const server = http.createServer(function(req, res) {
res.writeHead(200, headers);
res.end();
});
server.listen(common.PORT, function() {
bench.http({
path: '/',
connections: 10
}, function() {
server.close();
});
});
}
178 changes: 79 additions & 99 deletions lib/_http_outgoing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ const { utcDate } = internalHttp;

const kIsCorked = Symbol('isCorked');

const hasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty);

var RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
var RE_TE_CHUNKED = common.chunkExpression;

Expand Down Expand Up @@ -116,7 +118,7 @@ Object.defineProperty(OutgoingMessage.prototype, '_headers', {
if (val == null) {
this[outHeadersKey] = null;
} else if (typeof val === 'object') {
const headers = this[outHeadersKey] = {};
const headers = this[outHeadersKey] = Object.create(null);
const keys = Object.keys(val);
for (var i = 0; i < keys.length; ++i) {
const name = keys[i];
Expand All @@ -129,7 +131,7 @@ Object.defineProperty(OutgoingMessage.prototype, '_headers', {
Object.defineProperty(OutgoingMessage.prototype, '_headerNames', {
get: function() {
const headers = this[outHeadersKey];
if (headers) {
if (headers !== null) {
const out = Object.create(null);
const keys = Object.keys(headers);
for (var i = 0; i < keys.length; ++i) {
Expand All @@ -138,9 +140,8 @@ Object.defineProperty(OutgoingMessage.prototype, '_headerNames', {
out[key] = val;
}
return out;
} else {
return headers;
}
return null;
},
set: function(val) {
if (typeof val === 'object' && val !== null) {
Expand All @@ -164,14 +165,14 @@ OutgoingMessage.prototype._renderHeaders = function _renderHeaders() {
}

var headersMap = this[outHeadersKey];
if (!headersMap) return {};

var headers = {};
var keys = Object.keys(headersMap);
const headers = {};

for (var i = 0, l = keys.length; i < l; i++) {
var key = keys[i];
headers[headersMap[key][0]] = headersMap[key][1];
if (headersMap !== null) {
const keys = Object.keys(headersMap);
for (var i = 0, l = keys.length; i < l; i++) {
const key = keys[i];
headers[headersMap[key][0]] = headersMap[key][1];
}
}
return headers;
};
Expand Down Expand Up @@ -285,72 +286,40 @@ OutgoingMessage.prototype._storeHeader = _storeHeader;
function _storeHeader(firstLine, headers) {
// firstLine in the case of request is: 'GET /index.html HTTP/1.1\r\n'
// in the case of response it is: 'HTTP/1.1 200 OK\r\n'
var state = {
const state = {
connection: false,
contLen: false,
te: false,
date: false,
expect: false,
trailer: false,
upgrade: false,
header: firstLine
};

var field;
var key;
var value;
var i;
var j;
if (headers === this[outHeadersKey]) {
for (key in headers) {
var entry = headers[key];
field = entry[0];
value = entry[1];

if (value instanceof Array) {
if (value.length < 2 || !isCookieField(field)) {
for (j = 0; j < value.length; j++)
storeHeader(this, state, field, value[j], false);
continue;
}
value = value.join('; ');
}
storeHeader(this, state, field, value, false);
const entry = headers[key];
processHeader(this, state, entry[0], entry[1], false);
}
} else 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], true);
}
} else {
storeHeader(this, state, field, value, true);
}
} else if (Array.isArray(headers)) {
for (var i = 0; i < headers.length; i++) {
const entry = headers[i];
processHeader(this, state, entry[0], entry[1], true);
}
} else if (headers) {
var keys = Object.keys(headers);
for (i = 0; i < keys.length; i++) {
field = keys[i];
value = headers[field];

if (value instanceof Array) {
if (value.length < 2 || !isCookieField(field)) {
for (j = 0; j < value.length; j++)
storeHeader(this, state, field, value[j], true);
continue;
}
value = value.join('; ');
for (key in headers) {
if (hasOwnProperty(headers, key)) {
processHeader(this, state, key, headers[key], true);
}
storeHeader(this, state, field, value, true);
}
}

let { header } = state;

// Date header
if (this.sendDate && !state.date) {
state.header += 'Date: ' + utcDate() + CRLF;
header += 'Date: ' + utcDate() + CRLF;
}

// Force the connection to close when the response is a 204 No Content or
Expand All @@ -364,9 +333,9 @@ function _storeHeader(firstLine, headers) {
// It was pointed out that this might confuse reverse proxies to the point
// 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) {
debug(statusCode + ' response should not use chunked encoding,' +
if (this.chunkedEncoding && (this.statusCode === 204 ||
this.statusCode === 304)) {
debug(this.statusCode + ' response should not use chunked encoding,' +
' closing connection.');
this.chunkedEncoding = false;
this.shouldKeepAlive = false;
Expand All @@ -377,13 +346,13 @@ function _storeHeader(firstLine, headers) {
this._last = true;
this.shouldKeepAlive = false;
} else if (!state.connection) {
var shouldSendKeepAlive = this.shouldKeepAlive &&
const shouldSendKeepAlive = this.shouldKeepAlive &&
(state.contLen || this.useChunkedEncodingByDefault || this.agent);
if (shouldSendKeepAlive) {
state.header += 'Connection: keep-alive\r\n';
header += 'Connection: keep-alive\r\n';
} else {
this._last = true;
state.header += 'Connection: close\r\n';
header += 'Connection: close\r\n';
}
}

Expand All @@ -396,9 +365,9 @@ function _storeHeader(firstLine, headers) {
} else if (!state.trailer &&
!this._removedContLen &&
typeof this._contentLength === 'number') {
state.header += 'Content-Length: ' + this._contentLength + CRLF;
header += 'Content-Length: ' + this._contentLength + CRLF;
} else if (!this._removedTE) {
state.header += 'Transfer-Encoding: chunked\r\n';
header += 'Transfer-Encoding: chunked\r\n';
this.chunkedEncoding = true;
} else {
// We should only be able to get here if both Content-Length and
Expand All @@ -416,18 +385,31 @@ function _storeHeader(firstLine, headers) {
throw new ERR_HTTP_TRAILER_INVALID();
}

this._header = state.header + CRLF;
this._header = header + CRLF;
this._headerSent = false;

// wait until the first body chunk, or close(), is sent to flush,
// UNLESS we're sending Expect: 100-continue.
if (state.expect) this._send('');
}

function storeHeader(self, state, key, value, validate) {
if (validate) {
validateHeader(key, value);
function processHeader(self, state, key, value, validate) {
if (validate)
validateHeaderName(key);
if (Array.isArray(value)) {
if (value.length < 2 || !isCookieField(key)) {
for (var i = 0; i < value.length; i++)
storeHeader(self, state, key, value[i], validate);
return;
}
value = value.join('; ');
}
storeHeader(self, state, key, value, validate);
}

function storeHeader(self, state, key, value, validate) {
if (validate)
validateHeaderValue(key, value);
state.header += key + ': ' + escapeHeaderValue(value) + CRLF;
matchHeader(self, state, key, value);
}
Expand All @@ -439,39 +421,47 @@ function matchHeader(self, state, field, value) {
switch (field) {
case 'connection':
state.connection = true;
self._removedConnection = false;
if (RE_CONN_CLOSE.test(value))
self._last = true;
else
self.shouldKeepAlive = true;
break;
case 'transfer-encoding':
state.te = true;
self._removedTE = false;
if (RE_TE_CHUNKED.test(value)) self.chunkedEncoding = true;
break;
case 'content-length':
state.contLen = true;
self._removedContLen = false;
break;
case 'date':
case 'expect':
case 'trailer':
case 'upgrade':
state[field] = true;
break;
}
}

function validateHeader(name, value) {
let err;
function validateHeaderName(name) {
if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) {
err = new ERR_INVALID_HTTP_TOKEN('Header name', name);
} else if (value === undefined) {
const err = new ERR_INVALID_HTTP_TOKEN('Header name', name);
Error.captureStackTrace(err, validateHeaderName);
throw err;
}
}

function validateHeaderValue(name, value) {
let err;
if (value === undefined) {
err = new ERR_HTTP_INVALID_HEADER_VALUE(value, name);
} else if (checkInvalidHeaderChar(value)) {
debug('Header "%s" contains invalid characters', name);
err = new ERR_INVALID_CHAR('header content', name);
}
if (err !== undefined) {
Error.captureStackTrace(err, validateHeader);
Error.captureStackTrace(err, validateHeaderValue);
throw err;
}
}
Expand All @@ -480,25 +470,14 @@ OutgoingMessage.prototype.setHeader = function setHeader(name, value) {
if (this._header) {
throw new ERR_HTTP_HEADERS_SENT('set');
}
validateHeader(name, value);
validateHeaderName(name);
validateHeaderValue(name, value);

if (!this[outHeadersKey])
this[outHeadersKey] = {};
let headers = this[outHeadersKey];
if (headers === null)
this[outHeadersKey] = headers = Object.create(null);

const key = name.toLowerCase();
this[outHeadersKey][key] = [name, value];

switch (key) {
case 'connection':
this._removedConnection = false;
break;
case 'content-length':
this._removedContLen = false;
break;
case 'transfer-encoding':
this._removedTE = false;
break;
}
headers[name.toLowerCase()] = [name, value];
};


Expand All @@ -507,18 +486,18 @@ OutgoingMessage.prototype.getHeader = function getHeader(name) {
throw new ERR_INVALID_ARG_TYPE('name', 'string', name);
}

if (!this[outHeadersKey]) return;

var entry = this[outHeadersKey][name.toLowerCase()];
if (!entry)
const headers = this[outHeadersKey];
if (headers === null)
return;
return entry[1];

const entry = headers[name.toLowerCase()];
return entry && entry[1];
};


// Returns an array of the names of the current outgoing headers.
OutgoingMessage.prototype.getHeaderNames = function getHeaderNames() {
return (this[outHeadersKey] ? Object.keys(this[outHeadersKey]) : []);
return this[outHeadersKey] !== null ? Object.keys(this[outHeadersKey]) : [];
};


Expand All @@ -543,7 +522,8 @@ OutgoingMessage.prototype.hasHeader = function hasHeader(name) {
throw new ERR_INVALID_ARG_TYPE('name', 'string', name);
}

return !!(this[outHeadersKey] && this[outHeadersKey][name.toLowerCase()]);
return this[outHeadersKey] !== null &&
!!this[outHeadersKey][name.toLowerCase()];
};


Expand Down Expand Up @@ -573,7 +553,7 @@ OutgoingMessage.prototype.removeHeader = function removeHeader(name) {
break;
}

if (this[outHeadersKey]) {
if (this[outHeadersKey] !== null) {
delete this[outHeadersKey][key];
}
};
Expand Down
Loading

0 comments on commit f0b2b26

Please sign in to comment.