Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(v6.x backport) tools: enforce two arguments in assert.throws #13785

Closed
wants to merge 11 commits into from
2 changes: 1 addition & 1 deletion .eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ rules:

# Custom rules in tools/eslint-rules
align-multiline-assignment: 2
assert-throws-arguments: [2, { requireTwo: false }]
assert-throws-arguments: [2, { requireTwo: true }]
no-useless-regex-char-class-escape: [2, { override: ['[', ']'] }]

# Global scoped method and vars
Expand Down
14 changes: 10 additions & 4 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ assert.throws(
{
const re1 = /a/;
re1.lastIndex = 3;
assert.throws(makeBlock(a.deepStrictEqual, re1, /a/));
assert.throws(makeBlock(a.deepStrictEqual, re1, /a/),
/^AssertionError: \/a\/ deepStrictEqual \/a\/$/);
}

// 7.4 - strict
Expand All @@ -262,10 +263,12 @@ assert.doesNotThrow(makeBlock(a.deepStrictEqual, {a: 4}, {a: 4}));
assert.doesNotThrow(makeBlock(a.deepStrictEqual,
{a: 4, b: '2'},
{a: 4, b: '2'}));
assert.throws(makeBlock(a.deepStrictEqual, [4], ['4']));
assert.throws(makeBlock(a.deepStrictEqual, [4], ['4']),
/^AssertionError: \[ 4 ] deepStrictEqual \[ '4' ]$/);
assert.throws(makeBlock(a.deepStrictEqual, {a: 4}, {a: 4, b: true}),
a.AssertionError);
assert.throws(makeBlock(a.deepStrictEqual, ['a'], {0: 'a'}));
/^AssertionError: { a: 4 } deepStrictEqual { a: 4, b: true }$/);
assert.throws(makeBlock(a.deepStrictEqual, ['a'], {0: 'a'}),
/^AssertionError: \[ 'a' ] deepStrictEqual { '0': 'a' }$/);
//(although not necessarily the same order),
assert.doesNotThrow(makeBlock(a.deepStrictEqual,
{a: 4, b: '1'},
Expand Down Expand Up @@ -342,9 +345,11 @@ function thrower(errorConstructor) {
assert.throws(makeBlock(thrower, a.AssertionError),
a.AssertionError, 'message');
assert.throws(makeBlock(thrower, a.AssertionError), a.AssertionError);
// eslint-disable-next-line assert-throws-arguments
assert.throws(makeBlock(thrower, a.AssertionError));

// if not passing an error, catch all.
// eslint-disable-next-line assert-throws-arguments
assert.throws(makeBlock(thrower, TypeError));

// when passing a type, only catch errors of the appropriate type
Expand Down Expand Up @@ -517,6 +522,7 @@ testAssertionMessage({a: NaN, b: Infinity, c: -Infinity},

// #2893
try {
// eslint-disable-next-line assert-throws-arguments
assert.throws(function() {
assert.ifError(null);
});
Expand Down
9 changes: 6 additions & 3 deletions test/parallel/test-buffer-compare.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ assert.strictEqual(Buffer.compare(Buffer.alloc(0), Buffer.alloc(0)), 0);
assert.strictEqual(Buffer.compare(Buffer.alloc(0), Buffer.alloc(1)), -1);
assert.strictEqual(Buffer.compare(Buffer.alloc(1), Buffer.alloc(0)), 1);

assert.throws(() => Buffer.compare(Buffer.alloc(1), 'abc'));
assert.throws(() => Buffer.compare(Buffer.alloc(1), 'abc'),
/^TypeError: Arguments must be Buffers$/);

assert.throws(() => Buffer.compare('abc', Buffer.alloc(1)));
assert.throws(() => Buffer.compare('abc', Buffer.alloc(1)),
/^TypeError: Arguments must be Buffers$/);

assert.throws(() => Buffer.alloc(1).compare('abc'));
assert.throws(() => Buffer.alloc(1).compare('abc'),
/^TypeError: Argument must be a Buffer$/);
3 changes: 2 additions & 1 deletion test/parallel/test-buffer-equals.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ assert.ok(!c.equals(d));
assert.ok(!d.equals(e));
assert.ok(d.equals(d));

assert.throws(() => Buffer.alloc(1).equals('abc'));
assert.throws(() => Buffer.alloc(1).equals('abc'),
/^TypeError: Argument must be a Buffer$/);
16 changes: 9 additions & 7 deletions test/parallel/test-buffer-includes.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,15 +273,17 @@ for (let lengthIndex = 0; lengthIndex < lengths.length; lengthIndex++) {
}
}

assert.throws(function() {
b.includes(function() { });
});
assert.throws(function() {
const expectedError =
/^TypeError: "val" argument must be string, number or Buffer$/;
assert.throws(() => {
b.includes(() => {});
}, expectedError);
assert.throws(() => {
b.includes({});
});
assert.throws(function() {
}, expectedError);
assert.throws(() => {
b.includes([]);
});
}, expectedError);

// test truncation of Number arguments to uint8
{
Expand Down
19 changes: 12 additions & 7 deletions test/parallel/test-buffer-indexof.js
Original file line number Diff line number Diff line change
Expand Up @@ -346,15 +346,20 @@ assert.strictEqual(Buffer.from('aaaaa').indexOf('b', 'ucs2'), -1);
}
}

assert.throws(function() {
b.indexOf(function() { });
});
assert.throws(function() {
const argumentExpected =
/^TypeError: "val" argument must be string, number or Buffer$/;

assert.throws(() => {
b.indexOf(() => { });
}, argumentExpected);

assert.throws(() => {
b.indexOf({});
});
assert.throws(function() {
}, argumentExpected);

assert.throws(() => {
b.indexOf([]);
});
}, argumentExpected);

// Test weird offset arguments.
// The following offsets coerce to NaN or 0, searching the whole Buffer
Expand Down
18 changes: 10 additions & 8 deletions test/parallel/test-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ for (const [key, value] of e.entries()) {

assert.throws(function() {
Buffer(8).fill('a', -1);
});
}, /^RangeError: Out of range index$/);

assert.throws(function() {
Buffer(8).fill('a', 0, 9);
});
}, /^RangeError: Out of range index$/);

// Make sure this doesn't hang indefinitely.
Buffer(8).fill('');
Expand Down Expand Up @@ -1433,17 +1433,17 @@ if (common.hasCrypto) {
assert.throws(function() {
const b = Buffer(1);
Buffer.compare(b, 'abc');
});
}, /^TypeError: Arguments must be Buffers$/);

assert.throws(function() {
const b = Buffer(1);
Buffer.compare('abc', b);
});
}, /^TypeError: Arguments must be Buffers$/);

assert.throws(function() {
const b = Buffer(1);
b.compare('abc');
});
}, /^TypeError: Argument must be a Buffer$/);

// Test Equals
{
Expand All @@ -1461,10 +1461,12 @@ assert.throws(function() {
assert.throws(function() {
const b = Buffer(1);
b.equals('abc');
});
}, /^TypeError: Argument must be a Buffer$/);

// Regression test for https://github.com/nodejs/node/issues/649.
assert.throws(function() { Buffer(1422561062959).toString('utf8'); });
assert.throws(function() {
Buffer(1422561062959).toString('utf8');
}, /^RangeError: Invalid typed array length$/);

const ps = Buffer.poolSize;
Buffer.poolSize = 0;
Expand All @@ -1474,7 +1476,7 @@ Buffer.poolSize = ps;
// Test Buffer.copy() segfault
assert.throws(function() {
Buffer(10).copy();
});
}, /^TypeError: argument should be a Buffer$/);

const regErrorMsg = new RegExp('First argument must be a string, Buffer, ' +
'ArrayBuffer, Array, or array-like object.');
Expand Down
70 changes: 41 additions & 29 deletions test/parallel/test-crypto-dh.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,28 @@ let key2 = dh2.generateKeys('hex');
let secret1 = dh1.computeSecret(key2, 'hex', 'base64');
let secret2 = dh2.computeSecret(key1, 'latin1', 'buffer');

assert.strictEqual(secret1, secret2.toString('base64'));
assert.strictEqual(secret2.toString('base64'), secret1);
assert.strictEqual(dh1.verifyError, 0);
assert.strictEqual(dh2.verifyError, 0);

assert.throws(function() {
const argumentsError =
/^TypeError: First argument should be number, string or Buffer$/;

assert.throws(() => {
crypto.createDiffieHellman([0x1, 0x2]);
});
}, argumentsError);

assert.throws(function() {
crypto.createDiffieHellman(function() { });
});
assert.throws(() => {
crypto.createDiffieHellman(() => { });
}, argumentsError);

assert.throws(function() {
assert.throws(() => {
crypto.createDiffieHellman(/abc/);
});
}, argumentsError);

assert.throws(function() {
assert.throws(() => {
crypto.createDiffieHellman({});
});
}, argumentsError);

// Create "another dh1" using generated keys from dh1,
// and compute secret again
Expand All @@ -56,21 +59,29 @@ const secret3 = dh3.computeSecret(key2, 'hex', 'base64');

assert.strictEqual(secret1, secret3);

const wrongBlockLength =
new RegExp('^Error: error:0606506D:digital envelope' +
' routines:EVP_DecryptFinal_ex:wrong final block length$');

// Run this one twice to make sure that the dh3 clears its error properly
{
const c = crypto.createDecipheriv('aes-128-ecb', crypto.randomBytes(16), '');
assert.throws(function() { c.final('utf8'); }, /wrong final block length/);
assert.throws(() => {
c.final('utf8');
}, wrongBlockLength);
}

assert.throws(function() {
dh3.computeSecret('');
}, /key is too small/i);

{
const c = crypto.createDecipheriv('aes-128-ecb', crypto.randomBytes(16), '');
assert.throws(function() { c.final('utf8'); }, /wrong final block length/);
assert.throws(() => {
c.final('utf8');
}, wrongBlockLength);
}

assert.throws(() => {
dh3.computeSecret('');
}, /^Error: Supplied key is too small$/);

// Create a shared using a DH group.
const alice = crypto.createDiffieHellmanGroup('modp5');
const bob = crypto.createDiffieHellmanGroup('modp5');
Expand Down Expand Up @@ -180,30 +191,31 @@ assert.throws(() => {
const ecdh3 = crypto.createECDH('secp256k1');
const key3 = ecdh3.generateKeys();

assert.throws(function() {
assert.throws(() => {
ecdh2.computeSecret(key3, 'latin1', 'buffer');
});
}, /^Error: Failed to translate Buffer to a EC_POINT$/);

// ECDH should allow .setPrivateKey()/.setPublicKey()
const ecdh4 = crypto.createECDH('prime256v1');

ecdh4.setPrivateKey(ecdh1.getPrivateKey());
ecdh4.setPublicKey(ecdh1.getPublicKey());

assert.throws(function() {
assert.throws(() => {
ecdh4.setPublicKey(ecdh3.getPublicKey());
}, /Failed to convert Buffer to EC_POINT/);
}, /^Error: Failed to convert Buffer to EC_POINT$/);

// Verify that we can use ECDH without having to use newly generated keys.
const ecdh5 = crypto.createECDH('secp256k1');

// Verify errors are thrown when retrieving keys from an uninitialized object.
assert.throws(function() {
assert.throws(() => {
ecdh5.getPublicKey();
}, /Failed to get ECDH public key/);
assert.throws(function() {
}, /^Error: Failed to get ECDH public key$/);

assert.throws(() => {
ecdh5.getPrivateKey();
}, /Failed to get ECDH private key/);
}, /^Error: Failed to get ECDH private key$/);

// A valid private key for the secp256k1 curve.
const cafebabeKey = 'cafebabe'.repeat(8);
Expand Down Expand Up @@ -249,10 +261,10 @@ assert.strictEqual(ecdh5.getPublicKey('hex', 'compressed'), cafebabePubPtComp);
// Show why allowing the public key to be set on this type does not make sense.
ecdh5.setPublicKey(peerPubPtComp, 'hex');
assert.strictEqual(ecdh5.getPublicKey('hex'), peerPubPtUnComp);
assert.throws(function() {
assert.throws(() => {
// Error because the public key does not match the private key anymore.
ecdh5.computeSecret(peerPubPtComp, 'hex', 'hex');
}, /Invalid key pair/);
}, /^Error: Invalid key pair$/);

// Set to a valid key to show that later attempts to set an invalid key are
// rejected.
Expand All @@ -262,10 +274,10 @@ ecdh5.setPrivateKey(cafebabeKey, 'hex');
'0000000000000000000000000000000000000000000000000000000000000000',
'FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141',
'FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF',
].forEach(function(element, index, object) {
assert.throws(function() {
].forEach((element) => {
assert.throws(() => {
ecdh5.setPrivateKey(element, 'hex');
}, /Private key is not valid for specified curve/);
}, /^Error: Private key is not valid for specified curve.$/);
// Verify object state did not change.
assert.strictEqual(ecdh5.getPrivateKey('hex'), cafebabeKey);
});
Expand Down
Loading