Skip to content

Commit 8b3feee

Browse files
Ayase-252targos
Ayase-252
authored and
targos
committed
http,https: align server option of https with http
Fixes: #38954 PR-URL: #38992 Refs: #30570 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 3a60de0 commit 8b3feee

File tree

4 files changed

+268
-18
lines changed

4 files changed

+268
-18
lines changed

‎lib/_http_server.js‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -337,18 +337,7 @@ function writeHead(statusCode, reason, obj) {
337337
// Docs-only deprecated: DEP0063
338338
ServerResponse.prototype.writeHeader = ServerResponse.prototype.writeHead;
339339

340-
function Server(options, requestListener) {
341-
if (!(this instanceof Server)) return new Server(options, requestListener);
342-
343-
if (typeof options === 'function') {
344-
requestListener = options;
345-
options = {};
346-
} else if (options == null || typeof options === 'object') {
347-
options = { ...options };
348-
} else {
349-
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
350-
}
351-
340+
function storeHTTPOptions(options) {
352341
this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
353342
this[kServerResponse] = options.ServerResponse || ServerResponse;
354343

@@ -361,7 +350,21 @@ function Server(options, requestListener) {
361350
if (insecureHTTPParser !== undefined)
362351
validateBoolean(insecureHTTPParser, 'options.insecureHTTPParser');
363352
this.insecureHTTPParser = insecureHTTPParser;
353+
}
354+
355+
function Server(options, requestListener) {
356+
if (!(this instanceof Server)) return new Server(options, requestListener);
357+
358+
if (typeof options === 'function') {
359+
requestListener = options;
360+
options = {};
361+
} else if (options == null || typeof options === 'object') {
362+
options = { ...options };
363+
} else {
364+
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
365+
}
364366

367+
storeHTTPOptions.call(this, options);
365368
net.Server.call(this, { allowHalfOpen: true });
366369

367370
if (requestListener) {
@@ -963,6 +966,7 @@ module.exports = {
963966
STATUS_CODES,
964967
Server,
965968
ServerResponse,
969+
storeHTTPOptions,
966970
_connectionListener: connectionListener,
967971
kServerResponse
968972
};

‎lib/https.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,14 @@ const url = require('url');
4141
const { Agent: HttpAgent } = require('_http_agent');
4242
const {
4343
Server: HttpServer,
44+
storeHTTPOptions,
4445
_connectionListener,
45-
kServerResponse
4646
} = require('_http_server');
4747
const { ClientRequest } = require('_http_client');
4848
let debug = require('internal/util/debuglog').debuglog('https', (fn) => {
4949
debug = fn;
5050
});
5151
const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
52-
const { IncomingMessage, ServerResponse } = require('http');
53-
const { kIncomingMessage } = require('_http_common');
5452

5553
function Server(opts, requestListener) {
5654
if (!(this instanceof Server)) return new Server(opts, requestListener);
@@ -68,9 +66,7 @@ function Server(opts, requestListener) {
6866
opts.ALPNProtocols = ['http/1.1'];
6967
}
7068

71-
this[kIncomingMessage] = opts.IncomingMessage || IncomingMessage;
72-
this[kServerResponse] = opts.ServerResponse || ServerResponse;
73-
69+
FunctionPrototypeCall(storeHTTPOptions, this, opts);
7470
FunctionPrototypeCall(tls.Server, this, opts, _connectionListener);
7571

7672
this.httpAllowHalfOpen = false;

‎test/parallel/test-https-insecure-parse-per-stream.js‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto) {
4+
common.skip('missing crypto');
5+
}
6+
7+
const fixtures = require('../common/fixtures');
8+
const assert = require('assert');
9+
const https = require('https');
10+
const MakeDuplexPair = require('../common/duplexpair');
11+
const tls = require('tls');
12+
const { finished } = require('stream');
13+
14+
const certFixture = {
15+
key: fixtures.readKey('agent1-key.pem'),
16+
cert: fixtures.readKey('agent1-cert.pem'),
17+
ca: fixtures.readKey('ca1-cert.pem'),
18+
};
19+
20+
21+
// Test that setting the `insecureHTTPParse` option works on a per-stream-basis.
22+
23+
// Test 1: The server sends an invalid header.
24+
{
25+
const { clientSide, serverSide } = MakeDuplexPair();
26+
27+
const req = https.request({
28+
rejectUnauthorized: false,
29+
createConnection: common.mustCall(() => clientSide),
30+
insecureHTTPParser: true
31+
}, common.mustCall((res) => {
32+
assert.strictEqual(res.headers.hello, 'foo\x08foo');
33+
res.resume(); // We don’t actually care about contents.
34+
res.on('end', common.mustCall());
35+
}));
36+
req.end();
37+
38+
serverSide.resume(); // Dump the request
39+
serverSide.end('HTTP/1.1 200 OK\r\n' +
40+
'Hello: foo\x08foo\r\n' +
41+
'Content-Length: 0\r\n' +
42+
'\r\n\r\n');
43+
}
44+
45+
// Test 2: The same as Test 1 except without the option, to make sure it fails.
46+
{
47+
const { clientSide, serverSide } = MakeDuplexPair();
48+
49+
const req = https.request({
50+
rejectUnauthorized: false,
51+
createConnection: common.mustCall(() => clientSide)
52+
}, common.mustNotCall());
53+
req.end();
54+
req.on('error', common.mustCall());
55+
56+
serverSide.resume(); // Dump the request
57+
serverSide.end('HTTP/1.1 200 OK\r\n' +
58+
'Hello: foo\x08foo\r\n' +
59+
'Content-Length: 0\r\n' +
60+
'\r\n\r\n');
61+
}
62+
63+
// Test 3: The client sends an invalid header.
64+
{
65+
const testData = 'Hello, World!\n';
66+
const server = https.createServer(
67+
{ insecureHTTPParser: true,
68+
...certFixture },
69+
common.mustCall((req, res) => {
70+
res.statusCode = 200;
71+
res.setHeader('Content-Type', 'text/plain');
72+
res.end(testData);
73+
}));
74+
75+
server.on('clientError', common.mustNotCall());
76+
77+
server.listen(0, common.mustCall(() => {
78+
const client = tls.connect({
79+
port: server.address().port,
80+
rejectUnauthorized: false
81+
});
82+
client.write(
83+
'GET / HTTP/1.1\r\n' +
84+
'Hello: foo\x08foo\r\n' +
85+
'\r\n\r\n');
86+
client.end();
87+
88+
client.on('data', () => {});
89+
finished(client, common.mustCall(() => {
90+
server.close();
91+
}));
92+
}));
93+
}
94+
95+
// Test 4: The same as Test 3 except without the option, to make sure it fails.
96+
{
97+
const server = https.createServer(
98+
{ ...certFixture },
99+
common.mustNotCall());
100+
101+
server.on('clientError', common.mustCall());
102+
103+
server.listen(0, common.mustCall(() => {
104+
const client = tls.connect({
105+
port: server.address().port,
106+
rejectUnauthorized: false
107+
});
108+
client.write(
109+
'GET / HTTP/1.1\r\n' +
110+
'Hello: foo\x08foo\r\n' +
111+
'\r\n\r\n');
112+
client.end();
113+
114+
client.on('data', () => {});
115+
finished(client, common.mustCall(() => {
116+
server.close();
117+
}));
118+
}));
119+
}
120+
121+
// Test 5: Invalid argument type
122+
{
123+
assert.throws(
124+
() => https.request({ insecureHTTPParser: 0 }, common.mustNotCall()),
125+
common.expectsError({
126+
code: 'ERR_INVALID_ARG_TYPE',
127+
message: 'The "options.insecureHTTPParser" property must be of' +
128+
' type boolean. Received type number (0)'
129+
})
130+
);
131+
}

‎test/parallel/test-https-max-header-size-per-stream.js‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
if (!common.hasCrypto) {
5+
common.skip('missing crypto');
6+
}
7+
8+
const fixtures = require('../common/fixtures');
9+
const assert = require('assert');
10+
const https = require('https');
11+
const http = require('http');
12+
const tls = require('tls');
13+
const MakeDuplexPair = require('../common/duplexpair');
14+
const { finished } = require('stream');
15+
16+
const certFixture = {
17+
key: fixtures.readKey('agent1-key.pem'),
18+
cert: fixtures.readKey('agent1-cert.pem'),
19+
ca: fixtures.readKey('ca1-cert.pem'),
20+
};
21+
22+
23+
// Test that setting the `maxHeaderSize` option works on a per-stream-basis.
24+
25+
// Test 1: The server sends larger headers than what would otherwise be allowed.
26+
{
27+
const { clientSide, serverSide } = MakeDuplexPair();
28+
29+
const req = https.request({
30+
createConnection: common.mustCall(() => clientSide),
31+
maxHeaderSize: http.maxHeaderSize * 4
32+
}, common.mustCall((res) => {
33+
assert.strictEqual(res.headers.hello, 'A'.repeat(http.maxHeaderSize * 3));
34+
res.resume(); // We don’t actually care about contents.
35+
res.on('end', common.mustCall());
36+
}));
37+
req.end();
38+
39+
serverSide.resume(); // Dump the request
40+
serverSide.end('HTTP/1.1 200 OK\r\n' +
41+
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
42+
'Content-Length: 0\r\n' +
43+
'\r\n\r\n');
44+
}
45+
46+
// Test 2: The same as Test 1 except without the option, to make sure it fails.
47+
{
48+
const { clientSide, serverSide } = MakeDuplexPair();
49+
50+
const req = https.request({
51+
createConnection: common.mustCall(() => clientSide)
52+
}, common.mustNotCall());
53+
req.end();
54+
req.on('error', common.mustCall());
55+
56+
serverSide.resume(); // Dump the request
57+
serverSide.end('HTTP/1.1 200 OK\r\n' +
58+
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
59+
'Content-Length: 0\r\n' +
60+
'\r\n\r\n');
61+
}
62+
63+
// Test 3: The client sends larger headers than what would otherwise be allowed.
64+
{
65+
const testData = 'Hello, World!\n';
66+
const server = https.createServer(
67+
{ maxHeaderSize: http.maxHeaderSize * 4,
68+
...certFixture },
69+
common.mustCall((req, res) => {
70+
res.statusCode = 200;
71+
res.setHeader('Content-Type', 'text/plain');
72+
res.end(testData);
73+
}));
74+
75+
server.on('clientError', common.mustNotCall());
76+
77+
server.listen(0, common.mustCall(() => {
78+
const client = tls.connect({
79+
port: server.address().port,
80+
rejectUnauthorized: false
81+
});
82+
client.write(
83+
'GET / HTTP/1.1\r\n' +
84+
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
85+
'\r\n\r\n');
86+
client.end();
87+
88+
client.on('data', () => {});
89+
finished(client, common.mustCall(() => {
90+
server.close();
91+
}));
92+
}));
93+
}
94+
95+
// Test 4: The same as Test 3 except without the option, to make sure it fails.
96+
{
97+
const server = https.createServer({ ...certFixture }, common.mustNotCall());
98+
99+
// clientError may be emitted multiple times when header is larger than
100+
// maxHeaderSize.
101+
server.on('clientError', common.mustCallAtLeast(() => {}, 1));
102+
103+
server.listen(0, common.mustCall(() => {
104+
const client = tls.connect({
105+
port: server.address().port,
106+
rejectUnauthorized: false
107+
});
108+
client.write(
109+
'GET / HTTP/1.1\r\n' +
110+
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
111+
'\r\n\r\n');
112+
client.end();
113+
114+
client.on('data', () => {});
115+
finished(client, common.mustCall(() => {
116+
server.close();
117+
}));
118+
}));
119+
}

0 commit comments

Comments
 (0)