From 54056b674800d2f3729f5c42e152183661501667 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Mon, 3 Apr 2023 17:50:18 -0400 Subject: [PATCH 1/6] wasi: make version non-optional - remove default for version - updates tests to specify version - add test for when version is not specified Signed-off-by: Michael Dawson --- doc/api/wasi.md | 7 +++- lib/wasi.js | 38 +++++++++----------- test/wasi/test-return-on-exit.js | 4 +-- test/wasi/test-wasi-initialize-validation.js | 14 ++++---- test/wasi/test-wasi-not-started.js | 1 + test/wasi/test-wasi-options-validation.js | 21 ++++++----- test/wasi/test-wasi-start-validation.js | 14 ++++---- test/wasi/test-wasi-stdio.js | 2 +- test/wasi/test-wasi-symlinks.js | 1 + test/wasi/test-wasi-worker-terminate.js | 2 +- test/wasi/test-wasi.js | 35 ++---------------- 11 files changed, 56 insertions(+), 83 deletions(-) diff --git a/doc/api/wasi.md b/doc/api/wasi.md index f70470d8dc0732..23537ab31c6961 100644 --- a/doc/api/wasi.md +++ b/doc/api/wasi.md @@ -118,6 +118,10 @@ added: - v13.3.0 - v12.16.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/47391 + description: default removed from version option such that + version much be specified, otherwise error is thrown. - version: v19.8.0 pr-url: https://github.com/nodejs/node/pull/46469 description: version field added to options. @@ -144,7 +148,8 @@ changes: * `stderr` {integer} The file descriptor used as standard error in the WebAssembly application. **Default:** `2`. * `version` {string} The version of WASI requested. Currently the only - supported versions are `unstable` and `preview1`. **Default:** `preview1`. + supported versions are `unstable` and `preview1`. This option + mandatory and there is no default. ### `wasi.getImportObject()` diff --git a/lib/wasi.js b/lib/wasi.js index 78e7025ba338df..c577df7eb0411f 100644 --- a/lib/wasi.js +++ b/lib/wasi.js @@ -48,28 +48,22 @@ class WASI { validateObject(options, 'options'); let _WASI; - if (options.version !== undefined) { - validateString(options.version, 'options.version'); - switch (options.version) { - case 'unstable': - ({ WASI: _WASI } = internalBinding('wasi')); - this[kBindingName] = 'wasi_unstable'; - break; - // When adding support for additional wasi versions add case here - case 'preview1': - ({ WASI: _WASI } = internalBinding('wasi')); - this[kBindingName] = 'wasi_snapshot_preview1'; - break; - // When adding support for additional wasi versions add case here - default: - throw new ERR_INVALID_ARG_VALUE('options.version', - options.version, - 'unsupported WASI version'); - } - } else { - // TODO(mdawson): Remove this in a SemVer major PR before Node.js 20 - ({ WASI: _WASI } = internalBinding('wasi')); - this[kBindingName] = 'wasi_snapshot_preview1'; + validateString(options.version, 'options.version'); + switch (options.version) { + case 'unstable': + ({ WASI: _WASI } = internalBinding('wasi')); + this[kBindingName] = 'wasi_unstable'; + break; + // When adding support for additional wasi versions add case here + case 'preview1': + ({ WASI: _WASI } = internalBinding('wasi')); + this[kBindingName] = 'wasi_snapshot_preview1'; + break; + // When adding support for additional wasi versions add case here + default: + throw new ERR_INVALID_ARG_VALUE('options.version', + options.version, + 'unsupported WASI version'); } if (options.args !== undefined) diff --git a/test/wasi/test-return-on-exit.js b/test/wasi/test-return-on-exit.js index 3956ee02066f27..204e5efbb83682 100644 --- a/test/wasi/test-return-on-exit.js +++ b/test/wasi/test-return-on-exit.js @@ -9,7 +9,7 @@ const modulePath = path.join(wasmDir, 'exitcode.wasm'); const buffer = fs.readFileSync(modulePath); (async () => { - const wasi = new WASI({ returnOnExit: true }); + const wasi = new WASI({ version: 'preview1', returnOnExit: true }); const importObject = { wasi_snapshot_preview1: wasi.wasiImport }; const { instance } = await WebAssembly.instantiate(buffer, importObject); @@ -19,7 +19,7 @@ const buffer = fs.readFileSync(modulePath); (async () => { // Verify that if a WASI application throws an exception, Node rethrows it // properly. - const wasi = new WASI({ returnOnExit: true }); + const wasi = new WASI({ version: 'preview1', returnOnExit: true }); const patchedExit = () => { throw new Error('test error'); }; wasi.wasiImport.proc_exit = patchedExit.bind(wasi.wasiImport); const importObject = { wasi_snapshot_preview1: wasi.wasiImport }; diff --git a/test/wasi/test-wasi-initialize-validation.js b/test/wasi/test-wasi-initialize-validation.js index 11464dcf35229d..63ad4747ec3d53 100644 --- a/test/wasi/test-wasi-initialize-validation.js +++ b/test/wasi/test-wasi-initialize-validation.js @@ -11,7 +11,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); (async () => { { // Verify that a WebAssembly.Instance is passed in. - const wasi = new WASI(); + const wasi = new WASI({ version: 'preview1' }); assert.throws( () => { wasi.initialize(); }, @@ -24,7 +24,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that the passed instance has an exports objects. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); @@ -40,7 +40,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that a _initialize() export was passed. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); @@ -63,7 +63,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that a _start export was not passed. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); @@ -88,7 +88,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that a memory export was passed. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); @@ -106,7 +106,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that a WebAssembly.Instance from another VM context is accepted. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const instance = await vm.runInNewContext(` (async () => { const wasm = await WebAssembly.compile(bufferSource); @@ -130,7 +130,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that initialize() can only be called once. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); diff --git a/test/wasi/test-wasi-not-started.js b/test/wasi/test-wasi-not-started.js index fac42ae193deee..b6c1b538ef96cc 100644 --- a/test/wasi/test-wasi-not-started.js +++ b/test/wasi/test-wasi-not-started.js @@ -8,6 +8,7 @@ if (process.argv[2] === 'wasi-child') { const { WASI } = require('wasi'); const wasi = new WASI({ + version: 'preview1', args: ['foo', '-bar', '--baz=value'], }); const importObject = { wasi_snapshot_preview1: wasi.wasiImport }; diff --git a/test/wasi/test-wasi-options-validation.js b/test/wasi/test-wasi-options-validation.js index 4beb5e8f52a4fb..a1e672032a961c 100644 --- a/test/wasi/test-wasi-options-validation.js +++ b/test/wasi/test-wasi-options-validation.js @@ -5,34 +5,34 @@ const assert = require('assert'); const { WASI } = require('wasi'); // If args is undefined, it should default to [] and should not throw. -new WASI({}); +new WASI({ version: 'preview1' }); // If args is not an Array and not undefined, it should throw. -assert.throws(() => { new WASI({ args: 'fhqwhgads' }); }, +assert.throws(() => { new WASI({ version: 'preview1', args: 'fhqwhgads' }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\bargs\b/ }); // If env is not an Object and not undefined, it should throw. -assert.throws(() => { new WASI({ env: 'fhqwhgads' }); }, +assert.throws(() => { new WASI({ version: 'preview1', env: 'fhqwhgads' }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\benv\b/ }); // If preopens is not an Object and not undefined, it should throw. -assert.throws(() => { new WASI({ preopens: 'fhqwhgads' }); }, +assert.throws(() => { new WASI({ version: 'preview1', preopens: 'fhqwhgads' }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\bpreopens\b/ }); // If returnOnExit is not a boolean and not undefined, it should throw. -assert.throws(() => { new WASI({ returnOnExit: 'fhqwhgads' }); }, +assert.throws(() => { new WASI({ version: 'preview1', returnOnExit: 'fhqwhgads' }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\breturnOnExit\b/ }); // If stdin is not an int32 and not undefined, it should throw. -assert.throws(() => { new WASI({ stdin: 'fhqwhgads' }); }, +assert.throws(() => { new WASI({ version: 'preview1', stdin: 'fhqwhgads' }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\bstdin\b/ }); // If stdout is not an int32 and not undefined, it should throw. -assert.throws(() => { new WASI({ stdout: 'fhqwhgads' }); }, +assert.throws(() => { new WASI({ version: 'preview1', stdout: 'fhqwhgads' }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\bstdout\b/ }); // If stderr is not an int32 and not undefined, it should throw. -assert.throws(() => { new WASI({ stderr: 'fhqwhgads' }); }, +assert.throws(() => { new WASI({ version: 'preview1', stderr: 'fhqwhgads' }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\bstderr\b/ }); // If options is provided, but not an object, the constructor should throw. @@ -43,13 +43,16 @@ assert.throws(() => { new WASI({ stderr: 'fhqwhgads' }); }, // Verify that exceptions thrown from the binding layer are handled. assert.throws(() => { - new WASI({ preopens: { '/sandbox': '__/not/real/path' } }); + new WASI({ version: 'preview1', preopens: { '/sandbox': '__/not/real/path' } }); }, { code: 'UVWASI_ENOENT', message: /uvwasi_init/ }); // If version is not a string, it should throw assert.throws(() => { new WASI({ version: { x: 'y' } }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\bversion\b/ }); +// If version is not specificed, it should throw +assert.throws(() => { new WASI(); }, + { code: 'ERR_INVALID_ARG_TYPE', message: /\bversion\b/ }); // If version is an unsupported version, it should throw assert.throws(() => { new WASI({ version: 'not_a_version' }); }, diff --git a/test/wasi/test-wasi-start-validation.js b/test/wasi/test-wasi-start-validation.js index d856c5c0b8f19c..24f11b0d1e910f 100644 --- a/test/wasi/test-wasi-start-validation.js +++ b/test/wasi/test-wasi-start-validation.js @@ -11,7 +11,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); (async () => { { // Verify that a WebAssembly.Instance is passed in. - const wasi = new WASI(); + const wasi = new WASI({ version: 'preview1' }); assert.throws( () => { wasi.start(); }, @@ -24,7 +24,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that the passed instance has an exports objects. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); @@ -40,7 +40,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that a _start() export was passed. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); @@ -60,7 +60,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that an _initialize export was not passed. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); @@ -85,7 +85,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that a memory export was passed. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); @@ -103,7 +103,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that a WebAssembly.Instance from another VM context is accepted. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const instance = await vm.runInNewContext(` (async () => { const wasm = await WebAssembly.compile(bufferSource); @@ -127,7 +127,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); { // Verify that start() can only be called once. - const wasi = new WASI({}); + const wasi = new WASI({ version: 'preview1' }); const wasm = await WebAssembly.compile(bufferSource); const instance = await WebAssembly.instantiate(wasm); diff --git a/test/wasi/test-wasi-stdio.js b/test/wasi/test-wasi-stdio.js index 83b9beff8dcfe7..29e91281553817 100644 --- a/test/wasi/test-wasi-stdio.js +++ b/test/wasi/test-wasi-stdio.js @@ -18,7 +18,7 @@ writeFileSync(stdinFile, 'x'.repeat(33)); const stdin = openSync(stdinFile, 'r'); const stdout = openSync(stdoutFile, 'a'); const stderr = openSync(stderrFile, 'a'); -const wasi = new WASI({ stdin, stdout, stderr, returnOnExit: true }); +const wasi = new WASI({ version: 'preview1', stdin, stdout, stderr, returnOnExit: true }); const importObject = { wasi_snapshot_preview1: wasi.wasiImport }; (async () => { diff --git a/test/wasi/test-wasi-symlinks.js b/test/wasi/test-wasi-symlinks.js index 339c33db22b068..79369fd4c18247 100644 --- a/test/wasi/test-wasi-symlinks.js +++ b/test/wasi/test-wasi-symlinks.js @@ -10,6 +10,7 @@ if (process.argv[2] === 'wasi-child') { const { WASI } = require('wasi'); const wasmDir = path.join(__dirname, 'wasm'); const wasi = new WASI({ + version: 'preview1', args: [], env: process.env, preopens: { diff --git a/test/wasi/test-wasi-worker-terminate.js b/test/wasi/test-wasi-worker-terminate.js index 29657f003ba62c..52985703001aa6 100644 --- a/test/wasi/test-wasi-worker-terminate.js +++ b/test/wasi/test-wasi-worker-terminate.js @@ -34,7 +34,7 @@ if (!process.env.HAS_STARTED_WORKER) { } async function go() { - const wasi = new WASI({ returnOnExit: true }); + const wasi = new WASI({ version: 'preview1', returnOnExit: true }); const imports = { wasi_snapshot_preview1: wasi.wasiImport }; const module = await WebAssembly.compile(bytecode); const instance = await WebAssembly.instantiate(module, imports); diff --git a/test/wasi/test-wasi.js b/test/wasi/test-wasi.js index 2ddc4290352a20..cbfaf35d208b26 100644 --- a/test/wasi/test-wasi.js +++ b/test/wasi/test-wasi.js @@ -1,37 +1,7 @@ 'use strict'; const common = require('../common'); -if (process.argv[2] === 'wasi-child-default') { - // test default case - const fixtures = require('../common/fixtures'); - const tmpdir = require('../common/tmpdir'); - const fs = require('fs'); - const path = require('path'); - - common.expectWarning('ExperimentalWarning', - 'WASI is an experimental feature and might change at any time'); - - const { WASI } = require('wasi'); - tmpdir.refresh(); - const wasmDir = path.join(__dirname, 'wasm'); - const wasi = new WASI({ - args: ['foo', '-bar', '--baz=value'], - env: process.env, - preopens: { - '/sandbox': fixtures.path('wasi'), - '/tmp': tmpdir.path, - }, - }); - const importObject = { wasi_snapshot_preview1: wasi.wasiImport }; - const modulePath = path.join(wasmDir, `${process.argv[3]}.wasm`); - const buffer = fs.readFileSync(modulePath); - - (async () => { - const { instance } = await WebAssembly.instantiate(buffer, importObject); - - wasi.start(instance); - })().then(common.mustCall()); -} else if (process.argv[2] === 'wasi-child-preview1') { +if (process.argv[2] === 'wasi-child-preview1') { // Test version set to preview1 const assert = require('assert'); const fixtures = require('../common/fixtures'); @@ -73,7 +43,7 @@ if (process.argv[2] === 'wasi-child-default') { const cp = require('child_process'); const { checkoutEOL } = common; - function innerRunWASI(options, args, flavor = 'default') { + function innerRunWASI(options, args, flavor = 'preview1') { console.log('executing', options.test); const opts = { env: { @@ -101,7 +71,6 @@ if (process.argv[2] === 'wasi-child-default') { function runWASI(options) { innerRunWASI(options, ['--no-turbo-fast-api-calls']); innerRunWASI(options, ['--turbo-fast-api-calls']); - innerRunWASI(options, ['--turbo-fast-api-calls'], 'preview1'); } runWASI({ test: 'cant_dotdot' }); From ec117c747351ca24e2f6ecf0dfd768fd3f2f5f35 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:09:08 -0400 Subject: [PATCH 2/6] Update doc/api/wasi.md Co-authored-by: Colin Ihrig --- doc/api/wasi.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/wasi.md b/doc/api/wasi.md index 23537ab31c6961..80e7fb4207a461 100644 --- a/doc/api/wasi.md +++ b/doc/api/wasi.md @@ -148,7 +148,7 @@ changes: * `stderr` {integer} The file descriptor used as standard error in the WebAssembly application. **Default:** `2`. * `version` {string} The version of WASI requested. Currently the only - supported versions are `unstable` and `preview1`. This option + supported versions are `unstable` and `preview1`. This option is mandatory and there is no default. ### `wasi.getImportObject()` From 50612778d4c233adcd66b9280d9fe757c8148c90 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:09:21 -0400 Subject: [PATCH 3/6] Update test/wasi/test-wasi-options-validation.js Co-authored-by: Colin Ihrig --- test/wasi/test-wasi-options-validation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/wasi/test-wasi-options-validation.js b/test/wasi/test-wasi-options-validation.js index a1e672032a961c..6e24edc790ef34 100644 --- a/test/wasi/test-wasi-options-validation.js +++ b/test/wasi/test-wasi-options-validation.js @@ -50,7 +50,7 @@ assert.throws(() => { assert.throws(() => { new WASI({ version: { x: 'y' } }); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\bversion\b/ }); -// If version is not specificed, it should throw +// If version is not specified, it should throw. assert.throws(() => { new WASI(); }, { code: 'ERR_INVALID_ARG_TYPE', message: /\bversion\b/ }); From b48a7e620cff55ef25b9a67852791b4903f1e6dc Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:12:13 -0400 Subject: [PATCH 4/6] Update lib/wasi.js --- lib/wasi.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/wasi.js b/lib/wasi.js index c577df7eb0411f..e3d4294eeae743 100644 --- a/lib/wasi.js +++ b/lib/wasi.js @@ -54,7 +54,6 @@ class WASI { ({ WASI: _WASI } = internalBinding('wasi')); this[kBindingName] = 'wasi_unstable'; break; - // When adding support for additional wasi versions add case here case 'preview1': ({ WASI: _WASI } = internalBinding('wasi')); this[kBindingName] = 'wasi_snapshot_preview1'; From 39c56fc2f7e3a9de6151d078e3e8facf2df7ddcf Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:13:23 -0400 Subject: [PATCH 5/6] squash: address comments --- doc/api/wasi.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/api/wasi.md b/doc/api/wasi.md index 80e7fb4207a461..47b596d62f57e2 100644 --- a/doc/api/wasi.md +++ b/doc/api/wasi.md @@ -120,8 +120,7 @@ added: changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/47391 - description: default removed from version option such that - version much be specified, otherwise error is thrown. + description: The version option is now required and has no default value. - version: v19.8.0 pr-url: https://github.com/nodejs/node/pull/46469 description: version field added to options. From 21a40d29448fafad6ddfc86fdaf78202979426e0 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Wed, 5 Apr 2023 11:10:44 -0400 Subject: [PATCH 6/6] Update doc/api/wasi.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tobias Nießen --- doc/api/wasi.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/wasi.md b/doc/api/wasi.md index 47b596d62f57e2..ad4597ab30dbc4 100644 --- a/doc/api/wasi.md +++ b/doc/api/wasi.md @@ -148,7 +148,7 @@ changes: WebAssembly application. **Default:** `2`. * `version` {string} The version of WASI requested. Currently the only supported versions are `unstable` and `preview1`. This option is - mandatory and there is no default. + mandatory. ### `wasi.getImportObject()`