Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: update handling of streams on rst_stream frames #39622

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 12 additions & 12 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2196,21 +2196,21 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
CHECK(!this->is_destroyed());
code_ = code;

// If RST_STREAM frame is received and stream is not writable
// because it is busy reading data, don't try force purging it.
// Instead add the stream to pending stream list and process
// the pending data when it is safe to do so. This is to avoid
// double free error due to unwanted behavior of nghttp2.
// Ref:https://github.com/nodejs/node/issues/38964

// Add stream to the pending list if it is received with scope
auto is_stream_cancel = [](const uint32_t code) {
return code == NGHTTP2_CANCEL;
};

// If RST_STREAM frame is received with error code NGHTTP2_CANCEL,
// add it to the pending list and don't force purge the data. It is
// to avoids the double free error due to unwanted behavior of nghttp2.

// Add stream to the pending list only if it is received with scope
// below in the stack. The pending list may not get processed
// if RST_STREAM received is not in scope and added to the list
// causing endpoint to hang.
if (session_->is_in_scope() &&
!is_writable() && is_reading()) {
session_->AddPendingRstStream(id_);
return;
if (session_->is_in_scope() && is_stream_cancel(code)) {
session_->AddPendingRstStream(id_);
return;
}


Expand Down
36 changes: 36 additions & 0 deletions test/parallel/test-http2-cancel-while-client-reading.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';
const common = require('../common');
const fixtures = require('../common/fixtures');
if (!common.hasCrypto) {
common.skip('missing crypto');
}

const http2 = require('http2');
const key = fixtures.readKey('agent1-key.pem', 'binary');
const cert = fixtures.readKey('agent1-cert.pem', 'binary');

const server = http2.createSecureServer({ key, cert });

let client_stream;

server.on('stream', common.mustCall(function(stream) {
stream.resume();
stream.on('data', function(chunk) {
stream.write(chunk);
client_stream.pause();
client_stream.close(http2.constants.NGHTTP2_CANCEL);
});
}));

server.listen(0, function() {
const client = http2.connect(`https://localhost:${server.address().port}`,
{ rejectUnauthorized: false }
);
client_stream = client.request({ ':method': 'POST' });
client_stream.on('close', common.mustCall(() => {
client.close();
server.close();
}));
client_stream.resume();
client_stream.write(Buffer.alloc(1024 * 1024));
});