Skip to content

Commit 479b622

Browse files
committed
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: #18987 Fixes: #18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent e584113 commit 479b622

File tree

2 files changed

+38
-7
lines changed

2 files changed

+38
-7
lines changed

src/tls_wrap.cc

+6-7
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,12 @@ int TLSWrap::DoWrite(WriteWrap* w,
565565
size_t count,
566566
uv_stream_t* send_handle) {
567567
CHECK_EQ(send_handle, nullptr);
568-
CHECK_NE(ssl_, nullptr);
568+
569+
if (ssl_ == nullptr) {
570+
ClearError();
571+
error_ = "Write after DestroySSL";
572+
return UV_EPROTO;
573+
}
569574

570575
bool empty = true;
571576

@@ -605,12 +610,6 @@ int TLSWrap::DoWrite(WriteWrap* w,
605610
return 0;
606611
}
607612

608-
if (ssl_ == nullptr) {
609-
ClearError();
610-
error_ = "Write after DestroySSL";
611-
return UV_EPROTO;
612-
}
613-
614613
crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
615614

616615
int written = 0;
+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict';
2+
const common = require('../common');
3+
const fixtures = require('../common/fixtures');
4+
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
8+
const child_process = require('child_process');
9+
const http2 = require('http2');
10+
const fs = require('fs');
11+
12+
const key = fixtures.readKey('agent8-key.pem', 'binary');
13+
const cert = fixtures.readKey('agent8-cert.pem', 'binary');
14+
15+
const server = http2.createSecureServer({ key, cert }, (request, response) => {
16+
fs.createReadStream(process.execPath).pipe(response);
17+
});
18+
19+
// This should be doable with a reproduction purely written in Node;
20+
// that just requires somebody to take the time and actually do it.
21+
server.listen(0, () => {
22+
const proc = child_process.spawn('h2load', [
23+
'-n', '1000',
24+
`https://localhost:${server.address().port}/`
25+
]);
26+
proc.on('error', (err) => {
27+
if (err.code === 'ENOENT')
28+
common.skip('no h2load');
29+
});
30+
proc.on('exit', () => server.close());
31+
setTimeout(() => proc.kill(2), 100);
32+
});

0 commit comments

Comments
 (0)