From 0c25cdf39a40a94fcb829ea91caa217d640054b1 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sun, 25 Feb 2018 21:56:08 +0100
Subject: [PATCH] tls,http2: handle writes after SSL destroy more gracefully
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This might otherwise result in a hard crash when trying
to write to a socket after a sudden disconnect.

Note that the test here uses an aborted `h2load` run to create
the failing requests; That’s far from ideal, but it provides
a reasonably reliably reproduction at this point.

PR-URL: https://github.com/nodejs/node/pull/18987
Fixes: https://github.com/nodejs/node/issues/18973
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 src/tls_wrap.cc                            | 13 ++++-----
 test/parallel/test-http2-tls-disconnect.js | 32 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 7 deletions(-)
 create mode 100644 test/parallel/test-http2-tls-disconnect.js

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 9f8888b0c54434..1cc5478bb57296 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -553,7 +553,12 @@ int TLSWrap::DoWrite(WriteWrap* w,
                      size_t count,
                      uv_stream_t* send_handle) {
   CHECK_EQ(send_handle, nullptr);
-  CHECK_NE(ssl_, nullptr);
+
+  if (ssl_ == nullptr) {
+    ClearError();
+    error_ = "Write after DestroySSL";
+    return UV_EPROTO;
+  }
 
   bool empty = true;
 
@@ -593,12 +598,6 @@ int TLSWrap::DoWrite(WriteWrap* w,
     return 0;
   }
 
-  if (ssl_ == nullptr) {
-    ClearError();
-    error_ = "Write after DestroySSL";
-    return UV_EPROTO;
-  }
-
   crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
 
   int written = 0;
diff --git a/test/parallel/test-http2-tls-disconnect.js b/test/parallel/test-http2-tls-disconnect.js
new file mode 100644
index 00000000000000..2e635fe1376a51
--- /dev/null
+++ b/test/parallel/test-http2-tls-disconnect.js
@@ -0,0 +1,32 @@
+'use strict';
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const child_process = require('child_process');
+const http2 = require('http2');
+const fs = require('fs');
+
+const key = fixtures.readKey('agent8-key.pem', 'binary');
+const cert = fixtures.readKey('agent8-cert.pem', 'binary');
+
+const server = http2.createSecureServer({ key, cert }, (request, response) => {
+  fs.createReadStream(process.execPath).pipe(response);
+});
+
+// This should be doable with a reproduction purely written in Node;
+// that just requires somebody to take the time and actually do it.
+server.listen(0, () => {
+  const proc = child_process.spawn('h2load', [
+    '-n', '1000',
+    `https://localhost:${server.address().port}/`
+  ]);
+  proc.on('error', (err) => {
+    if (err.code === 'ENOENT')
+      common.skip('no h2load');
+  });
+  proc.on('exit', () => server.close());
+  setTimeout(() => proc.kill(2), 100);
+});