OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2025-12-06 00:20:08 +01:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

http2: do not start reading after write if new write is on wire

Browse Source 
Don’t start reading more input data if we’re still busy writing output.
This was overlooked in 8a4a1931b8.

Fixes: https://github.com/nodejs/node/issues/29353
Fixes: https://github.com/nodejs/node/issues/29393

PR-URL: https://github.com/nodejs/node/pull/29399
Backport-PR-URL: https://github.com/nodejs/node/pull/29618
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
Anna Henningsen 2019-09-02 01:32:15 +02:00 committed by Beth Griggs
parent 559a8e342b
commit e45b6a3b98
No known key found for this signature in database
GPG Key ID: D7062848A1AB005C
2 changed files with 52 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/node_http2.cc
View File

@ -791,8 +791,10 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
flags_ |= SESSION_STATE_CLOSING;
// Stop reading on the i/o stream
if (stream_ != nullptr)
if (stream_ != nullptr) {
flags_ |= SESSION_STATE_READING_STOPPED;
stream_->ReadStop();
}
// If the socket is not closed, then attempt to send a closing GOAWAY
// frame. There is no guarantee that this GOAWAY will be received by
@ -1280,6 +1282,7 @@ inline int Http2Session::OnDataChunkReceived(nghttp2_session* handle,
// If we are currently waiting for a write operation to finish, we should
// tell nghttp2 that we want to wait before we process more input data.
if (session->flags_ & SESSION_STATE_WRITE_IN_PROGRESS) {
CHECK_NE(session->flags_ & SESSION_STATE_READING_STOPPED, 0);
session->flags_ |= SESSION_STATE_NGHTTP2_RECV_PAUSED;
return NGHTTP2_ERR_PAUSE;
}
@ -1626,6 +1629,7 @@ void Http2Session::OnStreamAfterWriteImpl(WriteWrap* w, int status, void* ctx) {
session->ClearOutgoing(status);
if ((session->flags_ & SESSION_STATE_READING_STOPPED) &&
!(session->flags_ & SESSION_STATE_WRITE_IN_PROGRESS) &&
nghttp2_session_want_read(session->session_)) {
session->flags_ &= ~SESSION_STATE_READING_STOPPED;
session->stream_->ReadStart();

47
test/parallel/test-http2-multistream-destroy-on-read-tls.js Normal file
View File

@ -0,0 +1,47 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const fixtures = require('../common/fixtures');
const http2 = require('http2');
// Regression test for https://github.com/nodejs/node/issues/29353.
// Test that it’s okay for an HTTP2 + TLS server to destroy a stream instance
// while reading it.
const server = http2.createSecureServer({
key: fixtures.readKey('agent2-key.pem'),
cert: fixtures.readKey('agent2-cert.pem')
});
const filenames = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
server.on('stream', common.mustCall((stream) => {
function write() {
stream.write('a'.repeat(10240));
stream.once('drain', write);
}
write();
}, filenames.length));
server.listen(0, common.mustCall(() => {
const client = http2.connect(`https://localhost:${server.address().port}`, {
ca: fixtures.readKey('agent2-cert.pem'),
servername: 'agent2'
});
let destroyed = 0;
for (const entry of filenames) {
const stream = client.request({
':path': `/${entry}`
});
stream.once('data', common.mustCall(() => {
stream.destroy();
if (++destroyed === filenames.length) {
client.destroy();
server.close();
}
}));
}
}));