From 787c310f5810e77a038ecdd742f776cd0dfc857b Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Wed, 27 Nov 2024 21:25:18 +0300 Subject: [PATCH] test: migrate tests to use node:test module for better test structure --- test/parallel/test-worker-heapdump-failure.js | 49 ++--- ...-transfer-fake-js-transferable-internal.js | 79 +++++--- ...sage-port-transfer-fake-js-transferable.js | 64 ++++--- ...worker-message-port-transfer-filehandle.js | 176 +++++++++--------- .../test-worker-stack-overflow-stack-size.js | 98 +++++----- 5 files changed, 253 insertions(+), 213 deletions(-) diff --git a/test/parallel/test-worker-heapdump-failure.js b/test/parallel/test-worker-heapdump-failure.js index c5d24cdcf658a2..1dd8556c1c2111 100644 --- a/test/parallel/test-worker-heapdump-failure.js +++ b/test/parallel/test-worker-heapdump-failure.js @@ -1,30 +1,37 @@ 'use strict'; + const common = require('../common'); -const assert = require('assert'); -const { Worker } = require('worker_threads'); -const { once } = require('events'); -(async function() { - const w = new Worker('', { eval: true }); +const { test } = require('node:test'); +const { Worker } = require('node:worker_threads'); +const { once } = require('node:events'); +const assert = require('node:assert'); + +// Test for ERR_WORKER_NOT_RUNNING when calling getHeapSnapshot on a worker that's not running +test('Worker heap snapshot tests', async (t) => { + await t.test('should throw ERR_WORKER_NOT_RUNNING when worker is not running', async () => { + const w = new Worker('', { eval: true }); - await once(w, 'exit'); - await assert.rejects(() => w.getHeapSnapshot(), { - name: 'Error', - code: 'ERR_WORKER_NOT_RUNNING' + await once(w, 'exit'); + await assert.rejects(() => w.getHeapSnapshot(), { + name: 'Error', + code: 'ERR_WORKER_NOT_RUNNING', + }); }); -})().then(common.mustCall()); -(async function() { - const worker = new Worker('setInterval(() => {}, 1000);', { eval: true }); - await once(worker, 'online'); + await t.test('should throw ERR_INVALID_ARG_TYPE for invalid options argument in getHeapSnapshot', async () => { + const worker = new Worker('setInterval(() => {}, 1000);', { eval: true }); + await once(worker, 'online'); - [1, true, [], null, Infinity, NaN].forEach((i) => { - assert.throws(() => worker.getHeapSnapshot(i), { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError', - message: 'The "options" argument must be of type object.' + - common.invalidArgTypeHelper(i) + // Test various invalid types for the `options` argument + [1, true, [], null, Infinity, NaN].forEach((i) => { + assert.throws(() => worker.getHeapSnapshot(i), { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError', + message: `The "options" argument must be of type object.${common.invalidArgTypeHelper(i)}`, + }); }); + + await worker.terminate(); }); - await worker.terminate(); -})().then(common.mustCall()); +}); diff --git a/test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js b/test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js index 87e34f054b24f7..2bad6b5eb3b4a5 100644 --- a/test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js +++ b/test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js @@ -1,33 +1,50 @@ 'use strict'; + const common = require('../common'); -const assert = require('assert'); -const fs = require('fs').promises; -const { MessageChannel } = require('worker_threads'); -const { once } = require('events'); - -// Test that overriding the internal kTransfer method of a JSTransferable does -// not enable loading arbitrary code from internal Node.js core modules. - -(async function() { - const fh = await fs.open(__filename); - assert.strictEqual(fh.constructor.name, 'FileHandle'); - - const kTransfer = Object.getOwnPropertySymbols(Object.getPrototypeOf(fh)) - .filter((symbol) => symbol.description === 'messaging_transfer_symbol')[0]; - assert.strictEqual(typeof kTransfer, 'symbol'); - fh[kTransfer] = () => { - return { - data: '✨', - deserializeInfo: 'net:Socket' - }; - }; - - const { port1, port2 } = new MessageChannel(); - port1.postMessage(fh, [ fh ]); - port2.on('message', common.mustNotCall()); - - const [ exception ] = await once(port2, 'messageerror'); - - assert.strictEqual(exception.message, 'Unknown deserialize spec net:Socket'); - port2.close(); -})().then(common.mustCall()); +const fs = require('node:fs').promises; +const assert = require('node:assert'); + +const { test } = require('node:test'); +const { MessageChannel } = require('node:worker_threads'); +const { once } = require('node:events'); + +// Test: Overriding kTransfer method should not enable loading arbitrary code from Node.js core modules. +test('Security test for overriding kTransfer method', async (t) => { + await t.test( + 'should throw an error for invalid deserializeInfo from core module', + async () => { + const fh = await fs.open(__filename); + assert.strictEqual(fh.constructor.name, 'FileHandle'); + + const kTransfer = Object.getOwnPropertySymbols( + Object.getPrototypeOf(fh), + ).find((symbol) => symbol.description === 'messaging_transfer_symbol'); + assert.strictEqual(typeof kTransfer, 'symbol'); + + // Override the kTransfer method + fh[kTransfer] = () => ({ + data: '✨', + deserializeInfo: 'net:Socket', // Attempt to load an internal module + }); + + const { port1, port2 } = new MessageChannel(); + port1.postMessage(fh, [fh]); + + // Verify that no valid message is processed + port2.on('message', common.mustNotCall()); + + // Capture the 'messageerror' event and validate the error + const [exception] = await once(port2, 'messageerror'); + const errorMessage = + 'Unexpected error message for invalid deserializeInfo'; + + assert.strictEqual( + exception.message, + 'Unknown deserialize spec net:Socket', + errorMessage, + ); + + port2.close(); + }, + ); +}); diff --git a/test/parallel/test-worker-message-port-transfer-fake-js-transferable.js b/test/parallel/test-worker-message-port-transfer-fake-js-transferable.js index 494adbb9e2bc7c..9022f8d7a62c9c 100644 --- a/test/parallel/test-worker-message-port-transfer-fake-js-transferable.js +++ b/test/parallel/test-worker-message-port-transfer-fake-js-transferable.js @@ -1,37 +1,39 @@ 'use strict'; + const common = require('../common'); -const assert = require('assert'); -const fs = require('fs').promises; -const { MessageChannel } = require('worker_threads'); -const { once } = require('events'); - -// Test that overriding the internal kTransfer method of a JSTransferable does -// not enable loading arbitrary code from the disk. - -module.exports = { - NotARealClass: common.mustNotCall() -}; - -(async function() { - const fh = await fs.open(__filename); - assert.strictEqual(fh.constructor.name, 'FileHandle'); - - const kTransfer = Object.getOwnPropertySymbols(Object.getPrototypeOf(fh)) - .filter((symbol) => symbol.description === 'messaging_transfer_symbol')[0]; - assert.strictEqual(typeof kTransfer, 'symbol'); - fh[kTransfer] = () => { - return { +const assert = require('node:assert'); +const fs = require('node:fs').promises; + +const { test } = require('node:test'); +const { MessageChannel } = require('node:worker_threads'); +const { once } = require('node:events'); + +// Test: Overriding the internal kTransfer method should not enable arbitrary code loading. +test('Overriding kTransfer method security test', async (t) => { + await t.test('should throw an error for invalid deserializeInfo', async () => { + const fh = await fs.open(__filename); + assert.strictEqual(fh.constructor.name, 'FileHandle'); + + const kTransfer = Object.getOwnPropertySymbols(Object.getPrototypeOf(fh)) + .find((symbol) => symbol.description === 'messaging_transfer_symbol'); + assert.strictEqual(typeof kTransfer, 'symbol'); + + // Override the kTransfer method + fh[kTransfer] = () => ({ data: '✨', - deserializeInfo: `${__filename}:NotARealClass` - }; - }; + deserializeInfo: `${__filename}:NotARealClass`, + }); + + const { port1, port2 } = new MessageChannel(); + port1.postMessage(fh, [fh]); - const { port1, port2 } = new MessageChannel(); - port1.postMessage(fh, [ fh ]); - port2.on('message', common.mustNotCall()); + // Verify that no valid message is processed + port2.on('message', common.mustNotCall()); - const [ exception ] = await once(port2, 'messageerror'); + // Capture the 'messageerror' event and validate the error + const [exception] = await once(port2, 'messageerror'); + assert.match(exception.message, /Missing internal module/, 'Unexpected error message'); - assert.match(exception.message, /Missing internal module/); - port2.close(); -})().then(common.mustCall()); + port2.close(); + }); +}); diff --git a/test/parallel/test-worker-message-port-transfer-filehandle.js b/test/parallel/test-worker-message-port-transfer-filehandle.js index 3e6afe22a8c636..d96a8f17870778 100644 --- a/test/parallel/test-worker-message-port-transfer-filehandle.js +++ b/test/parallel/test-worker-message-port-transfer-filehandle.js @@ -1,105 +1,107 @@ 'use strict'; -const common = require('../common'); -const assert = require('assert'); -const fs = require('fs').promises; -const vm = require('vm'); -const { MessageChannel, moveMessagePortToContext } = require('worker_threads'); -const { once } = require('events'); - -(async function() { - const fh = await fs.open(__filename); - - const { port1, port2 } = new MessageChannel(); - - assert.throws(() => { - port1.postMessage(fh); - }, { - constructor: DOMException, - name: 'DataCloneError', - code: 25, - }); - - // Check that transferring FileHandle instances works. - assert.notStrictEqual(fh.fd, -1); - port1.postMessage(fh, [ fh ]); - assert.strictEqual(fh.fd, -1); - - const [ fh2 ] = await once(port2, 'message'); - assert.strictEqual(Object.getPrototypeOf(fh2), Object.getPrototypeOf(fh)); - - assert.deepStrictEqual(await fh2.readFile(), await fs.readFile(__filename)); - await fh2.close(); - await assert.rejects(() => fh.readFile(), { code: 'EBADF' }); -})().then(common.mustCall()); - -(async function() { - // Check that there is no crash if the message is never read. - const fh = await fs.open(__filename); +require('../common'); + +const { test } = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs').promises; +const vm = require('node:vm'); +const { MessageChannel, moveMessagePortToContext } = require('node:worker_threads'); +const { once } = require('node:events'); + +test('FileHandle transfer behavior', async (t) => { + await t.test('should throw when posting a FileHandle without transferring', async () => { + const fh = await fs.open(__filename); + const { port1 } = new MessageChannel(); + + assert.throws(() => { + port1.postMessage(fh); + }, { + constructor: DOMException, + name: 'DataCloneError', + code: 25, + }); + + await fh.close(); + }); - const { port1 } = new MessageChannel(); + await t.test('should transfer a FileHandle successfully', async () => { + const fh = await fs.open(__filename); + const { port1, port2 } = new MessageChannel(); - assert.notStrictEqual(fh.fd, -1); - port1.postMessage(fh, [ fh ]); - assert.strictEqual(fh.fd, -1); -})().then(common.mustCall()); + assert.notStrictEqual(fh.fd, -1); + port1.postMessage(fh, [fh]); + assert.strictEqual(fh.fd, -1); -(async function() { - // Check that in the case of a context mismatch the message is discarded. - const fh = await fs.open(__filename); + const [fh2] = await once(port2, 'message'); + assert.strictEqual(Object.getPrototypeOf(fh2), Object.getPrototypeOf(fh)); - const { port1, port2 } = new MessageChannel(); + assert.deepStrictEqual(await fh2.readFile(), await fs.readFile(__filename)); + await fh2.close(); - const ctx = vm.createContext(); - const port2moved = moveMessagePortToContext(port2, ctx); - port2moved.onmessage = common.mustCall((msgEvent) => { - assert.strictEqual(msgEvent.data, 'second message'); - port1.close(); - }); - // TODO(addaleax): Switch this to a 'messageerror' event once MessagePort - // implements EventTarget fully and in a cross-context manner. - port2moved.onmessageerror = common.mustCall((event) => { - assert.strictEqual(event.data.code, - 'ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE'); + await assert.rejects(() => fh.readFile(), { code: 'EBADF' }); }); - port2moved.start(); - assert.notStrictEqual(fh.fd, -1); - port1.postMessage(fh, [ fh ]); - assert.strictEqual(fh.fd, -1); + await t.test('should not crash if the message is never read', async () => { + const fh = await fs.open(__filename); + const { port1 } = new MessageChannel(); - port1.postMessage('second message'); -})().then(common.mustCall()); - -(async function() { - // Check that a FileHandle with a read in progress cannot be transferred. - const fh = await fs.open(__filename); - - const { port1 } = new MessageChannel(); + assert.notStrictEqual(fh.fd, -1); + port1.postMessage(fh, [fh]); + assert.strictEqual(fh.fd, -1); + }); - const readPromise = fh.readFile(); - assert.throws(() => { + await t.test('should discard message with context mismatch', async () => { + const fh = await fs.open(__filename); + const { port1, port2 } = new MessageChannel(); + + const ctx = vm.createContext(); + const port2moved = moveMessagePortToContext(port2, ctx); + port2moved.onmessage = (msgEvent) => { + assert.strictEqual(msgEvent.data, 'second message'); + port1.close(); + }; + port2moved.onmessageerror = (event) => { + assert.strictEqual( + event.data.code, + 'ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE' + ); + }; + port2moved.start(); + + assert.notStrictEqual(fh.fd, -1); port1.postMessage(fh, [fh]); - }, { - message: 'Cannot transfer FileHandle while in use', - name: 'DataCloneError' + assert.strictEqual(fh.fd, -1); + + port1.postMessage('second message'); }); - assert.deepStrictEqual(await readPromise, await fs.readFile(__filename)); -})().then(common.mustCall()); + await t.test('should not transfer FileHandle with read in progress', async () => { + const fh = await fs.open(__filename); + const { port1 } = new MessageChannel(); -(async function() { - // Check that filehandles with a close in progress cannot be transferred. - const fh = await fs.open(__filename); + const readPromise = fh.readFile(); + assert.throws(() => { + port1.postMessage(fh, [fh]); + }, { + message: 'Cannot transfer FileHandle while in use', + name: 'DataCloneError', + }); - const { port1 } = new MessageChannel(); + assert.deepStrictEqual(await readPromise, await fs.readFile(__filename)); + }); - const closePromise = fh.close(); - assert.throws(() => { - port1.postMessage(fh, [fh]); - }, { - message: 'Cannot transfer FileHandle while in use', - name: 'DataCloneError' + await t.test('should not transfer FileHandle with close in progress', async () => { + const fh = await fs.open(__filename); + const { port1 } = new MessageChannel(); + + const closePromise = fh.close(); + assert.throws(() => { + port1.postMessage(fh, [fh]); + }, { + message: 'Cannot transfer FileHandle while in use', + name: 'DataCloneError', + }); + await closePromise; }); - await closePromise; -})().then(common.mustCall()); +}); diff --git a/test/parallel/test-worker-stack-overflow-stack-size.js b/test/parallel/test-worker-stack-overflow-stack-size.js index 481ff55100ac76..278d80e1a8920b 100644 --- a/test/parallel/test-worker-stack-overflow-stack-size.js +++ b/test/parallel/test-worker-stack-overflow-stack-size.js @@ -1,63 +1,75 @@ 'use strict'; -const common = require('../common'); -const assert = require('assert'); -const { once } = require('events'); -const v8 = require('v8'); -const { Worker } = require('worker_threads'); -// Verify that Workers don't care about --stack-size, as they have their own -// fixed and known stack sizes. +require('../common'); + +const { test } = require('node:test'); +const { once } = require('node:events'); +const assert = require('node:assert'); +const v8 = require('node:v8'); +const { Worker } = require('node:worker_threads'); async function runWorker(options = {}) { const empiricalStackDepth = new Uint32Array(new SharedArrayBuffer(4)); - const worker = new Worker(` - const { workerData: { empiricalStackDepth } } = require('worker_threads'); - function f() { - empiricalStackDepth[0]++; - f(); - } - f();`, { - eval: true, - workerData: { empiricalStackDepth }, - ...options - }); + const worker = new Worker( + ` + const { workerData: { empiricalStackDepth } } = require('worker_threads'); + function f() { + empiricalStackDepth[0]++; + f(); + } + f();`, + { + eval: true, + workerData: { empiricalStackDepth }, + ...options, + }, + ); - const [ error ] = await once(worker, 'error'); + const [error] = await once(worker, 'error'); if (!options.skipErrorCheck) { - common.expectsError({ - constructor: RangeError, - message: 'Maximum call stack size exceeded' - })(error); + const expectedErrorMessage = 'Expected a RangeError'; + const unexpectedErrorMessage = 'Unexpected error message'; + + assert.strictEqual(error.constructor, RangeError, expectedErrorMessage); + assert.match( + error.message, + /Maximum call stack size exceeded/, + unexpectedErrorMessage, + ); } return empiricalStackDepth[0]; } -(async function() { - { +test('Worker threads stack size tests', async (t) => { + await t.test('Workers are unaffected by --stack-size flag', async () => { v8.setFlagsFromString('--stack-size=500'); const w1stack = await runWorker(); v8.setFlagsFromString('--stack-size=1000'); const w2stack = await runWorker(); - // Make sure the two stack sizes are within 10 % of each other, i.e. not - // affected by the different `--stack-size` settings. - assert(Math.max(w1stack, w2stack) / Math.min(w1stack, w2stack) < 1.1, - `w1stack = ${w1stack}, w2stack = ${w2stack} are too far apart`); - } - { + assert( + Math.max(w1stack, w2stack) / Math.min(w1stack, w2stack) < 1.1, + `w1stack = ${w1stack}, w2stack = ${w2stack} are too far apart`, + ); + }); + + await t.test('Workers respect resourceLimits.stackSizeMb', async () => { const w1stack = await runWorker({ resourceLimits: { stackSizeMb: 0.5 } }); const w2stack = await runWorker({ resourceLimits: { stackSizeMb: 1.0 } }); - // Make sure the two stack sizes are at least 40 % apart from each other, - // i.e. affected by the different `stackSizeMb` settings. - assert(w2stack > w1stack * 1.4, - `w1stack = ${w1stack}, w2stack = ${w2stack} are too close`); - } - // Test that various low stack sizes result in an 'error' event. - // Currently the stack size needs to be at least 0.3MB for the worker to be - // bootstrapped properly. - for (const stackSizeMb of [ 0.3, 0.5, 1 ]) { - await runWorker({ resourceLimits: { stackSizeMb }, skipErrorCheck: true }); - } -})().then(common.mustCall()); + assert( + w2stack > w1stack * 1.4, + `w1stack = ${w1stack}, w2stack = ${w2stack} are too close`, + ); + }); + + await t.test('Low stack sizes result in error events', async () => { + for (const stackSizeMb of [0.3, 0.5, 1]) { + await runWorker({ + resourceLimits: { stackSizeMb }, + skipErrorCheck: true, + }); + } + }); +});