From 6a396ff91153c55115e5b1cf39c04dbbbe0412c6 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 23 Aug 2018 10:38:31 -0700 Subject: [PATCH] http2: throw better error when accessing unbound socket proxy Fixes: https://github.com/nodejs/node/issues/22268 Backport-PR-URL: https://github.com/nodejs/node/pull/22850 PR-URL: https://github.com/nodejs/node/pull/22486 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott Reviewed-By: Trivikram Kamat Reviewed-By: Matteo Collina --- doc/api/errors.md | 6 ++ lib/internal/errors.js | 2 + lib/internal/http2/core.js | 12 +++- .../test-http2-unbound-socket-proxy.js | 69 +++++++++++++++++++ 4 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-http2-unbound-socket-proxy.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 8632bb7be28484..d7c5da0ceb4f58 100755 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -843,6 +843,12 @@ The `Http2Session` settings canceled. An attempt was made to connect a `Http2Session` object to a `net.Socket` or `tls.TLSSocket` that had already been bound to another `Http2Session` object. + +### ERR_HTTP2_SOCKET_UNBOUND + +An attempt was made to use the `socket` property of an `Http2Session` that +has already been closed. + ### ERR_HTTP2_STATUS_101 diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 587fce65613374..0863f951830938 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -338,6 +338,8 @@ E('ERR_HTTP2_SESSION_ERROR', 'Session closed with error code %s'); E('ERR_HTTP2_SETTINGS_CANCEL', 'HTTP2 session settings canceled'); E('ERR_HTTP2_SOCKET_BOUND', 'The socket is already bound to an Http2Session'); +E('ERR_HTTP2_SOCKET_UNBOUND', + 'The socket has been disconnected from the Http2Session'); E('ERR_HTTP2_STATUS_101', 'HTTP status code 101 (Switching Protocols) is forbidden in HTTP/2'); E('ERR_HTTP2_STATUS_INVALID', 'Invalid status code: %s'); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 2dea50ea065556..9aa7a0e2a2f79e 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -641,12 +641,17 @@ const proxySocketHandler = { throw new errors.Error('ERR_HTTP2_NO_SOCKET_MANIPULATION'); default: const socket = session[kSocket]; + if (socket === undefined) + throw new errors.Error('ERR_HTTP2_SOCKET_UNBOUND'); const value = socket[prop]; return typeof value === 'function' ? value.bind(socket) : value; } }, getPrototypeOf(session) { - return Reflect.getPrototypeOf(session[kSocket]); + const socket = session[kSocket]; + if (socket === undefined) + throw new errors.Error('ERR_HTTP2_SOCKET_UNBOUND'); + return Reflect.getPrototypeOf(socket); }, set(session, prop, value) { switch (prop) { @@ -662,7 +667,10 @@ const proxySocketHandler = { case 'write': throw new errors.Error('ERR_HTTP2_NO_SOCKET_MANIPULATION'); default: - session[kSocket][prop] = value; + const socket = session[kSocket]; + if (socket === undefined) + throw new errors.Error('ERR_HTTP2_SOCKET_UNBOUND'); + socket[prop] = value; return true; } } diff --git a/test/parallel/test-http2-unbound-socket-proxy.js b/test/parallel/test-http2-unbound-socket-proxy.js new file mode 100644 index 00000000000000..44f113bac962f7 --- /dev/null +++ b/test/parallel/test-http2-unbound-socket-proxy.js @@ -0,0 +1,69 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); +const net = require('net'); + +const server = http2.createServer(); +server.on('stream', common.mustCall((stream) => { + stream.respond(); + stream.end('ok'); +})); + +server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + const socket = client.socket; + const req = client.request(); + req.resume(); + req.on('close', common.mustCall(() => { + client.close(); + server.close(); + + // Tests to make sure accessing the socket proxy fails with an + // informative error. + setImmediate(common.mustCall(() => { + common.expectsError(() => { + socket.example; + }, { + code: 'ERR_HTTP2_SOCKET_UNBOUND' + }); + common.expectsError(() => { + socket.example = 1; + }, { + code: 'ERR_HTTP2_SOCKET_UNBOUND' + }); + common.expectsError(() => { + socket instanceof net.Socket; + }, { + code: 'ERR_HTTP2_SOCKET_UNBOUND' + }); + common.expectsError(() => { + socket.ref(); + }, { + code: 'ERR_HTTP2_SOCKET_UNBOUND' + }); + common.expectsError(() => { + socket.unref(); + }, { + code: 'ERR_HTTP2_SOCKET_UNBOUND' + }); + common.expectsError(() => { + socket.setEncoding(); + }, { + code: 'ERR_HTTP2_SOCKET_UNBOUND' + }); + common.expectsError(() => { + socket.setKeepAlive(); + }, { + code: 'ERR_HTTP2_SOCKET_UNBOUND' + }); + common.expectsError(() => { + socket.setNoDelay(); + }, { + code: 'ERR_HTTP2_SOCKET_UNBOUND' + }); + })); + })); +}));