diff --git a/lib/net.js b/lib/net.js index d50b3729d968a9..f1061922ca6150 100644 --- a/lib/net.js +++ b/lib/net.js @@ -87,7 +87,6 @@ function connect() { // TODO(joyeecheung): use destructuring when V8 is fast enough var normalized = normalizeArgs(args); var options = normalized[0]; - var cb = normalized[1]; debug('createConnection', normalized); var socket = new Socket(options); @@ -95,7 +94,7 @@ function connect() { socket.setTimeout(options.timeout); } - return realConnect.call(socket, options, cb); + return Socket.prototype.connect.call(socket, normalized); } @@ -915,18 +914,23 @@ function internalConnect( Socket.prototype.connect = function() { - 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 - var normalized = normalizeArgs(args); - var options = normalized[0]; - var cb = normalized[1]; - return realConnect.call(this, options, cb); -}; - + let normalized; + // If passed an array, it's treated as an array of arguments that have + // already been normalized (so we don't normalize more than once). This has + // been solved before in https://github.com/nodejs/node/pull/12342, but was + // reverted as it had unintended side effects. + if (arguments.length === 1 && Array.isArray(arguments[0])) { + normalized = arguments[0]; + } 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) this.write = Socket.prototype.write; @@ -967,7 +971,7 @@ function realConnect(options, cb) { lookupAndConnect(this, options); } return this; -} +}; function lookupAndConnect(self, options) { diff --git a/test/parallel/test-net-connect-call-socket-connect.js b/test/parallel/test-net-connect-call-socket-connect.js new file mode 100644 index 00000000000000..8d3413d9aa7f33 --- /dev/null +++ b/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(); + })); +}));