Skip to content

Commit

Permalink
http,https: align server option of https with http
Browse files Browse the repository at this point in the history 
Fixes: #38954

PR-URL: #38992
Refs: #30570
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
@Ayase-252 @targos
Ayase-252 authored and targos committed Sep 4, 2021
1 parent 3a60de0 commit 8b3feee
Show file tree
Hide file tree
Showing 4 changed files with 268 additions and 18 deletions.
28 changes: 16 additions & 12 deletions lib/_http_server.js
View file
Expand Up @@ -337,18 +337,7 @@ function writeHead(statusCode, reason, obj) {
// Docs-only deprecated: DEP0063
ServerResponse.prototype.writeHeader = ServerResponse.prototype.writeHead;

function Server(options, requestListener) {
if (!(this instanceof Server)) return new Server(options, requestListener);

if (typeof options === 'function') {
requestListener = options;
options = {};
} else if (options == null || typeof options === 'object') {
options = { ...options };
} else {
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
}

function storeHTTPOptions(options) {
this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
this[kServerResponse] = options.ServerResponse || ServerResponse;

Expand All @@ -361,7 +350,21 @@ function Server(options, requestListener) {
if (insecureHTTPParser !== undefined)
validateBoolean(insecureHTTPParser, 'options.insecureHTTPParser');
this.insecureHTTPParser = insecureHTTPParser;
}

function Server(options, requestListener) {
if (!(this instanceof Server)) return new Server(options, requestListener);

if (typeof options === 'function') {
requestListener = options;
options = {};
} else if (options == null || typeof options === 'object') {
options = { ...options };
} else {
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
}

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

if (requestListener) {
Expand Down Expand Up @@ -963,6 +966,7 @@ module.exports = {
STATUS_CODES,
Server,
ServerResponse,
storeHTTPOptions,
_connectionListener: connectionListener,
kServerResponse
};
8 changes: 2 additions & 6 deletions lib/https.js
View file
Expand Up @@ -41,16 +41,14 @@ const url = require('url');
const { Agent: HttpAgent } = require('_http_agent');
const {
Server: HttpServer,
storeHTTPOptions,
_connectionListener,
kServerResponse
} = require('_http_server');
const { ClientRequest } = require('_http_client');
let debug = require('internal/util/debuglog').debuglog('https', (fn) => {
debug = fn;
});
const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
const { IncomingMessage, ServerResponse } = require('http');
const { kIncomingMessage } = require('_http_common');

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

this[kIncomingMessage] = opts.IncomingMessage || IncomingMessage;
this[kServerResponse] = opts.ServerResponse || ServerResponse;

FunctionPrototypeCall(storeHTTPOptions, this, opts);
FunctionPrototypeCall(tls.Server, this, opts, _connectionListener);

this.httpAllowHalfOpen = false;
Expand Down
131 changes: 131 additions & 0 deletions test/parallel/test-https-insecure-parse-per-stream.js
View file
@@ -0,0 +1,131 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto) {
common.skip('missing crypto');
}

const fixtures = require('../common/fixtures');
const assert = require('assert');
const https = require('https');
const MakeDuplexPair = require('../common/duplexpair');
const tls = require('tls');
const { finished } = require('stream');

const certFixture = {
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem'),
ca: fixtures.readKey('ca1-cert.pem'),
};


// Test that setting the `insecureHTTPParse` option works on a per-stream-basis.

// Test 1: The server sends an invalid header.
{
const { clientSide, serverSide } = MakeDuplexPair();

const req = https.request({
rejectUnauthorized: false,
createConnection: common.mustCall(() => clientSide),
insecureHTTPParser: true
}, common.mustCall((res) => {
assert.strictEqual(res.headers.hello, 'foo\x08foo');
res.resume(); // We don’t actually care about contents.
res.on('end', common.mustCall());
}));
req.end();

serverSide.resume(); // Dump the request
serverSide.end('HTTP/1.1 200 OK\r\n' +
'Hello: foo\x08foo\r\n' +
'Content-Length: 0\r\n' +
'\r\n\r\n');
}

// Test 2: The same as Test 1 except without the option, to make sure it fails.
{
const { clientSide, serverSide } = MakeDuplexPair();

const req = https.request({
rejectUnauthorized: false,
createConnection: common.mustCall(() => clientSide)
}, common.mustNotCall());
req.end();
req.on('error', common.mustCall());

serverSide.resume(); // Dump the request
serverSide.end('HTTP/1.1 200 OK\r\n' +
'Hello: foo\x08foo\r\n' +
'Content-Length: 0\r\n' +
'\r\n\r\n');
}

// Test 3: The client sends an invalid header.
{
const testData = 'Hello, World!\n';
const server = https.createServer(
{ insecureHTTPParser: true,
...certFixture },
common.mustCall((req, res) => {
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
res.end(testData);
}));

server.on('clientError', common.mustNotCall());

server.listen(0, common.mustCall(() => {
const client = tls.connect({
port: server.address().port,
rejectUnauthorized: false
});
client.write(
'GET / HTTP/1.1\r\n' +
'Hello: foo\x08foo\r\n' +
'\r\n\r\n');
client.end();

client.on('data', () => {});
finished(client, common.mustCall(() => {
server.close();
}));
}));
}

// Test 4: The same as Test 3 except without the option, to make sure it fails.
{
const server = https.createServer(
{ ...certFixture },
common.mustNotCall());

server.on('clientError', common.mustCall());

server.listen(0, common.mustCall(() => {
const client = tls.connect({
port: server.address().port,
rejectUnauthorized: false
});
client.write(
'GET / HTTP/1.1\r\n' +
'Hello: foo\x08foo\r\n' +
'\r\n\r\n');
client.end();

client.on('data', () => {});
finished(client, common.mustCall(() => {
server.close();
}));
}));
}

// Test 5: Invalid argument type
{
assert.throws(
() => https.request({ insecureHTTPParser: 0 }, common.mustNotCall()),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "options.insecureHTTPParser" property must be of' +
' type boolean. Received type number (0)'
})
);
}
119 changes: 119 additions & 0 deletions test/parallel/test-https-max-header-size-per-stream.js
View file
@@ -0,0 +1,119 @@
'use strict';
const common = require('../common');

if (!common.hasCrypto) {
common.skip('missing crypto');
}

const fixtures = require('../common/fixtures');
const assert = require('assert');
const https = require('https');
const http = require('http');
const tls = require('tls');
const MakeDuplexPair = require('../common/duplexpair');
const { finished } = require('stream');

const certFixture = {
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem'),
ca: fixtures.readKey('ca1-cert.pem'),
};


// Test that setting the `maxHeaderSize` option works on a per-stream-basis.

// Test 1: The server sends larger headers than what would otherwise be allowed.
{
const { clientSide, serverSide } = MakeDuplexPair();

const req = https.request({
createConnection: common.mustCall(() => clientSide),
maxHeaderSize: http.maxHeaderSize * 4
}, common.mustCall((res) => {
assert.strictEqual(res.headers.hello, 'A'.repeat(http.maxHeaderSize * 3));
res.resume(); // We don’t actually care about contents.
res.on('end', common.mustCall());
}));
req.end();

serverSide.resume(); // Dump the request
serverSide.end('HTTP/1.1 200 OK\r\n' +
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
'Content-Length: 0\r\n' +
'\r\n\r\n');
}

// Test 2: The same as Test 1 except without the option, to make sure it fails.
{
const { clientSide, serverSide } = MakeDuplexPair();

const req = https.request({
createConnection: common.mustCall(() => clientSide)
}, common.mustNotCall());
req.end();
req.on('error', common.mustCall());

serverSide.resume(); // Dump the request
serverSide.end('HTTP/1.1 200 OK\r\n' +
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
'Content-Length: 0\r\n' +
'\r\n\r\n');
}

// Test 3: The client sends larger headers than what would otherwise be allowed.
{
const testData = 'Hello, World!\n';
const server = https.createServer(
{ maxHeaderSize: http.maxHeaderSize * 4,
...certFixture },
common.mustCall((req, res) => {
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
res.end(testData);
}));

server.on('clientError', common.mustNotCall());

server.listen(0, common.mustCall(() => {
const client = tls.connect({
port: server.address().port,
rejectUnauthorized: false
});
client.write(
'GET / HTTP/1.1\r\n' +
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
'\r\n\r\n');
client.end();

client.on('data', () => {});
finished(client, common.mustCall(() => {
server.close();
}));
}));
}

// Test 4: The same as Test 3 except without the option, to make sure it fails.
{
const server = https.createServer({ ...certFixture }, common.mustNotCall());

// clientError may be emitted multiple times when header is larger than
// maxHeaderSize.
server.on('clientError', common.mustCallAtLeast(() => {}, 1));

server.listen(0, common.mustCall(() => {
const client = tls.connect({
port: server.address().port,
rejectUnauthorized: false
});
client.write(
'GET / HTTP/1.1\r\n' +
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
'\r\n\r\n');
client.end();

client.on('data', () => {});
finished(client, common.mustCall(() => {
server.close();
}));
}));
}

0 comments on commit 8b3feee

Please sign in to comment.