From 7e5035acb178a48370645e3588a0392e1bf777b8 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Tue, 24 Oct 2017 14:11:57 +1100 Subject: [PATCH] http: process headers after setting up agent Added tests to clarify the implicit behaviour of array header setting vs object header setting PR-URL: https://github.com/nodejs/node/pull/16568 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- lib/_http_client.js | 91 ++++++++++--------- .../test-http-client-headers-array.js | 60 ++++++++++++ 2 files changed, 108 insertions(+), 43 deletions(-) create mode 100644 test/parallel/test-http-client-headers-array.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 2287c2751b8ce6..9dea39d0216c80 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -164,6 +164,40 @@ function ClientRequest(options, cb) { this.once('response', cb); } + if (method === 'GET' || + method === 'HEAD' || + method === 'DELETE' || + method === 'OPTIONS' || + method === 'CONNECT') { + this.useChunkedEncodingByDefault = false; + } else { + this.useChunkedEncodingByDefault = true; + } + + this._ended = false; + this.res = null; + this.aborted = undefined; + this.timeoutCb = null; + this.upgradeOrConnect = false; + this.parser = null; + this.maxHeadersCount = null; + + var called = false; + + if (this.agent) { + // If there is an agent we should default to Connection:keep-alive, + // but only if the Agent will actually reuse the connection! + // If it's not a keepAlive agent, and the maxSockets==Infinity, then + // there's never a case where this socket will actually be reused + if (!this.agent.keepAlive && !Number.isFinite(this.agent.maxSockets)) { + this._last = true; + this.shouldKeepAlive = false; + } else { + this._last = false; + this.shouldKeepAlive = true; + } + } + var headersArray = Array.isArray(options.headers); if (!headersArray) { if (options.headers) { @@ -173,6 +207,7 @@ function ClientRequest(options, cb) { this.setHeader(key, options.headers[key]); } } + if (host && !this.getHeader('host') && setHost) { var hostHeader = host; @@ -191,45 +226,25 @@ function ClientRequest(options, cb) { } this.setHeader('Host', hostHeader); } - } - if (options.auth && !this.getHeader('Authorization')) { - this.setHeader('Authorization', 'Basic ' + - Buffer.from(options.auth).toString('base64')); - } + if (options.auth && !this.getHeader('Authorization')) { + this.setHeader('Authorization', 'Basic ' + + Buffer.from(options.auth).toString('base64')); + } - if (method === 'GET' || - method === 'HEAD' || - method === 'DELETE' || - method === 'OPTIONS' || - method === 'CONNECT') { - this.useChunkedEncodingByDefault = false; - } else { - this.useChunkedEncodingByDefault = true; - } + if (this.getHeader('expect')) { + if (this._header) { + throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render'); + } - if (headersArray) { - this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n', - options.headers); - } else if (this.getHeader('expect')) { - if (this._header) { - throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render'); + this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n', + this[outHeadersKey]); } - + } else { this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n', - this[outHeadersKey]); + options.headers); } - this._ended = false; - this.res = null; - this.aborted = undefined; - this.timeoutCb = null; - this.upgradeOrConnect = false; - this.parser = null; - this.maxHeadersCount = null; - - var called = false; - var oncreate = (err, socket) => { if (called) return; @@ -242,18 +257,8 @@ function ClientRequest(options, cb) { this._deferToConnect(null, null, () => this._flush()); }; + // initiate connection if (this.agent) { - // If there is an agent we should default to Connection:keep-alive, - // but only if the Agent will actually reuse the connection! - // If it's not a keepAlive agent, and the maxSockets==Infinity, then - // there's never a case where this socket will actually be reused - if (!this.agent.keepAlive && !Number.isFinite(this.agent.maxSockets)) { - this._last = true; - this.shouldKeepAlive = false; - } else { - this._last = false; - this.shouldKeepAlive = true; - } this.agent.addRequest(this, options); } else { // No agent, default to Connection:close. diff --git a/test/parallel/test-http-client-headers-array.js b/test/parallel/test-http-client-headers-array.js new file mode 100644 index 00000000000000..dffe04bb108401 --- /dev/null +++ b/test/parallel/test-http-client-headers-array.js @@ -0,0 +1,60 @@ +'use strict'; + +require('../common'); + +const assert = require('assert'); +const http = require('http'); + +function execute(options) { + http.createServer(function(req, res) { + const expectHeaders = { + 'x-foo': 'boom', + cookie: 'a=1; b=2; c=3', + connection: 'close' + }; + + // no Host header when you set headers an array + if (!Array.isArray(options.headers)) { + expectHeaders.host = `localhost:${this.address().port}`; + } + + // no Authorization header when you set headers an array + if (options.auth && !Array.isArray(options.headers)) { + expectHeaders.authorization = + `Basic ${Buffer.from(options.auth).toString('base64')}`; + } + + this.close(); + + assert.deepStrictEqual(req.headers, expectHeaders); + + res.end(); + }).listen(0, function() { + options = Object.assign(options, { + port: this.address().port, + path: '/' + }); + const req = http.request(options); + req.end(); + }); +} + +// should be the same except for implicit Host header on the first two +execute({ headers: { 'x-foo': 'boom', 'cookie': 'a=1; b=2; c=3' } }); +execute({ headers: { 'x-foo': 'boom', 'cookie': [ 'a=1', 'b=2', 'c=3' ] } }); +execute({ headers: [[ 'x-foo', 'boom' ], [ 'cookie', 'a=1; b=2; c=3' ]] }); +execute({ headers: [ + [ 'x-foo', 'boom' ], [ 'cookie', [ 'a=1', 'b=2', 'c=3' ]] +] }); +execute({ headers: [ + [ 'x-foo', 'boom' ], [ 'cookie', 'a=1' ], + [ 'cookie', 'b=2' ], [ 'cookie', 'c=3'] +] }); + +// Authorization and Host header both missing from the second +execute({ auth: 'foo:bar', headers: + { 'x-foo': 'boom', 'cookie': 'a=1; b=2; c=3' } }); +execute({ auth: 'foo:bar', headers: [ + [ 'x-foo', 'boom' ], [ 'cookie', 'a=1' ], + [ 'cookie', 'b=2' ], [ 'cookie', 'c=3'] +] });