From 0073954f33bbbb2e7f279db561b0335beba4715c Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Thu, 13 Dec 2018 13:39:26 -0800 Subject: [PATCH 1/3] test: merge test with unnecessary child process Test didn't require child process creation. While this test has not been unstable, child process creation is slower and can be flaky in ci, so test directly for the segfault regression. --- test/parallel/test-crypto-hash.js | 11 ++++++++ test/parallel/test-crypto-hmac.js | 10 +++++++ .../parallel/test-crypto-tostring-segfault.js | 27 ------------------- 3 files changed, 21 insertions(+), 27 deletions(-) delete mode 100644 test/parallel/test-crypto-tostring-segfault.js diff --git a/test/parallel/test-crypto-hash.js b/test/parallel/test-crypto-hash.js index ec35cad7e8960c..f0def3fe8c2f30 100644 --- a/test/parallel/test-crypto-hash.js +++ b/test/parallel/test-crypto-hash.js @@ -111,6 +111,17 @@ assert.throws(function() { crypto.createHash('xyzzy'); }, /Digest method not supported/); +// Issue https://github.com/nodejs/node/issues/9819: throwing encoding used to +// segfault. +common.expectsError( + () => crypto.createHash('sha256').digest({ + toString: () => { throw new Error("boom"); }, + }), + { + type: Error, + message: 'boom' + }); + // Default UTF-8 encoding const hutf8 = crypto.createHash('sha512').update('УТФ-8 text').digest('hex'); assert.strictEqual( diff --git a/test/parallel/test-crypto-hmac.js b/test/parallel/test-crypto-hmac.js index 223b5c3c077251..0928d390634b23 100644 --- a/test/parallel/test-crypto-hmac.js +++ b/test/parallel/test-crypto-hmac.js @@ -21,6 +21,16 @@ common.expectsError( message: 'The "hmac" argument must be of type string. Received type object' }); +// This used to segfault. See: https://github.com/nodejs/node/issues/9819 +common.expectsError( + () => crypto.createHmac('sha256', 'key').digest({ + toString: () => { throw new Error("boom"); }, + }), + { + type: Error, + message: 'boom' + }); + common.expectsError( () => crypto.createHmac('sha1', null), { diff --git a/test/parallel/test-crypto-tostring-segfault.js b/test/parallel/test-crypto-tostring-segfault.js deleted file mode 100644 index b2c95117d3b1f8..00000000000000 --- a/test/parallel/test-crypto-tostring-segfault.js +++ /dev/null @@ -1,27 +0,0 @@ -'use strict'; -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); - -// This test ensures that node doesn't SEGFAULT when either of -// `crypto.createHash` or `crypto.createHmac` are given an object that defines -// a throwing `toString`. -// https://github.com/nodejs/node/issues/9819 - -const assert = require('assert'); -const execFile = require('child_process').execFile; - -const setup = 'const enc = { toString: () => { throw new Error("xyz"); } };'; - -const scripts = [ - 'crypto.createHash("sha256").digest(enc)', - 'crypto.createHmac("sha256", "msg").digest(enc)' -]; - -scripts.forEach((script) => { - const node = process.execPath; - const code = `${setup};${script}`; - execFile(node, [ '-e', code ], common.mustCall((err, stdout, stderr) => { - assert(stderr.includes('Error: xyz'), 'digest crashes'); - })); -}); From 081381bdfd9969dc519569619763e5435a647920 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Fri, 14 Dec 2018 09:13:01 -0800 Subject: [PATCH 2/3] fixup! test: merge test with unnecessary child process --- test/parallel/test-crypto-hash.js | 2 +- test/parallel/test-crypto-hmac.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-crypto-hash.js b/test/parallel/test-crypto-hash.js index f0def3fe8c2f30..5cc622b48c76c4 100644 --- a/test/parallel/test-crypto-hash.js +++ b/test/parallel/test-crypto-hash.js @@ -115,7 +115,7 @@ assert.throws(function() { // segfault. common.expectsError( () => crypto.createHash('sha256').digest({ - toString: () => { throw new Error("boom"); }, + toString: () => { throw new Error('boom'); }, }), { type: Error, diff --git a/test/parallel/test-crypto-hmac.js b/test/parallel/test-crypto-hmac.js index 0928d390634b23..6fd2a0dbc2e589 100644 --- a/test/parallel/test-crypto-hmac.js +++ b/test/parallel/test-crypto-hmac.js @@ -24,7 +24,7 @@ common.expectsError( // This used to segfault. See: https://github.com/nodejs/node/issues/9819 common.expectsError( () => crypto.createHmac('sha256', 'key').digest({ - toString: () => { throw new Error("boom"); }, + toString: () => { throw new Error('boom'); }, }), { type: Error, From 421539012fe7ab02a97ba003b6e98a7eb904c78a Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Fri, 14 Dec 2018 09:15:56 -0800 Subject: [PATCH 3/3] fixup! fixup! test: merge test with unnecessary child process --- test/parallel/test-crypto-hmac.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-crypto-hmac.js b/test/parallel/test-crypto-hmac.js index 6fd2a0dbc2e589..f21db29d36107f 100644 --- a/test/parallel/test-crypto-hmac.js +++ b/test/parallel/test-crypto-hmac.js @@ -24,8 +24,8 @@ common.expectsError( // This used to segfault. See: https://github.com/nodejs/node/issues/9819 common.expectsError( () => crypto.createHmac('sha256', 'key').digest({ - toString: () => { throw new Error('boom'); }, - }), + toString: () => { throw new Error('boom'); }, + }), { type: Error, message: 'boom'