diff --git a/src/stream_base.cc b/src/stream_base.cc index ad9affc53873..afc2288f186d 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -799,6 +799,8 @@ void CustomBufferJSListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { void ReportWritesToJSStreamListener::OnStreamAfterReqFinished( StreamReq* req_wrap, int status) { StreamBase* stream = static_cast(stream_); + if (stream == nullptr) return; + if (req_wrap == nullptr) return; Environment* env = stream->stream_env(); if (!env->can_call_into_js()) return; AsyncWrap* async_wrap = req_wrap->GetAsyncWrap(); diff --git a/test/parallel/test-http2-respond-with-file-cancelled-crash.js b/test/parallel/test-http2-respond-with-file-cancelled-crash.js new file mode 100644 index 000000000000..c9687a7f92aa --- /dev/null +++ b/test/parallel/test-http2-respond-with-file-cancelled-crash.js @@ -0,0 +1,51 @@ +'use strict'; + +// Regression test for https://hackerone.com/4150894 (NULL dereference in +// StreamPipe). Cancelling an in-flight `respondWithFile()` download while a +// file read is still pending on the thread pool crashed the server: a NULL +// WriteWrap was forwarded to ReportWritesToJSStreamListener, which then +// dereferenced it in OnStreamAfterReqFinished(). + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); + +const server = http2.createServer(); +server.on('stream', (stream) => { + stream.on('error', () => {}); + // Use a file large enough that a read is still in flight when the client + // destroys the session. + stream.respondWithFile(process.execPath); +}); +server.on('sessionError', () => {}); + +server.listen(0, common.mustCall(() => { + const port = server.address().port; + const rounds = 50; + let i = 0; + let done = false; + + function cancelOne() { + if (done) return; + if (i >= rounds) { + done = true; + server.close(common.mustCall()); + return; + } + i++; + const client = http2.connect(`http://127.0.0.1:${port}`); + client.on('error', () => {}); + const req = client.request({ ':path': '/' }); + req.on('error', () => {}); + req.resume(); + // Destroy the session shortly after the request is sent, while the server + // may still be reading the file. Vary the delay to hit the race window. + setTimeout(() => { + client.destroy(); + setTimeout(cancelOne, 1); + }, 3 + (i * 7) % 40); + } + + cancelOne(); +}));