From 20d05f4d309b9ed65a77e64099c5e53c1a73ede0 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 30 May 2019 11:32:51 +0200 Subject: [PATCH 1/2] http: fix socketOnWrap edge cases Properly handle prependListener wrapping on http server socket, in addition to on and addListener. --- lib/_http_server.js | 29 ++++++++------- test/parallel/test-http-server-unconsume.js | 41 ++++++++++----------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index 62b58dd2e1e000..b13c417667b738 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -405,8 +405,9 @@ function connectionListenerInternal(server, socket) { socket.on('pause', onSocketPause); // Override on to unconsume on `data`, `readable` listeners - socket.on = socketOnWrap; - socket.addListener = socket.on; + socket.on = generateSocketListenerWrapper('on'); + socket.addListener = generateSocketListenerWrapper('addListener'); + socket.prependListener = generateSocketListenerWrapper('prependListener'); // We only consume the socket if it has never been consumed before. if (socket._handle && socket._handle.isStreamBase && @@ -754,19 +755,21 @@ function unconsume(parser, socket) { } } -function socketOnWrap(ev, fn) { - const res = net.Socket.prototype.on.call(this, ev, fn); - if (!this.parser) { - this.prependListener = net.Socket.prototype.prependListener; - this.on = net.Socket.prototype.on; - this.addListener = this.on; - return res; - } +function generateSocketListenerWrapper(originalFnName) { + return function socketListenerWrap(ev, fn) { + const res = net.Socket.prototype[originalFnName].call(this, ev, fn); + if (!this.parser) { + this.on = net.Socket.prototype.on; + this.addListener = net.Socket.prototype.addListener; + this.prependListener = net.Socket.prototype.prependListener; + return res; + } - if (ev === 'data' || ev === 'readable') - unconsume(this.parser, this); + if (ev === 'data' || ev === 'readable') + unconsume(this.parser, this); - return res; + return res; + }; } function resetHeadersTimeoutOnReqEnd() { diff --git a/test/parallel/test-http-server-unconsume.js b/test/parallel/test-http-server-unconsume.js index c285a53c7ac215..a8a307e53b5a30 100644 --- a/test/parallel/test-http-server-unconsume.js +++ b/test/parallel/test-http-server-unconsume.js @@ -1,34 +1,33 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const http = require('http'); const net = require('net'); -let received = ''; +['on', 'addListener', 'prependListener'].forEach((testFn) => { + let received = ''; -const server = http.createServer(function(req, res) { - res.writeHead(200); - res.end(); + const server = http.createServer(function(req, res) { + res.writeHead(200); + res.end(); - req.socket.on('data', function(data) { - received += data; - }); + req.socket[testFn]('data', function(data) { + received += data; + }); - assert.strictEqual(req.socket.on, req.socket.addListener); - assert.strictEqual(req.socket.prependListener, - net.Socket.prototype.prependListener); + server.close(); + }).listen(0, function() { + const socket = net.connect(this.address().port, function() { + socket.write('PUT / HTTP/1.1\r\n\r\n'); - server.close(); -}).listen(0, function() { - const socket = net.connect(this.address().port, function() { - socket.write('PUT / HTTP/1.1\r\n\r\n'); + socket.once('data', function() { + socket.end('hello world'); + }); - socket.once('data', function() { - socket.end('hello world'); + socket.on('end', common.mustCall(() => { + assert.strictEqual(received, 'hello world', + `failed for socket.${testFn}`); + })); }); }); }); - -process.on('exit', function() { - assert.strictEqual(received, 'hello world'); -}); From e1edbd6ed41225625c8cd5c8acbfc0b9289a7239 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Fri, 31 May 2019 10:45:01 +0200 Subject: [PATCH 2/2] fixup: clarify comment --- lib/_http_server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index b13c417667b738..39a49fedc39c1a 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -404,7 +404,7 @@ function connectionListenerInternal(server, socket) { socket.on('resume', onSocketResume); socket.on('pause', onSocketPause); - // Override on to unconsume on `data`, `readable` listeners + // Overrides to unconsume on `data`, `readable` listeners socket.on = generateSocketListenerWrapper('on'); socket.addListener = generateSocketListenerWrapper('addListener'); socket.prependListener = generateSocketListenerWrapper('prependListener');