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 <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
Rod Vagg 2017-10-24 14:11:57 +11:00 committed by Ruben Bridgewater
parent 7d4b7724ec
commit 4404c7619b
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
2 changed files with 109 additions and 44 deletions

View File

@ -132,6 +132,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) {
@ -141,6 +175,7 @@ function ClientRequest(options, cb) {
this.setHeader(key, options.headers[key]);
}
}
if (host && !this.getHeader('host') && setHost) {
var hostHeader = host;
@ -159,45 +194,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 (method === 'GET' ||
method === 'HEAD' ||
method === 'DELETE' ||
method === 'OPTIONS' ||
method === 'CONNECT') {
this.useChunkedEncodingByDefault = false;
} else {
this.useChunkedEncodingByDefault = true;
}
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');
if (options.auth && !this.getHeader('Authorization')) {
this.setHeader('Authorization', 'Basic ' +
Buffer.from(options.auth).toString('base64'));
}
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;
@ -210,18 +225,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.

View File

@ -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']
] });