Skip to content

Commit

Permalink
tls,http2: handle writes after SSL destroy more gracefully
Browse files Browse the repository at this point in the history
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: #18987
Fixes: #18973
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
addaleax committed Mar 5, 2018
1 parent b24d65d commit 0c25cdf
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
13 changes: 6 additions & 7 deletions src/tls_wrap.cc
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
32 changes: 32 additions & 0 deletions 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);
});

0 comments on commit 0c25cdf

Please sign in to comment.