From b0f10e96b291e78dfd7e52456f10a398d23db1f4 Mon Sep 17 00:00:00 2001 From: Anna Henningsen 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 Reviewed-By: Matteo Collina --- 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); +});