Skip to content

Commit 4404c76

Browse files
rvaggBridgeAR
rvagg
authored and
BridgeAR
committed
http: process headers after setting up agent
Added tests to clarify the implicit behaviour of array header setting vs object header setting PR-URL: #16568 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 7d4b772 commit 4404c76

2 files changed

Lines changed: 108 additions & 43 deletions

File tree

‎lib/_http_client.js‎

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,40 @@ function ClientRequest(options, cb) {
132132
this.once('response', cb);
133133
}
134134

135+
if (method === 'GET' ||
136+
method === 'HEAD' ||
137+
method === 'DELETE' ||
138+
method === 'OPTIONS' ||
139+
method === 'CONNECT') {
140+
this.useChunkedEncodingByDefault = false;
141+
} else {
142+
this.useChunkedEncodingByDefault = true;
143+
}
144+
145+
this._ended = false;
146+
this.res = null;
147+
this.aborted = undefined;
148+
this.timeoutCb = null;
149+
this.upgradeOrConnect = false;
150+
this.parser = null;
151+
this.maxHeadersCount = null;
152+
153+
var called = false;
154+
155+
if (this.agent) {
156+
// If there is an agent we should default to Connection:keep-alive,
157+
// but only if the Agent will actually reuse the connection!
158+
// If it's not a keepAlive agent, and the maxSockets==Infinity, then
159+
// there's never a case where this socket will actually be reused
160+
if (!this.agent.keepAlive && !Number.isFinite(this.agent.maxSockets)) {
161+
this._last = true;
162+
this.shouldKeepAlive = false;
163+
} else {
164+
this._last = false;
165+
this.shouldKeepAlive = true;
166+
}
167+
}
168+
135169
var headersArray = Array.isArray(options.headers);
136170
if (!headersArray) {
137171
if (options.headers) {
@@ -141,6 +175,7 @@ function ClientRequest(options, cb) {
141175
this.setHeader(key, options.headers[key]);
142176
}
143177
}
178+
144179
if (host && !this.getHeader('host') && setHost) {
145180
var hostHeader = host;
146181

@@ -159,45 +194,25 @@ function ClientRequest(options, cb) {
159194
}
160195
this.setHeader('Host', hostHeader);
161196
}
162-
}
163197

164-
if (options.auth && !this.getHeader('Authorization')) {
165-
this.setHeader('Authorization', 'Basic ' +
166-
Buffer.from(options.auth).toString('base64'));
167-
}
198+
if (options.auth && !this.getHeader('Authorization')) {
199+
this.setHeader('Authorization', 'Basic ' +
200+
Buffer.from(options.auth).toString('base64'));
201+
}
168202

169-
if (method === 'GET' ||
170-
method === 'HEAD' ||
171-
method === 'DELETE' ||
172-
method === 'OPTIONS' ||
173-
method === 'CONNECT') {
174-
this.useChunkedEncodingByDefault = false;
175-
} else {
176-
this.useChunkedEncodingByDefault = true;
177-
}
203+
if (this.getHeader('expect')) {
204+
if (this._header) {
205+
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render');
206+
}
178207

179-
if (headersArray) {
180-
this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
181-
options.headers);
182-
} else if (this.getHeader('expect')) {
183-
if (this._header) {
184-
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render');
208+
this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
209+
this[outHeadersKey]);
185210
}
186-
211+
} else {
187212
this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
188-
this[outHeadersKey]);
213+
options.headers);
189214
}
190215

191-
this._ended = false;
192-
this.res = null;
193-
this.aborted = undefined;
194-
this.timeoutCb = null;
195-
this.upgradeOrConnect = false;
196-
this.parser = null;
197-
this.maxHeadersCount = null;
198-
199-
var called = false;
200-
201216
var oncreate = (err, socket) => {
202217
if (called)
203218
return;
@@ -210,18 +225,8 @@ function ClientRequest(options, cb) {
210225
this._deferToConnect(null, null, () => this._flush());
211226
};
212227

228+
// initiate connection
213229
if (this.agent) {
214-
// If there is an agent we should default to Connection:keep-alive,
215-
// but only if the Agent will actually reuse the connection!
216-
// If it's not a keepAlive agent, and the maxSockets==Infinity, then
217-
// there's never a case where this socket will actually be reused
218-
if (!this.agent.keepAlive && !Number.isFinite(this.agent.maxSockets)) {
219-
this._last = true;
220-
this.shouldKeepAlive = false;
221-
} else {
222-
this._last = false;
223-
this.shouldKeepAlive = true;
224-
}
225230
this.agent.addRequest(this, options);
226231
} else {
227232
// No agent, default to Connection:close.

‎test/parallel/test-http-client-headers-array.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
'use strict';
2+
3+
require('../common');
4+
5+
const assert = require('assert');
6+
const http = require('http');
7+
8+
function execute(options) {
9+
http.createServer(function(req, res) {
10+
const expectHeaders = {
11+
'x-foo': 'boom',
12+
cookie: 'a=1; b=2; c=3',
13+
connection: 'close'
14+
};
15+
16+
// no Host header when you set headers an array
17+
if (!Array.isArray(options.headers)) {
18+
expectHeaders.host = `localhost:${this.address().port}`;
19+
}
20+
21+
// no Authorization header when you set headers an array
22+
if (options.auth && !Array.isArray(options.headers)) {
23+
expectHeaders.authorization =
24+
`Basic ${Buffer.from(options.auth).toString('base64')}`;
25+
}
26+
27+
this.close();
28+
29+
assert.deepStrictEqual(req.headers, expectHeaders);
30+
31+
res.end();
32+
}).listen(0, function() {
33+
options = Object.assign(options, {
34+
port: this.address().port,
35+
path: '/'
36+
});
37+
const req = http.request(options);
38+
req.end();
39+
});
40+
}
41+
42+
// should be the same except for implicit Host header on the first two
43+
execute({ headers: { 'x-foo': 'boom', 'cookie': 'a=1; b=2; c=3' } });
44+
execute({ headers: { 'x-foo': 'boom', 'cookie': [ 'a=1', 'b=2', 'c=3' ] } });
45+
execute({ headers: [[ 'x-foo', 'boom' ], [ 'cookie', 'a=1; b=2; c=3' ]] });
46+
execute({ headers: [
47+
[ 'x-foo', 'boom' ], [ 'cookie', [ 'a=1', 'b=2', 'c=3' ]]
48+
] });
49+
execute({ headers: [
50+
[ 'x-foo', 'boom' ], [ 'cookie', 'a=1' ],
51+
[ 'cookie', 'b=2' ], [ 'cookie', 'c=3']
52+
] });
53+
54+
// Authorization and Host header both missing from the second
55+
execute({ auth: 'foo:bar', headers:
56+
{ 'x-foo': 'boom', 'cookie': 'a=1; b=2; c=3' } });
57+
execute({ auth: 'foo:bar', headers: [
58+
[ 'x-foo', 'boom' ], [ 'cookie', 'a=1' ],
59+
[ 'cookie', 'b=2' ], [ 'cookie', 'c=3']
60+
] });

0 commit comments

Comments
 (0)