Skip to content

Commit

Permalink
tls: add highWaterMark option for connect
Browse files Browse the repository at this point in the history
PR-URL: #32786
Fixes: #32781
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Andrey Pechkurov <[email protected]>
  • Loading branch information
rickyes authored and BridgeAR committed Apr 28, 2020
1 parent 3956ab5 commit efb3c71
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 2 deletions.
6 changes: 5 additions & 1 deletion doc/api/https.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ Global instance of [`https.Agent`][] for all HTTPS client requests.
<!-- YAML
added: v0.3.6
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/32786
description: The `highWaterMark` option is accepted now.
- version: v10.9.0
pr-url: https://github.com/nodejs/node/pull/21616
description: The `url` parameter can now be passed along with a separate
Expand All @@ -263,7 +266,8 @@ Makes a request to a secure web server.
The following additional `options` from [`tls.connect()`][] are also accepted:
`ca`, `cert`, `ciphers`, `clientCertEngine`, `crl`, `dhparam`, `ecdhCurve`,
`honorCipherOrder`, `key`, `passphrase`, `pfx`, `rejectUnauthorized`,
`secureOptions`, `secureProtocol`, `servername`, `sessionIdContext`.
`secureOptions`, `secureProtocol`, `servername`, `sessionIdContext`,
`highWaterMark`.

`options` can be an object, a string, or a [`URL`][] object. If `options` is a
string, it is automatically parsed with [`new URL()`][]. If it is a [`URL`][]
Expand Down
5 changes: 5 additions & 0 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,9 @@ being issued by trusted CA (`options.ca`).
<!-- YAML
added: v0.11.3
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/32786
description: The `highWaterMark` option is accepted now.
- version: v13.6.0
pr-url: https://github.com/nodejs/node/pull/23188
description: The `pskCallback` option is now supported.
Expand Down Expand Up @@ -1370,6 +1373,8 @@ changes:
TLS connection. When a server offers a DH parameter with a size less
than `minDHSize`, the TLS connection is destroyed and an error is thrown.
**Default:** `1024`.
* `highWaterMark`: {number} Consistent with the readable stream `highWaterMark` parameter.
**Default:** `16 * 1024`.
* `secureContext`: TLS context object created with
[`tls.createSecureContext()`][]. If a `secureContext` is _not_ provided, one
will be created by passing the entire `options` object to
Expand Down
4 changes: 3 additions & 1 deletion lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,8 @@ function TLSSocket(socket, opts) {
pauseOnCreate: tlsOptions.pauseOnConnect,
// The readable flag is only needed if pauseOnCreate will be handled.
readable: tlsOptions.pauseOnConnect,
writable: false
writable: false,
highWaterMark: tlsOptions.highWaterMark
});

// Proxy for API compatibility
Expand Down Expand Up @@ -1584,6 +1585,7 @@ exports.connect = function connect(...args) {
requestOCSP: options.requestOCSP,
enableTrace: options.enableTrace,
pskCallback: options.pskCallback,
highWaterMark: options.highWaterMark,
});

tlssock[kConnectOptions] = options;
Expand Down
66 changes: 66 additions & 0 deletions test/parallel/test-https-hwm.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
'use strict';

// Test https highWaterMark

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const https = require('https');
const fixtures = require('../common/fixtures');

let counter = 0;

function loadCallback(highWaterMark) {
return common.mustCall(function(res) {
assert.strictEqual(highWaterMark, res.readableHighWaterMark);
counter--;
console.log('back from https request. ',
`highWaterMark = ${res.readableHighWaterMark}`);
if (counter === 0) {
httpsServer.close();
console.log('ok');
}
res.resume();
});
}

// create server
const httpsServer = https.createServer({
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem')
}, common.mustCall(function(req, res) {
res.writeHead(200, {});
res.end('ok');
}, 3)).listen(0, common.mustCall(function(err) {
console.log(`test https server listening on port ${this.address().port}`);
assert.ifError(err);

https.request({
method: 'GET',
path: `/${counter++}`,
host: 'localhost',
port: this.address().port,
rejectUnauthorized: false,
highWaterMark: 128000,
}, loadCallback(128000)).on('error', common.mustNotCall()).end();

https.request({
method: 'GET',
path: `/${counter++}`,
host: 'localhost',
port: this.address().port,
rejectUnauthorized: false,
highWaterMark: 0,
}, loadCallback(0)).on('error', common.mustNotCall()).end();

https.request({
method: 'GET',
path: `/${counter++}`,
host: 'localhost',
port: this.address().port,
rejectUnauthorized: false,
highWaterMark: undefined,
}, loadCallback(16 * 1024)).on('error', common.mustNotCall()).end();
}));
53 changes: 53 additions & 0 deletions test/parallel/test-tls-connect-hwm-option.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const tls = require('tls');
const fixtures = require('../common/fixtures');

const pem = (n) => fixtures.readKey(`${n}.pem`);

let clients = 0;

const server = tls.createServer({
key: pem('agent1-key'),
cert: pem('agent1-cert')
}, common.mustCall(() => {
if (--clients === 0)
server.close();
}, 3));

server.listen(0, common.mustCall(() => {
clients++;
const highBob = tls.connect({
port: server.address().port,
rejectUnauthorized: false,
highWaterMark: 128000,
}, common.mustCall(() => {
assert.strictEqual(highBob.readableHighWaterMark, 128000);
highBob.end();
}));

clients++;
const defaultHighBob = tls.connect({
port: server.address().port,
rejectUnauthorized: false,
highWaterMark: undefined,
}, common.mustCall(() => {
assert.strictEqual(defaultHighBob.readableHighWaterMark, 16 * 1024);
defaultHighBob.end();
}));

clients++;
const zeroHighBob = tls.connect({
port: server.address().port,
rejectUnauthorized: false,
highWaterMark: 0,
}, common.mustCall(() => {
assert.strictEqual(zeroHighBob.readableHighWaterMark, 0);
zeroHighBob.end();
}));
}));

0 comments on commit efb3c71

Please sign in to comment.