From 632473286fd1a3e09dae867ece61820cc8c3ec32 Mon Sep 17 00:00:00 2001
From: Andrey Pechkurov <apechkurov@gmail.com>
Date: Wed, 13 May 2020 15:02:50 +0300
Subject: [PATCH 1/2] Revert "tls: add highWaterMark option for connect"

This reverts commit 58682d823acb0e566f16d1d8b33b83dfebf3aa5d.
---
 doc/api/https.md                             |  8 +--
 doc/api/tls.md                               |  7 ---
 lib/_tls_wrap.js                             |  4 +-
 test/parallel/test-https-hwm.js              | 66 --------------------
 test/parallel/test-tls-connect-hwm-option.js | 53 ----------------
 5 files changed, 2 insertions(+), 136 deletions(-)
 delete mode 100644 test/parallel/test-https-hwm.js
 delete mode 100644 test/parallel/test-tls-connect-hwm-option.js

diff --git a/doc/api/https.md b/doc/api/https.md
index 4476cf5eaa4703..6c95bedcfd57ac 100644
--- a/doc/api/https.md
+++ b/doc/api/https.md
@@ -240,11 +240,6 @@ Global instance of [`https.Agent`][] for all HTTPS client requests.
 <!-- YAML
 added: v0.3.6
 changes:
-  - version:
-    - v14.1.0
-    - v13.14.0
-    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
@@ -270,8 +265,7 @@ 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`,
-`highWaterMark`.
+`secureOptions`, `secureProtocol`, `servername`, `sessionIdContext`.
 
 `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`][]
diff --git a/doc/api/tls.md b/doc/api/tls.md
index c29dfcb48ae58f..94340a772f38ff 100644
--- a/doc/api/tls.md
+++ b/doc/api/tls.md
@@ -1280,11 +1280,6 @@ being issued by trusted CA (`options.ca`).
 <!-- YAML
 added: v0.11.3
 changes:
-  - version:
-    - v14.1.0
-    - v13.14.0
-    pr-url: https://github.com/nodejs/node/pull/32786
-    description: The `highWaterMark` option is accepted now.
   - version:
      - v13.6.0
      - v12.16.0
@@ -1385,8 +1380,6 @@ 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
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index 47b2c6009dba9c..a4af0ebc57e96d 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -498,8 +498,7 @@ function TLSSocket(socket, opts) {
     handle: this._wrapHandle(wrap),
     allowHalfOpen: socket ? socket.allowHalfOpen : tlsOptions.allowHalfOpen,
     pauseOnCreate: tlsOptions.pauseOnConnect,
-    manualStart: true,
-    highWaterMark: tlsOptions.highWaterMark,
+    manualStart: true
   });
 
   // Proxy for API compatibility
@@ -1580,7 +1579,6 @@ exports.connect = function connect(...args) {
     requestOCSP: options.requestOCSP,
     enableTrace: options.enableTrace,
     pskCallback: options.pskCallback,
-    highWaterMark: options.highWaterMark,
   });
 
   tlssock[kConnectOptions] = options;
diff --git a/test/parallel/test-https-hwm.js b/test/parallel/test-https-hwm.js
deleted file mode 100644
index 71ee33b7086861..00000000000000
--- a/test/parallel/test-https-hwm.js
+++ /dev/null
@@ -1,66 +0,0 @@
-'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();
-}));
diff --git a/test/parallel/test-tls-connect-hwm-option.js b/test/parallel/test-tls-connect-hwm-option.js
deleted file mode 100644
index e016ccc6cba0dc..00000000000000
--- a/test/parallel/test-tls-connect-hwm-option.js
+++ /dev/null
@@ -1,53 +0,0 @@
-'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();
-  }));
-}));

From de46fc400dfc4f48904b6b2cfd8fb977dfb9d5b3 Mon Sep 17 00:00:00 2001
From: Andrey Pechkurov <apechkurov@gmail.com>
Date: Wed, 13 May 2020 15:22:37 +0300
Subject: [PATCH 2/2] doc: add note for hwm restriction in tls

Also restores change entries for `highWaterMark` option
in `tls` and `https` docs.
---
 doc/api/https.md |  8 ++++++++
 doc/api/tls.md   | 12 +++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/doc/api/https.md b/doc/api/https.md
index 6c95bedcfd57ac..8be0fea420c432 100644
--- a/doc/api/https.md
+++ b/doc/api/https.md
@@ -240,6 +240,14 @@ 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/33387
+    description: The `highWaterMark` option is not supported now.
+  - version:
+    - v14.1.0
+    - v13.14.0
+    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
diff --git a/doc/api/tls.md b/doc/api/tls.md
index 94340a772f38ff..68746f39e30d9f 100644
--- a/doc/api/tls.md
+++ b/doc/api/tls.md
@@ -1280,6 +1280,14 @@ being issued by trusted CA (`options.ca`).
 <!-- YAML
 added: v0.11.3
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/33387
+    description: The `highWaterMark` option is not supported now.
+  - version:
+    - v14.1.0
+    - v13.14.0
+    pr-url: https://github.com/nodejs/node/pull/32786
+    description: The `highWaterMark` option is accepted now.
   - version:
      - v13.6.0
      - v12.16.0
@@ -1386,7 +1394,9 @@ changes:
     `tls.createSecureContext()`.
   * ...: [`tls.createSecureContext()`][] options that are used if the
     `secureContext` option is missing, otherwise they are ignored.
-  * ...: Any [`socket.connect()`][] option not already listed.
+  * ...: Any [`socket.connect()`][] option not already listed with the following
+    exception. The `highWaterMark` option is not supported, as the TLS protocol
+    assumes 16KB as the maximum record size.
 * `callback` {Function}
 * Returns: {tls.TLSSocket}