Skip to content

Commit

Permalink
http: fix socketOnWrap edge cases
Browse files Browse the repository at this point in the history
Properly handle prependListener wrapping on http server
socket, in addition to on and addListener.

PR-URL: #27968
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
apapirovski authored and targos committed Jun 2, 2019
1 parent 70b4854 commit 24eaeed
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 35 deletions.
31 changes: 17 additions & 14 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,10 @@ function connectionListenerInternal(server, socket) {
socket.on('resume', onSocketResume);
socket.on('pause', onSocketPause);

// Override on to unconsume on `data`, `readable` listeners
socket.on = socketOnWrap;
socket.addListener = socket.on;
// Overrides to unconsume on `data`, `readable` listeners
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 &&
Expand Down Expand Up @@ -757,19 +758,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() {
Expand Down
41 changes: 20 additions & 21 deletions test/parallel/test-http-server-unconsume.js
Original file line number Diff line number Diff line change
@@ -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');
});

0 comments on commit 24eaeed

Please sign in to comment.