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 5335e29

Browse filesBrowse files
santigimenoRafaelGSS
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 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 f589961 commit 5335e29
Copy full SHA for 5335e29

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
@@ -1120,6 +1120,17 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
11201120
if (!stream || stream->is_destroyed())
11211121
return 0;
11221122

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

11251136
// 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.