Skip to content

Commit

Permalink
net: add symbol to normalized connect() args
Browse files Browse the repository at this point in the history
This commit attaches a Symbol to the result of
net._normalizeArgs(). This prevents normal arrays from being
passed to the internal Socket.prototype.connect() bypass logic.

PR-URL: #13069
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
cjihrig committed May 19, 2017
1 parent 6b1819c commit 51664fc
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 5 deletions.
3 changes: 2 additions & 1 deletion lib/internal/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ function isLegalPort(port) {
}

module.exports = {
isLegalPort
isLegalPort,
normalizedArgsSymbol: Symbol('normalizedArgs')
};
16 changes: 12 additions & 4 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ var dns;
const errnoException = util._errnoException;
const exceptionWithHostPort = util._exceptionWithHostPort;
const isLegalPort = internalNet.isLegalPort;
const normalizedArgsSymbol = internalNet.normalizedArgsSymbol;

function noop() {}

Expand Down Expand Up @@ -118,8 +119,12 @@ function connect() {
// For Server.prototype.listen(), the [...] part is [, backlog]
// but will not be handled here (handled in listen())
function normalizeArgs(args) {
var arr;

if (args.length === 0) {
return [{}, null];
arr = [{}, null];
arr[normalizedArgsSymbol] = true;
return arr;
}

const arg0 = args[0];
Expand All @@ -140,9 +145,12 @@ function normalizeArgs(args) {

var cb = args[args.length - 1];
if (typeof cb !== 'function')
return [options, null];
arr = [options, null];
else
return [options, cb];
arr = [options, cb];

arr[normalizedArgsSymbol] = true;
return arr;
}


Expand Down Expand Up @@ -947,7 +955,7 @@ Socket.prototype.connect = function() {
// 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])) {
if (Array.isArray(arguments[0]) && arguments[0][normalizedArgsSymbol]) {
normalized = arguments[0];
} else {
var args = new Array(arguments.length);
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-net-normalize-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict';
// Flags: --expose-internals
const common = require('../common');
const assert = require('assert');
const net = require('net');
const { normalizedArgsSymbol } = require('internal/net');

function validateNormalizedArgs(input, output) {
const args = net._normalizeArgs(input);

assert.deepStrictEqual(args, output);
assert.strictEqual(args[normalizedArgsSymbol], true);
}

// Test creation of normalized arguments.
validateNormalizedArgs([], [{}, null]);
validateNormalizedArgs([{ port: 1234 }], [{ port: 1234 }, null]);
validateNormalizedArgs([{ port: 1234 }, assert.fail],
[{ port: 1234 }, assert.fail]);

// Connecting to the server should fail with a standard array.
{
const server = net.createServer(common.mustNotCall('should not connect'));

server.listen(common.mustCall(() => {
const possibleErrors = ['ECONNREFUSED', 'EADDRNOTAVAIL'];
const port = server.address().port;
const socket = new net.Socket();

socket.on('error', common.mustCall((err) => {
assert(possibleErrors.includes(err.code));
assert(possibleErrors.includes(err.errno));
assert.strictEqual(err.syscall, 'connect');
server.close();
}));

socket.connect([{ port }, assert.fail]);
}));
}

// Connecting to the server should succeed with a normalized array.
{
const server = net.createServer(common.mustCall((connection) => {
connection.end();
server.close();
}));

server.listen(common.mustCall(() => {
const port = server.address().port;
const socket = new net.Socket();
const args = net._normalizeArgs([{ port }, common.mustCall()]);

socket.connect(args);
}));
}

0 comments on commit 51664fc

Please sign in to comment.