Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 479b622

Browse filesBrowse 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 <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent e584113 commit 479b622
Copy full SHA for 479b622

File tree

Expand file treeCollapse file tree

2 files changed

+38
-7
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+38
-7
lines changed
Open diff view settings
Collapse file

‎src/tls_wrap.cc‎

Copy file name to clipboardExpand all lines: src/tls_wrap.cc
+6-7Lines changed: 6 additions & 7 deletions
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;
Collapse file
+32Lines changed: 32 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.