Skip to content

Commit 35331cb

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 <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
1 parent ea8d83b commit 35331cb

File tree

4 files changed

+268
-18
lines changed

4 files changed

+268
-18
lines changed

Diff for: ‎lib/_http_server.js

+16-12
Original file line numberDiff line numberDiff line change
@@ -353,18 +353,7 @@ function writeHead(statusCode, reason, obj) {
353353
// Docs-only deprecated: DEP0063
354354
ServerResponse.prototype.writeHeader = ServerResponse.prototype.writeHead;
355355

356-
function Server(options, requestListener) {
357-
if (!(this instanceof Server)) return new Server(options, requestListener);
358-
359-
if (typeof options === 'function') {
360-
requestListener = options;
361-
options = {};
362-
} else if (options == null || typeof options === 'object') {
363-
options = { ...options };
364-
} else {
365-
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
366-
}
367-
356+
function storeHTTPOptions(options) {
368357
this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
369358
this[kServerResponse] = options.ServerResponse || ServerResponse;
370359

@@ -377,7 +366,21 @@ function Server(options, requestListener) {
377366
if (insecureHTTPParser !== undefined)
378367
validateBoolean(insecureHTTPParser, 'options.insecureHTTPParser');
379368
this.insecureHTTPParser = insecureHTTPParser;
369+
}
370+
371+
function Server(options, requestListener) {
372+
if (!(this instanceof Server)) return new Server(options, requestListener);
373+
374+
if (typeof options === 'function') {
375+
requestListener = options;
376+
options = {};
377+
} else if (options == null || typeof options === 'object') {
378+
options = { ...options };
379+
} else {
380+
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
381+
}
380382

383+
storeHTTPOptions.call(this, options);
381384
net.Server.call(this, { allowHalfOpen: true });
382385

383386
if (requestListener) {
@@ -991,6 +994,7 @@ module.exports = {
991994
STATUS_CODES,
992995
Server,
993996
ServerResponse,
997+
storeHTTPOptions,
994998
_connectionListener: connectionListener,
995999
kServerResponse
9961000
};

Diff for: ‎lib/https.js

+2-6
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,14 @@ const tls = require('tls');
4040
const { Agent: HttpAgent } = require('_http_agent');
4141
const {
4242
Server: HttpServer,
43+
storeHTTPOptions,
4344
_connectionListener,
44-
kServerResponse
4545
} = require('_http_server');
4646
const { ClientRequest } = require('_http_client');
4747
let debug = require('internal/util/debuglog').debuglog('https', (fn) => {
4848
debug = fn;
4949
});
5050
const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
51-
const { IncomingMessage, ServerResponse } = require('http');
52-
const { kIncomingMessage } = require('_http_common');
5351

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

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

7571
this.httpAllowHalfOpen = false;

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

+131
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+
}

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

+119
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)