Skip to content

Commit

Permalink
net: ensure net.connect calls Socket connect
Browse files Browse the repository at this point in the history
It's important for people who monkey-patch `Socket.prototype.connect`
that it's called by `net.connect` since it's not possible to
monkey-patch `net.connect` directly (as the `connect` function is called
directly by other parts of `lib/net.js` instead of calling the
`exports.connect` function).

Among the actors who monkey-patch `Socket.prototype.connect` are most
APM vendors, the async-listener module and the
continuation-local-storage module.

Related:
- #12342
- #12852

PR-URL: #12861
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luca Maraschi <luca.maraschi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
  • Loading branch information
watson authored and sam-github committed May 8, 2017
1 parent 65d6249 commit 212a7a6
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 14 deletions.
32 changes: 18 additions & 14 deletions lib/net.js
Expand Up @@ -87,15 +87,14 @@ function connect() {
// TODO(joyeecheung): use destructuring when V8 is fast enough // TODO(joyeecheung): use destructuring when V8 is fast enough
var normalized = normalizeArgs(args); var normalized = normalizeArgs(args);
var options = normalized[0]; var options = normalized[0];
var cb = normalized[1];
debug('createConnection', normalized); debug('createConnection', normalized);
var socket = new Socket(options); var socket = new Socket(options);


if (options.timeout) { if (options.timeout) {
socket.setTimeout(options.timeout); socket.setTimeout(options.timeout);
} }


return realConnect.call(socket, options, cb); return Socket.prototype.connect.call(socket, normalized);
} }




Expand Down Expand Up @@ -915,18 +914,23 @@ function internalConnect(




Socket.prototype.connect = function() { Socket.prototype.connect = function() {
var args = new Array(arguments.length); let normalized;
for (var i = 0; i < arguments.length; i++) // If passed an array, it's treated as an array of arguments that have
args[i] = arguments[i]; // already been normalized (so we don't normalize more than once). This has
// TODO(joyeecheung): use destructuring when V8 is fast enough // been solved before in https://github.com/nodejs/node/pull/12342, but was
var normalized = normalizeArgs(args); // reverted as it had unintended side effects.
var options = normalized[0]; if (arguments.length === 1 && Array.isArray(arguments[0])) {
var cb = normalized[1]; normalized = arguments[0];
return realConnect.call(this, options, cb); } else {
}; var args = new Array(arguments.length);

for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
// TODO(joyeecheung): use destructuring when V8 is fast enough
normalized = normalizeArgs(args);
}
const options = normalized[0];
const cb = normalized[1];


function realConnect(options, cb) {
if (this.write !== Socket.prototype.write) if (this.write !== Socket.prototype.write)
this.write = Socket.prototype.write; this.write = Socket.prototype.write;


Expand Down Expand Up @@ -967,7 +971,7 @@ function realConnect(options, cb) {
lookupAndConnect(this, options); lookupAndConnect(this, options);
} }
return this; return this;
} };




function lookupAndConnect(self, options) { function lookupAndConnect(self, options) {
Expand Down
39 changes: 39 additions & 0 deletions test/parallel/test-net-connect-call-socket-connect.js
@@ -0,0 +1,39 @@
'use strict';
const common = require('../common');

// This test checks that calling `net.connect` internally calls
// `Socket.prototype.connect`.
//
// This is important for people who monkey-patch `Socket.prototype.connect`
// since it's not possible to monkey-patch `net.connect` directly (as the core
// `connect` function is called internally in Node instead of calling the
// `exports.connect` function).
//
// Monkey-patching of `Socket.prototype.connect` is done by - among others -
// most APM vendors, the async-listener module and the
// continuation-local-storage module.
//
// Related:
// - https://github.com/nodejs/node/pull/12342
// - https://github.com/nodejs/node/pull/12852

const net = require('net');
const Socket = net.Socket;

// Monkey patch Socket.prototype.connect to check that it's called.
const orig = Socket.prototype.connect;
Socket.prototype.connect = common.mustCall(function() {
return orig.apply(this, arguments);
});

const server = net.createServer();

server.listen(common.mustCall(function() {
const port = server.address().port;
const client = net.connect({port}, common.mustCall(function() {
client.end();
}));
client.on('end', common.mustCall(function() {
server.close();
}));
}));

0 comments on commit 212a7a6

Please sign in to comment.