From 28d79556147353a382d832158d78c19dc79f9a01 Mon Sep 17 00:00:00 2001 From: iskore Date: Sat, 3 Feb 2018 12:12:32 -0500 Subject: [PATCH 1/2] http: setEncoding override for incoming packets added override to socket.setEncoding to not allow encoding changes for incoming HTTP requests added tests to ensure method throws JavaScript error because an HTTP buffer must be in US-ASCII, this function should not be allowed and should throw an Error currently, the process encounters a fatal v8 error and crashes error report detailed in [issue #18118](nodejs#18118) Fixes: nodejs#18118 Ref: nodejs#18178 --- lib/_http_server.js | 11 ++++++--- .../test-http-socket-encoding-error.js | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-http-socket-encoding-error.js diff --git a/lib/_http_server.js b/lib/_http_server.js index 496ebf285c814e..17e162c09532a5 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -363,6 +363,7 @@ function connectionListenerInternal(server, socket) { // Override on to unconsume on `data`, `readable` listeners socket.on = socketOnWrap; + socket.setEncoding = socketSetEncoding; // We only consume the socket if it has never been consumed before. if (socket._handle) { @@ -379,7 +380,6 @@ function connectionListenerInternal(server, socket) { socket._paused = false; } - function updateOutgoingData(socket, state, delta) { state.outgoingData += delta; if (socket._paused && @@ -454,7 +454,6 @@ function socketOnEnd(server, socket, parser, state) { function socketOnData(server, socket, parser, state, d) { assert(!socket._paused); debug('SERVER socketOnData %d', d.length); - var ret = parser.execute(d); onParserExecuteCommon(server, socket, parser, state, ret, d); } @@ -484,7 +483,6 @@ function socketOnError(e) { function onParserExecuteCommon(server, socket, parser, state, ret, d) { resetSocketTimeout(server, socket, state); - if (ret instanceof Error) { ret.rawPacket = d || parser.getCurrentBuffer(); debug('parse error', ret); @@ -498,6 +496,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { if (!d) d = parser.getCurrentBuffer(); + socket._setEncoding( socket._desiredEncoding ); socket.removeListener('data', state.onData); socket.removeListener('end', state.onEnd); socket.removeListener('close', state.onClose); @@ -519,6 +518,8 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { // Got upgrade header or CONNECT method, but have no handler. socket.destroy(); } + } else if( !parser.incoming.upgrade && socket._desiredEncoding ) { + throw new errors.Error('ERR_METHOD_NOT_IMPLEMENTED', 'setEncoding'); } if (socket._paused && socket.parser) { @@ -663,6 +664,10 @@ function onSocketPause() { } } +function socketSetEncoding() { + throw new errors.Error('ERR_METHOD_NOT_IMPLEMENTED','setEncoding'); +} + function unconsume(parser, socket) { if (socket._handle) { if (parser._consumed) diff --git a/test/parallel/test-http-socket-encoding-error.js b/test/parallel/test-http-socket-encoding-error.js new file mode 100644 index 00000000000000..e3ab5cd3eb5fdc --- /dev/null +++ b/test/parallel/test-http-socket-encoding-error.js @@ -0,0 +1,23 @@ +'use strict'; + +const common = require( '../common' ); +const http = require('http'); + +const server = http.createServer().listen(0, connectToServer); + +server.on('connection', socket => { + common.expectsError(() => socket.setEncoding(''), + { + code: 'ERR_METHOD_NOT_IMPLEMENTED', + type: Error + }); + + socket.end(); +}); + +function connectToServer() { + const client = new http.Agent().createConnection(this.address().port, () => { + client.end(); + }) + .on('end', () => server.close()); +} From a8822585803bbec459c1f364e37e7a1245af780a Mon Sep 17 00:00:00 2001 From: iskore Date: Sat, 3 Feb 2018 12:37:14 -0500 Subject: [PATCH 2/2] http: setEncoding override for incoming packets added override to socket.setEncoding to not allow encoding changes for incoming HTTP requests added tests to ensure method throws JavaScript error because an HTTP buffer must be in US-ASCII, this function should not be allowed and should throw an Error currently, the process encounters a fatal v8 error and crashes error report detailed in [issue #18118](nodejs#18118) Fixes: nodejs#18118 Ref: nodejs#18178 --- lib/_http_server.js | 6 ++---- test/parallel/test-http-socket-encoding-error.js | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index 17e162c09532a5..c78d7a8ac6de6c 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -483,6 +483,7 @@ function socketOnError(e) { function onParserExecuteCommon(server, socket, parser, state, ret, d) { resetSocketTimeout(server, socket, state); + if (ret instanceof Error) { ret.rawPacket = d || parser.getCurrentBuffer(); debug('parse error', ret); @@ -496,7 +497,6 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { if (!d) d = parser.getCurrentBuffer(); - socket._setEncoding( socket._desiredEncoding ); socket.removeListener('data', state.onData); socket.removeListener('end', state.onEnd); socket.removeListener('close', state.onClose); @@ -518,8 +518,6 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { // Got upgrade header or CONNECT method, but have no handler. socket.destroy(); } - } else if( !parser.incoming.upgrade && socket._desiredEncoding ) { - throw new errors.Error('ERR_METHOD_NOT_IMPLEMENTED', 'setEncoding'); } if (socket._paused && socket.parser) { @@ -665,7 +663,7 @@ function onSocketPause() { } function socketSetEncoding() { - throw new errors.Error('ERR_METHOD_NOT_IMPLEMENTED','setEncoding'); + throw new errors.Error('ERR_METHOD_NOT_IMPLEMENTED', 'setEncoding'); } function unconsume(parser, socket) { diff --git a/test/parallel/test-http-socket-encoding-error.js b/test/parallel/test-http-socket-encoding-error.js index e3ab5cd3eb5fdc..cfb96f4f81bc2e 100644 --- a/test/parallel/test-http-socket-encoding-error.js +++ b/test/parallel/test-http-socket-encoding-error.js @@ -1,11 +1,11 @@ 'use strict'; -const common = require( '../common' ); +const common = require('../common'); const http = require('http'); const server = http.createServer().listen(0, connectToServer); -server.on('connection', socket => { +server.on('connection', (socket) => { common.expectsError(() => socket.setEncoding(''), { code: 'ERR_METHOD_NOT_IMPLEMENTED',