summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAnna Henningsen <anna@addaleax.net>2018-02-25 21:56:08 +0100
committerAnna Henningsen <anna@addaleax.net>2018-03-05 09:35:15 +0000
commit0c25cdf39a40a94fcb829ea91caa217d640054b1 (patch)
tree2e6ed7d43a465deed131a0eb232746fcbba4890e
parentb24d65dcdc74074304405522a190daf782048133 (diff)
downloadandroid-node-v8-0c25cdf39a40a94fcb829ea91caa217d640054b1.tar.gz
android-node-v8-0c25cdf39a40a94fcb829ea91caa217d640054b1.tar.bz2
android-node-v8-0c25cdf39a40a94fcb829ea91caa217d640054b1.zip
tls,http2: handle writes after SSL destroy more gracefully
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>
-rw-r--r--src/tls_wrap.cc13
-rw-r--r--test/parallel/test-http2-tls-disconnect.js32
2 files changed, 38 insertions, 7 deletions
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 9f8888b0c5..1cc5478bb5 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 0000000000..2e635fe137
--- /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);
+});