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

‎lib/_http_server.js‎

Lines changed: 16 additions & 12 deletions
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-
functionServer(options,requestListener){
357-
if(!(thisinstanceofServer))returnnewServer(options,requestListener);
358-
359-
if(typeofoptions==='function'){
360-
requestListener=options;
361-
options={};
362-
}elseif(options==null||typeofoptions==='object'){
363-
options={ ...options};
364-
}else{
365-
thrownewERR_INVALID_ARG_TYPE('options','object',options);
366-
}
367-
356+
functionstoreHTTPOptions(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+
functionServer(options,requestListener){
372+
if(!(thisinstanceofServer))returnnewServer(options,requestListener);
373+
374+
if(typeofoptions==='function'){
375+
requestListener=options;
376+
options={};
377+
}elseif(options==null||typeofoptions==='object'){
378+
options={ ...options};
379+
}else{
380+
thrownewERR_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
};

‎lib/https.js‎

Lines changed: 2 additions & 6 deletions
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
letdebug=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
functionServer(opts,requestListener){
5553
if(!(thisinstanceofServer))returnnewServer(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;

‎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+
constcommon=require('../common');
3+
if(!common.hasCrypto){
4+
common.skip('missing crypto');
5+
}
6+
7+
constfixtures=require('../common/fixtures');
8+
constassert=require('assert');
9+
consthttps=require('https');
10+
constMakeDuplexPair=require('../common/duplexpair');
11+
consttls=require('tls');
12+
const{ finished }=require('stream');
13+
14+
constcertFixture={
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+
constreq=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+
constreq=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+
consttestData='Hello, World!\n';
66+
constserver=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+
constclient=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+
constserver=https.createServer(
98+
{ ...certFixture},
99+
common.mustNotCall());
100+
101+
server.on('clientError',common.mustCall());
102+
103+
server.listen(0,common.mustCall(()=>{
104+
constclient=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+
constcommon=require('../common');
3+
4+
if(!common.hasCrypto){
5+
common.skip('missing crypto');
6+
}
7+
8+
constfixtures=require('../common/fixtures');
9+
constassert=require('assert');
10+
consthttps=require('https');
11+
consthttp=require('http');
12+
consttls=require('tls');
13+
constMakeDuplexPair=require('../common/duplexpair');
14+
const{ finished }=require('stream');
15+
16+
constcertFixture={
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+
constreq=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+
constreq=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+
consttestData='Hello, World!\n';
66+
constserver=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+
constclient=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+
constserver=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+
constclient=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)