Skip to content

Commit 9bc7a95

Browse files
RafaelGSSsxa
authored andcommitted
http2: close stream and session on frameError
PR-URL: #42147 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent ce40927 commit 9bc7a95

File tree

4 files changed

+77
-3
lines changed

4 files changed

+77
-3
lines changed

lib/internal/http2/core.js

+2
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,8 @@ function onFrameError(id, type, code) {
607607
const emitter = session[kState].streams.get(id) || session;
608608
emitter[kUpdateTimer]();
609609
emitter.emit('frameError', type, code, id);
610+
session[kState].streams.get(id).close(code);
611+
session.close();
610612
}
611613

612614
function onAltSvc(stream, origin, alt) {

src/node_http2.cc

+22-1
Original file line numberDiff line numberDiff line change
@@ -1024,6 +1024,27 @@ void Http2Session::DecrefHeaders(const nghttp2_frame* frame) {
10241024
}
10251025
}
10261026

1027+
uint32_t TranslateNghttp2ErrorCode(const int libErrorCode) {
1028+
switch (libErrorCode) {
1029+
case NGHTTP2_ERR_STREAM_CLOSED:
1030+
return NGHTTP2_STREAM_CLOSED;
1031+
case NGHTTP2_ERR_HEADER_COMP:
1032+
return NGHTTP2_COMPRESSION_ERROR;
1033+
case NGHTTP2_ERR_FRAME_SIZE_ERROR:
1034+
return NGHTTP2_FRAME_SIZE_ERROR;
1035+
case NGHTTP2_ERR_FLOW_CONTROL:
1036+
return NGHTTP2_FLOW_CONTROL_ERROR;
1037+
case NGHTTP2_ERR_REFUSED_STREAM:
1038+
return NGHTTP2_REFUSED_STREAM;
1039+
case NGHTTP2_ERR_PROTO:
1040+
case NGHTTP2_ERR_HTTP_HEADER:
1041+
case NGHTTP2_ERR_HTTP_MESSAGING:
1042+
return NGHTTP2_PROTOCOL_ERROR;
1043+
default:
1044+
return NGHTTP2_INTERNAL_ERROR;
1045+
}
1046+
}
1047+
10271048
// If nghttp2 is unable to send a queued up frame, it will call this callback
10281049
// to let us know. If the failure occurred because we are in the process of
10291050
// closing down the session or stream, we go ahead and ignore it. We don't
@@ -1060,7 +1081,7 @@ int Http2Session::OnFrameNotSent(nghttp2_session* handle,
10601081
Local<Value> argv[3] = {
10611082
Integer::New(isolate, frame->hd.stream_id),
10621083
Integer::New(isolate, frame->hd.type),
1063-
Integer::New(isolate, error_code)
1084+
Integer::New(isolate, TranslateNghttp2ErrorCode(error_code))
10641085
};
10651086
session->MakeCallback(
10661087
env->http2session_on_frame_error_function(),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const assert = require('assert');
8+
const { createServer, constants, connect } = require('http2');
9+
10+
const server = createServer();
11+
12+
server.on('stream', (stream, headers) => {
13+
stream.respond(undefined, { waitForTrailers: true });
14+
15+
stream.on('data', common.mustNotCall());
16+
17+
stream.on('wantTrailers', common.mustCall(() => {
18+
// Trigger a frame error by sending a trailer that is too large
19+
stream.sendTrailers({ 'test-trailer': 'X'.repeat(64 * 1024) });
20+
}));
21+
22+
stream.on('frameError', common.mustCall((frameType, errorCode) => {
23+
assert.strictEqual(errorCode, constants.NGHTTP2_FRAME_SIZE_ERROR);
24+
}));
25+
26+
stream.on('error', common.expectsError({
27+
code: 'ERR_HTTP2_STREAM_ERROR',
28+
}));
29+
30+
stream.on('close', common.mustCall());
31+
32+
stream.end();
33+
});
34+
35+
server.listen(0, () => {
36+
const clientSession = connect(`http://localhost:${server.address().port}`);
37+
38+
clientSession.on('frameError', common.mustNotCall());
39+
clientSession.on('close', common.mustCall(() => {
40+
server.close();
41+
}));
42+
43+
const clientStream = clientSession.request();
44+
45+
clientStream.on('close', common.mustCall());
46+
// These events mustn't be called once the frame size error is from the server
47+
clientStream.on('frameError', common.mustNotCall());
48+
clientStream.on('error', common.mustNotCall());
49+
50+
clientStream.end();
51+
});

test/parallel/test-http2-options-max-headers-block-length.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ server.listen(0, common.mustCall(() => {
3232
}));
3333

3434
req.on('frameError', common.mustCall((type, code) => {
35-
assert.strictEqual(code, h2.constants.NGHTTP2_ERR_FRAME_SIZE_ERROR);
35+
assert.strictEqual(code, h2.constants.NGHTTP2_FRAME_SIZE_ERROR);
3636
}));
3737

3838
req.on('error', common.expectsError({
3939
code: 'ERR_HTTP2_STREAM_ERROR',
4040
name: 'Error',
41-
message: 'Stream closed with error code NGHTTP2_REFUSED_STREAM'
41+
message: 'Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR'
4242
}));
4343
}));

0 commit comments

Comments
 (0)