diff --git a/benchmark/http/create-clientrequest.js b/benchmark/http/create-clientrequest.js new file mode 100644 index 00000000000000..76134663d00a79 --- /dev/null +++ b/benchmark/http/create-clientrequest.js @@ -0,0 +1,23 @@ +'use strict'; + +var common = require('../common.js'); +var ClientRequest = require('http').ClientRequest; + +var bench = common.createBenchmark(main, { + pathlen: [1, 8, 16, 32, 64, 128], + n: [1e6] +}); + +function main(conf) { + var pathlen = +conf.pathlen; + var n = +conf.n; + + var path = '/'.repeat(pathlen); + var opts = { path: path, createConnection: function() {} }; + + bench.start(); + for (var i = 0; i < n; i++) { + new ClientRequest(opts); + } + bench.end(n); +} diff --git a/lib/_http_client.js b/lib/_http_client.js index 8e0da8a37c4b0b..d28481a67407c5 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -15,6 +15,34 @@ const Agent = require('_http_agent'); const Buffer = require('buffer').Buffer; const urlToOptions = require('internal/url').urlToOptions; +// The actual list of disallowed characters in regexp form is more like: +// /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/ +// with an additional rule for ignoring percentage-escaped characters, but +// that's a) hard to capture in a regular expression that performs well, and +// b) possibly too restrictive for real-world usage. So instead we restrict the +// filter to just control characters and spaces. +// +// This function is used in the case of small paths, where manual character code +// checks can greatly outperform the equivalent regexp (tested in V8 5.4). +function isInvalidPath(s) { + var i = 0; + if (s.charCodeAt(0) <= 32) return true; + if (++i >= s.length) return false; + if (s.charCodeAt(1) <= 32) return true; + if (++i >= s.length) return false; + if (s.charCodeAt(2) <= 32) return true; + if (++i >= s.length) return false; + if (s.charCodeAt(3) <= 32) return true; + if (++i >= s.length) return false; + if (s.charCodeAt(4) <= 32) return true; + if (++i >= s.length) return false; + if (s.charCodeAt(5) <= 32) return true; + ++i; + for (; i < s.length; ++i) + if (s.charCodeAt(i) <= 32) return true; + return false; +} + function ClientRequest(options, cb) { var self = this; OutgoingMessage.call(self); @@ -45,14 +73,20 @@ function ClientRequest(options, cb) { if (self.agent && self.agent.protocol) expectedProtocol = self.agent.protocol; - if (options.path && /[\u0000-\u0020]/.test(options.path)) { - // The actual regex is more like /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/ - // with an additional rule for ignoring percentage-escaped characters - // but that's a) hard to capture in a regular expression that performs - // well, and b) possibly too restrictive for real-world usage. - // Restrict the filter to control characters and spaces. - throw new TypeError('Request path contains unescaped characters'); - } else if (protocol !== expectedProtocol) { + var path; + if (options.path) { + path = '' + options.path; + var invalidPath; + if (path.length <= 39) { // Determined experimentally in V8 5.4 + invalidPath = isInvalidPath(path); + } else { + invalidPath = /[\u0000-\u0020]/.test(path); + } + if (invalidPath) + throw new TypeError('Request path contains unescaped characters'); + } + + if (protocol !== expectedProtocol) { throw new Error('Protocol "' + protocol + '" not supported. ' + 'Expected "' + expectedProtocol + '"'); } @@ -63,30 +97,36 @@ function ClientRequest(options, cb) { var port = options.port = options.port || defaultPort || 80; var host = options.host = options.hostname || options.host || 'localhost'; - if (options.setHost === undefined) { - var setHost = true; - } + var setHost = (options.setHost === undefined); self.socketPath = options.socketPath; self.timeout = options.timeout; var method = options.method; - if (method != null && typeof method !== 'string') { + var methodIsString = (typeof method === 'string'); + if (method != null && !methodIsString) { throw new TypeError('Method must be a string'); } - method = self.method = (method || 'GET').toUpperCase(); - if (!common._checkIsHttpToken(method)) { - throw new TypeError('Method must be a valid HTTP token'); + + if (methodIsString && method) { + if (!common._checkIsHttpToken(method)) { + throw new TypeError('Method must be a valid HTTP token'); + } + method = self.method = method.toUpperCase(); + } else { + method = self.method = 'GET'; } + self.path = options.path || '/'; if (cb) { self.once('response', cb); } - if (!Array.isArray(options.headers)) { + var headersArray = Array.isArray(options.headers); + if (!headersArray) { if (options.headers) { var keys = Object.keys(options.headers); - for (var i = 0, l = keys.length; i < l; i++) { + for (var i = 0; i < keys.length; i++) { var key = keys[i]; self.setHeader(key, options.headers[key]); } @@ -112,7 +152,6 @@ function ClientRequest(options, cb) { } if (options.auth && !this.getHeader('Authorization')) { - //basic auth this.setHeader('Authorization', 'Basic ' + Buffer.from(options.auth).toString('base64')); } @@ -127,7 +166,7 @@ function ClientRequest(options, cb) { self.useChunkedEncodingByDefault = true; } - if (Array.isArray(options.headers)) { + if (headersArray) { self._storeHeader(self.method + ' ' + self.path + ' HTTP/1.1\r\n', options.headers); } else if (self.getHeader('expect')) { diff --git a/lib/_http_common.js b/lib/_http_common.js index f585d97d7b6536..b8724f00ec6557 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -265,8 +265,6 @@ var validTokens = [ 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 (val.length < 2) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 8520328683ea47..30011e091525a3 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -314,21 +314,21 @@ function _storeHeader(firstLine, headers) { if (state.expect) this._send(''); } -function storeHeader(self, state, field, value, validate) { +function storeHeader(self, state, key, value, validate) { if (validate) { - if (!checkIsHttpToken(field)) { + if (typeof key !== 'string' || !key || !checkIsHttpToken(key)) { throw new TypeError( - 'Header name must be a valid HTTP Token ["' + field + '"]'); + 'Header name must be a valid HTTP Token ["' + key + '"]'); } if (value === undefined) { - throw new Error('Header "%s" value must not be undefined', field); + throw new Error('Header "%s" value must not be undefined', key); } else if (checkInvalidHeaderChar(value)) { - debug('Header "%s" contains invalid characters', field); + debug('Header "%s" contains invalid characters', key); throw new TypeError('The header content contains invalid characters'); } } - state.header += field + ': ' + escapeHeaderValue(value) + CRLF; - matchHeader(self, state, field, value); + state.header += key + ': ' + escapeHeaderValue(value) + CRLF; + matchHeader(self, state, key, value); } function matchConnValue(self, state, value) { @@ -374,7 +374,7 @@ function matchHeader(self, state, field, value) { } function validateHeader(msg, name, value) { - if (!checkIsHttpToken(name)) + if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) throw new TypeError( 'Header name must be a valid HTTP Token ["' + name + '"]'); if (value === undefined) @@ -568,7 +568,7 @@ OutgoingMessage.prototype.addTrailers = function addTrailers(headers) { field = key; value = headers[key]; } - if (!checkIsHttpToken(field)) { + if (typeof field !== 'string' || !field || !checkIsHttpToken(field)) { throw new TypeError( 'Trailer name must be a valid HTTP Token ["' + field + '"]'); } diff --git a/test/parallel/test-http-client-defaults.js b/test/parallel/test-http-client-defaults.js new file mode 100644 index 00000000000000..d277a60e3df55e --- /dev/null +++ b/test/parallel/test-http-client-defaults.js @@ -0,0 +1,24 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const ClientRequest = require('http').ClientRequest; + +function noop() {} + +{ + const req = new ClientRequest({ createConnection: noop }); + assert.strictEqual(req.path, '/'); + assert.strictEqual(req.method, 'GET'); +} + +{ + const req = new ClientRequest({ method: '', createConnection: noop }); + assert.strictEqual(req.path, '/'); + assert.strictEqual(req.method, 'GET'); +} + +{ + const req = new ClientRequest({ path: '', createConnection: noop }); + assert.strictEqual(req.path, '/'); + assert.strictEqual(req.method, 'GET'); +}