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: #18987Fixes: #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‎

Lines 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
return0;
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;
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
constfixtures=require('../common/fixtures');
4+
5+
if(!common.hasCrypto)
6+
common.skip('missing crypto');
7+
8+
constchild_process=require('child_process');
9+
consthttp2=require('http2');
10+
constfs=require('fs');
11+
12+
constkey=fixtures.readKey('agent8-key.pem','binary');
13+
constcert=fixtures.readKey('agent8-cert.pem','binary');
14+
15+
constserver=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+
constproc=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)