diff --git a/src/node_http2.cc b/src/node_http2.cc index 740072e92d6f1a..4a144b9e56cc8d 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -457,6 +457,8 @@ Http2Session::Callbacks::Callbacks(bool kHasGetPaddingCallback) { callbacks, OnNghttpError); nghttp2_session_callbacks_set_send_data_callback( callbacks, OnSendData); + nghttp2_session_callbacks_set_on_invalid_frame_recv_callback( + callbacks, OnInvalidFrame); if (kHasGetPaddingCallback) { nghttp2_session_callbacks_set_select_padding_callback( @@ -881,6 +883,31 @@ inline int Http2Session::OnFrameReceive(nghttp2_session* handle, return 0; } +inline int Http2Session::OnInvalidFrame(nghttp2_session* handle, + const nghttp2_frame *frame, + int lib_error_code, + void* user_data) { + Http2Session* session = static_cast(user_data); + + DEBUG_HTTP2SESSION2(session, "invalid frame received, code: %d", + lib_error_code); + + // If the error is fatal or if error code is ERR_STREAM_CLOSED... emit error + if (nghttp2_is_fatal(lib_error_code) || + lib_error_code == NGHTTP2_ERR_STREAM_CLOSED) { + Environment* env = session->env(); + Isolate* isolate = env->isolate(); + HandleScope scope(isolate); + Local context = env->context(); + Context::Scope context_scope(context); + + Local argv[1] = { + Integer::New(isolate, lib_error_code), + }; + session->MakeCallback(env->error_string(), arraysize(argv), argv); + } + return 0; +} // If nghttp2 is unable to send a queued up frame, it will call this callback // to let us know. If the failure occurred because we are in the process of diff --git a/src/node_http2.h b/src/node_http2.h index e154b2fc79a6b1..ae35314c1ad380 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -992,6 +992,11 @@ class Http2Session : public AsyncWrap { size_t length, nghttp2_data_source* source, void* user_data); + static inline int OnInvalidFrame( + nghttp2_session* session, + const nghttp2_frame *frame, + int lib_error_code, + void* user_data); static inline ssize_t OnStreamReadFD( diff --git a/test/common/http2.js b/test/common/http2.js new file mode 100644 index 00000000000000..44f5f9191e6e33 --- /dev/null +++ b/test/common/http2.js @@ -0,0 +1,129 @@ +/* eslint-disable required-modules */ +'use strict'; + +// An HTTP/2 testing tool used to create mock frames for direct testing +// of HTTP/2 endpoints. + +const kFrameData = Symbol('frame-data'); +const FLAG_EOS = 0x1; +const FLAG_ACK = 0x1; +const FLAG_EOH = 0x4; +const FLAG_PADDED = 0x8; +const PADDING = Buffer.alloc(255); + +const kClientMagic = Buffer.from('505249202a20485454502f322' + + 'e300d0a0d0a534d0d0a0d0a', 'hex'); + +const kFakeRequestHeaders = Buffer.from('828684410f7777772e65' + + '78616d706c652e636f6d', 'hex'); + + +const kFakeResponseHeaders = Buffer.from('4803333032580770726976617465611d' + + '4d6f6e2c203231204f63742032303133' + + '2032303a31333a323120474d546e1768' + + '747470733a2f2f7777772e6578616d70' + + '6c652e636f6d', 'hex'); + +function isUint32(val) { + return val >>> 0 === val; +} + +function isUint24(val) { + return val >>> 0 === val && val <= 0xFFFFFF; +} + +function isUint8(val) { + return val >>> 0 === val && val <= 0xFF; +} + +function write32BE(array, pos, val) { + if (!isUint32(val)) + throw new RangeError('val is not a 32-bit number'); + array[pos++] = (val >> 24) & 0xff; + array[pos++] = (val >> 16) & 0xff; + array[pos++] = (val >> 8) & 0xff; + array[pos++] = val & 0xff; +} + +function write24BE(array, pos, val) { + if (!isUint24(val)) + throw new RangeError('val is not a 24-bit number'); + array[pos++] = (val >> 16) & 0xff; + array[pos++] = (val >> 8) & 0xff; + array[pos++] = val & 0xff; +} + +function write8(array, pos, val) { + if (!isUint8(val)) + throw new RangeError('val is not an 8-bit number'); + array[pos] = val; +} + +class Frame { + constructor(length, type, flags, id) { + this[kFrameData] = Buffer.alloc(9); + write24BE(this[kFrameData], 0, length); + write8(this[kFrameData], 3, type); + write8(this[kFrameData], 4, flags); + write32BE(this[kFrameData], 5, id); + } + + get data() { + return this[kFrameData]; + } +} + +class SettingsFrame extends Frame { + constructor(ack = false) { + let flags = 0; + if (ack) + flags |= FLAG_ACK; + super(0, 4, flags, 0); + } +} + +class DataFrame extends Frame { + constructor(id, payload, padlen = 0, final = false) { + let len = payload.length; + let flags = 0; + if (final) flags |= FLAG_EOS; + const buffers = [payload]; + if (padlen > 0) { + buffers.unshift(Buffer.from([padlen])); + buffers.push(PADDING.slice(0, padlen)); + len += padlen + 1; + flags |= FLAG_PADDED; + } + super(len, 0, flags, id); + buffers.unshift(this[kFrameData]); + this[kFrameData] = Buffer.concat(buffers); + } +} + +class HeadersFrame extends Frame { + constructor(id, payload, padlen = 0, final = false) { + let len = payload.length; + let flags = FLAG_EOH; + if (final) flags |= FLAG_EOS; + const buffers = [payload]; + if (padlen > 0) { + buffers.unshift(Buffer.from([padlen])); + buffers.push(PADDING.slice(0, padlen)); + len += padlen + 1; + flags |= FLAG_PADDED; + } + super(len, 1, flags, id); + buffers.unshift(this[kFrameData]); + this[kFrameData] = Buffer.concat(buffers); + } +} + +module.exports = { + Frame, + DataFrame, + HeadersFrame, + SettingsFrame, + kFakeRequestHeaders, + kFakeResponseHeaders, + kClientMagic +}; diff --git a/test/parallel/test-http2-misbehaving-multiplex.js b/test/parallel/test-http2-misbehaving-multiplex.js new file mode 100644 index 00000000000000..7d5a7a2f552d49 --- /dev/null +++ b/test/parallel/test-http2-misbehaving-multiplex.js @@ -0,0 +1,59 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const h2 = require('http2'); +const net = require('net'); +const h2test = require('../common/http2'); +let client; + +const server = h2.createServer(); +server.on('stream', common.mustCall((stream) => { + stream.respond(); + stream.end('ok'); +}, 2)); +server.on('session', common.mustCall((session) => { + session.on('error', common.expectsError({ + code: 'ERR_HTTP2_ERROR', + type: Error, + message: 'Stream was already closed or invalid' + })); +})); + +const settings = new h2test.SettingsFrame(); +const settingsAck = new h2test.SettingsFrame(true); +const head1 = new h2test.HeadersFrame(1, h2test.kFakeRequestHeaders, 0, true); +const head2 = new h2test.HeadersFrame(3, h2test.kFakeRequestHeaders, 0, true); +const head3 = new h2test.HeadersFrame(1, h2test.kFakeRequestHeaders, 0, true); +const head4 = new h2test.HeadersFrame(5, h2test.kFakeRequestHeaders, 0, true); + +server.listen(0, () => { + client = net.connect(server.address().port, () => { + client.write(h2test.kClientMagic, () => { + client.write(settings.data, () => { + client.write(settingsAck.data); + // This will make it ok. + client.write(head1.data, () => { + // This will make it ok. + client.write(head2.data, () => { + // This will cause an error to occur because the client is + // attempting to reuse an already closed stream. This must + // cause the server session to be torn down. + client.write(head3.data, () => { + // This won't ever make it to the server + client.write(head4.data); + }); + }); + }); + }); + }); + }); + + // An error may or may not be emitted on the client side, we don't care + // either way if it is, but we don't want to die if it is. + client.on('error', () => {}); + client.on('close', common.mustCall(() => server.close())); +});