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 953072d

Browse filesBrowse files
santigimenorichardlau
authored andcommitted
src: let http2 streams end after session close
After the stream has been marked as closed by the nghttp2 stack, there might be still pending data to be sent: trailing headers is an example of this. In that case, avoid reentering the nghttp2 stack synchronously to allow the data to be written before actually closing the stream. Fixes: #42713 PR-URL: #45153 Backport-PR-URL: #45660 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent 6733556 commit 953072d
Copy full SHA for 953072d

File tree

Expand file treeCollapse file tree

2 files changed

+60
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+60
-0
lines changed
Open diff view settings
Collapse file

‎src/node_http2.cc‎

Copy file name to clipboardExpand all lines: src/node_http2.cc
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,6 +1115,17 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
11151115
if (!stream || stream->is_destroyed())
11161116
return 0;
11171117

1118+
// Don't close synchronously in case there's pending data to be written. This
1119+
// may happen when writing trailing headers.
1120+
if (code == NGHTTP2_NO_ERROR && nghttp2_session_want_write(handle) &&
1121+
!env->is_stopping()) {
1122+
env->SetImmediate([handle, id, code, user_data](Environment* env) {
1123+
OnStreamClose(handle, id, code, user_data);
1124+
});
1125+
1126+
return 0;
1127+
}
1128+
11181129
stream->Close(code);
11191130

11201131
// It is possible for the stream close to occur before the stream is
Collapse file
+49Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const http2 = require('http2');
8+
9+
const {
10+
HTTP2_HEADER_PATH,
11+
HTTP2_HEADER_STATUS,
12+
HTTP2_HEADER_METHOD,
13+
} = http2.constants;
14+
15+
const server = http2.createServer();
16+
server.on('stream', common.mustCall((stream) => {
17+
server.close();
18+
stream.session.close();
19+
stream.on('wantTrailers', common.mustCall(() => {
20+
stream.sendTrailers({ xyz: 'abc' });
21+
}));
22+
23+
stream.respond({ [HTTP2_HEADER_STATUS]: 200 }, { waitForTrailers: true });
24+
stream.write('some data');
25+
stream.end();
26+
}));
27+
28+
server.listen(0, common.mustCall(() => {
29+
const port = server.address().port;
30+
const client = http2.connect(`http://localhost:${port}`);
31+
client.socket.on('close', common.mustCall());
32+
const req = client.request({
33+
[HTTP2_HEADER_PATH]: '/',
34+
[HTTP2_HEADER_METHOD]: 'POST'
35+
});
36+
req.end();
37+
req.on('response', common.mustCall());
38+
let data = '';
39+
req.on('data', (chunk) => {
40+
data += chunk;
41+
});
42+
req.on('end', common.mustCall(() => {
43+
assert.strictEqual(data, 'some data');
44+
}));
45+
req.on('trailers', common.mustCall((headers) => {
46+
assert.strictEqual(headers.xyz, 'abc');
47+
}));
48+
req.on('close', common.mustCall());
49+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.