From 3d4164cbf8e73e94a135f643fc482110ac52f506 Mon Sep 17 00:00:00 2001 From: Chris Dickinson Date: Thu, 4 Jan 2024 23:35:13 -0800 Subject: [PATCH] feat,fix: respect EXTISM_ENABLE_WASI_OUTPUT I implemented this as an `ExtismPluginOption`, `enableWasiOutput`, that defaults to checking the environment variable (on platforms that support runtime environment variables.) - There was a WASI browser polyfill bug with the wasistdout module. Since `wasistdout` doesn't export a `initialize` or `start` method, we weren't initializing the context appropriately. This now fixed. - Neither Deno nor Node squelched WASI output before this commit. Now they do by default -- by opening /dev/null (or NUL on Windows) and sending that FD in instead. - And now the browser supports WASI output as well! - Additionally, simplify all `prop?: T | undefined` to `prop?: T` in `src/interfaces.ts`. Fixes #43. --- package-lock.json | 8 ++--- package.json | 2 +- src/foreground-plugin.ts | 2 +- src/interfaces.ts | 43 +++++++++++++++-------- src/mod.test.ts | 42 ++++++++++++++++++++++ src/mod.ts | 2 ++ src/polyfills/browser-capabilities.ts | 2 ++ src/polyfills/browser-wasi.ts | 48 +++++++++++++++++++++----- src/polyfills/bun-capabilities.ts | 2 ++ src/polyfills/deno-capabilities.ts | 4 +++ src/polyfills/deno-wasi.ts | 28 ++++++++++++++- src/polyfills/node-capabilities.ts | 2 ++ src/polyfills/node-wasi.ts | 31 +++++++++++++++-- types/deno/index.d.ts | 13 +++++++ wasm/wasistdout.wasm | Bin 0 -> 169 bytes 15 files changed, 198 insertions(+), 31 deletions(-) create mode 100644 types/deno/index.d.ts create mode 100755 wasm/wasistdout.wasm diff --git a/package-lock.json b/package-lock.json index 727b79d..e7e7b1e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.0.0-replaced-by-ci", "license": "BSD-3-Clause", "devDependencies": { - "@bjorn3/browser_wasi_shim": "^0.2.14", + "@bjorn3/browser_wasi_shim": "^0.2.17", "@playwright/test": "^1.39.0", "@types/node": "^20.8.7", "@typescript-eslint/eslint-plugin": "^6.8.0", @@ -37,9 +37,9 @@ } }, "node_modules/@bjorn3/browser_wasi_shim": { - "version": "0.2.14", - "resolved": "https://registry.npmjs.org/@bjorn3/browser_wasi_shim/-/browser_wasi_shim-0.2.14.tgz", - "integrity": "sha512-1S0NNBgFqkogICDL7DOYmXLvxv4tlrjNZps6XNB7P8rr3XwXG1DqtfgYImERUzz71GirexUVrD/jQZht7swCzw==", + "version": "0.2.17", + "resolved": "https://registry.npmjs.org/@bjorn3/browser_wasi_shim/-/browser_wasi_shim-0.2.17.tgz", + "integrity": "sha512-B2qcaGROo4e2s4nXb3VPATrczVrntM4BUXtAU1gEzUOfqKTcVuePq4NfhH5hmLBSvZ45YcT4gflDRUFYqLhkxA==", "dev": true }, "node_modules/@esbuild/android-arm": { diff --git a/package.json b/package.json index 248043e..fca8f88 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "author": "The Extism Authors ", "license": "BSD-3-Clause", "devDependencies": { - "@bjorn3/browser_wasi_shim": "^0.2.14", + "@bjorn3/browser_wasi_shim": "^0.2.17", "@playwright/test": "^1.39.0", "@types/node": "^20.8.7", "@typescript-eslint/eslint-plugin": "^6.8.0", diff --git a/src/foreground-plugin.ts b/src/foreground-plugin.ts index d7bb2bc..22f66e7 100644 --- a/src/foreground-plugin.ts +++ b/src/foreground-plugin.ts @@ -154,7 +154,7 @@ export async function createForegroundPlugin( modules: WebAssembly.Module[], context: CallContext = new CallContext(ArrayBuffer, opts.logger, opts.config), ): Promise { - const wasi = opts.wasiEnabled ? await loadWasi(opts.allowedPaths) : null; + const wasi = opts.wasiEnabled ? await loadWasi(opts.allowedPaths, opts.enableWasiOutput) : null; const imports: Record> = { ...(wasi ? { wasi_snapshot_preview1: await wasi.importObject() } : {}), diff --git a/src/interfaces.ts b/src/interfaces.ts index d83f00b..7e4e61e 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -49,11 +49,11 @@ export class PluginOutput extends DataView { throw new Error('Cannot set values on output'); } - setInt16(_byteOffset: number, _value: number, _littleEndian?: boolean | undefined): void { + setInt16(_byteOffset: number, _value: number, _littleEndian?: boolean): void { throw new Error('Cannot set values on output'); } - setInt32(_byteOffset: number, _value: number, _littleEndian?: boolean | undefined): void { + setInt32(_byteOffset: number, _value: number, _littleEndian?: boolean): void { throw new Error('Cannot set values on output'); } @@ -61,27 +61,27 @@ export class PluginOutput extends DataView { throw new Error('Cannot set values on output'); } - setUint16(_byteOffset: number, _value: number, _littleEndian?: boolean | undefined): void { + setUint16(_byteOffset: number, _value: number, _littleEndian?: boolean): void { throw new Error('Cannot set values on output'); } - setUint32(_byteOffset: number, _value: number, _littleEndian?: boolean | undefined): void { + setUint32(_byteOffset: number, _value: number, _littleEndian?: boolean): void { throw new Error('Cannot set values on output'); } - setFloat32(_byteOffset: number, _value: number, _littleEndian?: boolean | undefined): void { + setFloat32(_byteOffset: number, _value: number, _littleEndian?: boolean): void { throw new Error('Cannot set values on output'); } - setFloat64(_byteOffset: number, _value: number, _littleEndian?: boolean | undefined): void { + setFloat64(_byteOffset: number, _value: number, _littleEndian?: boolean): void { throw new Error('Cannot set values on output'); } - setBigInt64(_byteOffset: number, _value: bigint, _littleEndian?: boolean | undefined): void { + setBigInt64(_byteOffset: number, _value: bigint, _littleEndian?: boolean): void { throw new Error('Cannot set values on output'); } - setBigUint64(_byteOffset: number, _value: bigint, _littleEndian?: boolean | undefined): void { + setBigUint64(_byteOffset: number, _value: bigint, _littleEndian?: boolean): void { throw new Error('Cannot set values on output'); } } @@ -130,14 +130,14 @@ export interface ExtismPluginOptions { /** * Whether or not to enable WASI preview 1. */ - useWasi?: boolean | undefined; + useWasi?: boolean; /** * Whether or not to run the Wasm module in a Worker thread. Requires * {@link Capabilities#hasWorkerCapability | `CAPABILITIES.hasWorkerCapability`} to * be true. */ - runInWorker?: boolean | undefined; + runInWorker?: boolean; /** * A logger implementation. Must provide `info`, `debug`, `warn`, and `error` methods. @@ -163,7 +163,14 @@ export interface ExtismPluginOptions { functions?: { [key: string]: { [key: string]: (callContext: CallContext, ...args: any[]) => any } } | undefined; allowedPaths?: { [key: string]: string } | undefined; allowedHosts?: string[] | undefined; - config?: PluginConfigLike | undefined; + + /** + * Whether WASI stdout should be forwarded to the host. + * + * Overrides the `EXTISM_ENABLE_WASI_OUTPUT` environment variable. + */ + enableWasiOutput?: boolean; + config?: PluginConfigLike; fetch?: typeof fetch; sharedArrayBufferSize?: number; } @@ -172,6 +179,7 @@ export interface InternalConfig { logger: Console; allowedHosts: string[]; allowedPaths: { [key: string]: string }; + enableWasiOutput: boolean; functions: { [namespace: string]: { [func: string]: any } }; fetch: typeof fetch; wasiEnabled: boolean; @@ -241,8 +249,8 @@ export type ManifestWasm = ( | ManifestWasmResponse | ManifestWasmModule ) & { - name?: string | undefined; - hash?: string | undefined; + name?: string; + hash?: string; }; /** @@ -263,7 +271,7 @@ export type ManifestWasm = ( */ export interface Manifest { wasm: Array; - config?: PluginConfigLike | undefined; + config?: PluginConfigLike; } /** @@ -373,4 +381,11 @@ export interface Capabilities { * - ✅ webkit (via [`@bjorn3/browser_wasi_shim`](https://www.npmjs.com/package/@bjorn3/browser_wasi_shim)) */ supportsWasiPreview1: boolean; + + /** + * Whether or not the `EXTISM_ENABLE_WASI_OUTPUT` environment variable has been set. + * + * This value is consulted whenever {@link ExtismPluginOptions#enableWasiOutput} is omitted. + */ + extismStdoutEnvVarSet: boolean; } diff --git a/src/mod.test.ts b/src/mod.test.ts index d2ae345..cdc26da 100644 --- a/src/mod.test.ts +++ b/src/mod.test.ts @@ -538,6 +538,48 @@ if (typeof WebAssembly === 'undefined') { await plugin.close(); } }); + + // TODO(chrisdickinson): this turns out to be pretty tricky to test, since + // deno and node's wasi bindings bypass JS entirely and write directly to + // their respective FDs. I'm settling for tests that exercise both behaviors. + test('when EXTISM_ENABLE_WASI_OUTPUT is not set, WASI output is stifled', async () => { + if ((globalThis as unknown as any).process) { + ( + globalThis as unknown as Record }> + ).process.env.EXTISM_ENABLE_WASI_OUTPUT = ''; + } else if ((globalThis as unknown as any).Deno) { + globalThis.Deno.env.set('EXTISM_ENABLE_WASI_OUTPUT', ''); + } + const plugin = await createPlugin('http://localhost:8124/wasm/wasistdout.wasm', { + useWasi: true, + }); + + try { + await plugin.call('say_hello'); + } finally { + await plugin.close(); + } + }); + + test('respects enableWasiOutput', async () => { + if ((globalThis as unknown as any).process) { + ( + globalThis as unknown as Record }> + ).process.env.EXTISM_ENABLE_WASI_OUTPUT = ''; + } else if ((globalThis as unknown as any).Deno) { + globalThis.Deno.env.set('EXTISM_ENABLE_WASI_OUTPUT', ''); + } + const plugin = await createPlugin('http://localhost:8124/wasm/wasistdout.wasm', { + useWasi: true, + enableWasiOutput: true, + }); + + try { + await plugin.call('say_hello'); + } finally { + await plugin.close(); + } + }); } if (CAPABILITIES.fsAccess && CAPABILITIES.supportsWasiPreview1) { diff --git a/src/mod.ts b/src/mod.ts index 81820b1..0da7a24 100644 --- a/src/mod.ts +++ b/src/mod.ts @@ -69,6 +69,7 @@ export async function createPlugin( ): Promise { opts = { ...opts }; opts.useWasi ??= false; + opts.enableWasiOutput ??= opts.useWasi ? CAPABILITIES.extismStdoutEnvVarSet : false; opts.functions = opts.functions || {}; opts.allowedPaths ??= {}; opts.allowedHosts ??= [].concat(opts.allowedHosts || []); @@ -93,6 +94,7 @@ export async function createPlugin( wasiEnabled: opts.useWasi, logger: opts.logger, config: opts.config, + enableWasiOutput: opts.enableWasiOutput, sharedArrayBufferSize: Number(opts.sharedArrayBufferSize) || 1 << 16, }; diff --git a/src/polyfills/browser-capabilities.ts b/src/polyfills/browser-capabilities.ts index 67d753b..806b7eb 100644 --- a/src/polyfills/browser-capabilities.ts +++ b/src/polyfills/browser-capabilities.ts @@ -19,4 +19,6 @@ export const CAPABILITIES: Capabilities = { : true, supportsWasiPreview1: true, + + extismStdoutEnvVarSet: false, }; diff --git a/src/polyfills/browser-wasi.ts b/src/polyfills/browser-wasi.ts index bfd4eca..5de63e9 100644 --- a/src/polyfills/browser-wasi.ts +++ b/src/polyfills/browser-wasi.ts @@ -1,14 +1,46 @@ -import { WASI, Fd, File, OpenFile } from '@bjorn3/browser_wasi_shim'; +import { WASI, Fd, File, OpenFile, wasi } from '@bjorn3/browser_wasi_shim'; import { type InternalWasi } from '../mod.ts'; -export async function loadWasi(_allowedPaths: { [from: string]: string }): Promise { +class Output extends Fd { + #mode: string; + + constructor(mode: string) { + super(); + this.#mode = mode; + } + + fd_write(view8: Uint8Array, iovs: [wasi.Iovec]): { ret: number; nwritten: number } { + let nwritten = 0; + const decoder = new TextDecoder(); + const str = iovs.reduce((acc, iovec, idx, all) => { + nwritten += iovec.buf_len; + const buffer = view8.slice(iovec.buf, iovec.buf + iovec.buf_len); + return acc + decoder.decode(buffer, { stream: idx !== all.length - 1 }); + }, ''); + + (console[this.#mode] as any)(str); + + return { ret: 0, nwritten }; + } +} + +export async function loadWasi( + _allowedPaths: { [from: string]: string }, + enableWasiOutput: boolean, +): Promise { const args: Array = []; const envVars: Array = []; - const fds: Fd[] = [ - new OpenFile(new File([])), // stdin - new OpenFile(new File([])), // stdout - new OpenFile(new File([])), // stderr - ]; + const fds: Fd[] = enableWasiOutput + ? [ + new Output('log'), // fd 0 is dup'd to stdout + new Output('log'), + new Output('error'), + ] + : [ + new OpenFile(new File([])), // stdin + new OpenFile(new File([])), // stdout + new OpenFile(new File([])), // stderr + ]; const context = new WASI(args, envVars, fds); @@ -38,7 +70,7 @@ export async function loadWasi(_allowedPaths: { [from: string]: string }): Promi } else { init(); } - } else if (instance.exports._start) { + } else { context.start({ exports: { memory, diff --git a/src/polyfills/bun-capabilities.ts b/src/polyfills/bun-capabilities.ts index 1bdc565..9f2a80d 100644 --- a/src/polyfills/bun-capabilities.ts +++ b/src/polyfills/bun-capabilities.ts @@ -17,4 +17,6 @@ export const CAPABILITIES: Capabilities = { // See https://github.com/oven-sh/bun/issues/1960 supportsWasiPreview1: false, + + extismStdoutEnvVarSet: Boolean(process.env.EXTISM_ENABLE_WASI_OUTPUT), }; diff --git a/src/polyfills/deno-capabilities.ts b/src/polyfills/deno-capabilities.ts index e65b60e..e7734ce 100644 --- a/src/polyfills/deno-capabilities.ts +++ b/src/polyfills/deno-capabilities.ts @@ -1,5 +1,7 @@ import type { Capabilities } from '../interfaces.ts'; +const { Deno } = globalThis as unknown as { Deno: { env: Map } }; + export const CAPABILITIES: Capabilities = { // When false, shared buffers have to be copied to an array // buffer before passing to Text{En,De}coding() @@ -16,4 +18,6 @@ export const CAPABILITIES: Capabilities = { hasWorkerCapability: true, supportsWasiPreview1: true, + + extismStdoutEnvVarSet: Boolean(Deno.env.get('EXTISM_ENABLE_WASI_OUTPUT')), }; diff --git a/src/polyfills/deno-wasi.ts b/src/polyfills/deno-wasi.ts index 16304af..91fed35 100644 --- a/src/polyfills/deno-wasi.ts +++ b/src/polyfills/deno-wasi.ts @@ -1,10 +1,36 @@ import Context from 'https://deno.land/std@0.200.0/wasi/snapshot_preview1.ts'; import { type InternalWasi } from '../interfaces.ts'; +import { devNull } from 'node:os'; +import { open } from 'node:fs/promises'; +import { closeSync } from 'node:fs'; -export async function loadWasi(allowedPaths: { [from: string]: string }): Promise { +async function createDevNullFDs() { + const [stdin, stdout] = await Promise.all([open(devNull, 'r'), open(devNull, 'w')]); + + const fr = new globalThis.FinalizationRegistry((held: number) => { + try { + closeSync(held); + } catch { + // The fd may already be closed. + } + }); + fr.register(stdin, stdin.fd); + fr.register(stdout, stdout.fd); + + return [stdin.fd, stdout.fd, stdout.fd]; +} + +export async function loadWasi( + allowedPaths: { [from: string]: string }, + enableWasiOutput: boolean, +): Promise { + const [stdin, stdout, stderr] = enableWasiOutput ? [0, 1, 2] : await createDevNullFDs(); const context = new Context({ preopens: allowedPaths, exitOnReturn: false, + stdin, + stdout, + stderr, }); return { diff --git a/src/polyfills/node-capabilities.ts b/src/polyfills/node-capabilities.ts index 85ed178..60965db 100644 --- a/src/polyfills/node-capabilities.ts +++ b/src/polyfills/node-capabilities.ts @@ -16,4 +16,6 @@ export const CAPABILITIES: Capabilities = { hasWorkerCapability: true, supportsWasiPreview1: true, + + extismStdoutEnvVarSet: Boolean(process.env.EXTISM_ENABLE_WASI_OUTPUT), }; diff --git a/src/polyfills/node-wasi.ts b/src/polyfills/node-wasi.ts index 5907bf6..21833ff 100644 --- a/src/polyfills/node-wasi.ts +++ b/src/polyfills/node-wasi.ts @@ -1,10 +1,37 @@ import { WASI } from 'wasi'; -import { type InternalWasi } from '../mod.ts'; +import { type InternalWasi } from '../interfaces.ts'; +import { devNull } from 'node:os'; +import { open } from 'node:fs/promises'; +import { closeSync } from 'node:fs'; + +async function createDevNullFDs() { + const [stdin, stdout] = await Promise.all([open(devNull, 'r'), open(devNull, 'w')]); + + const fr = new globalThis.FinalizationRegistry((held: number) => { + try { + closeSync(held); + } catch { + // The fd may already be closed. + } + }); + fr.register(stdin, stdin.fd); + fr.register(stdout, stdout.fd); + + return [stdin.fd, stdout.fd, stdout.fd]; +} + +export async function loadWasi( + allowedPaths: { [from: string]: string }, + enableWasiOutput: boolean, +): Promise { + const [stdin, stdout, stderr] = enableWasiOutput ? [0, 1, 2] : await createDevNullFDs(); -export async function loadWasi(allowedPaths: { [from: string]: string }): Promise { const context = new WASI({ version: 'preview1', preopens: allowedPaths, + stdin, + stdout, + stderr, } as any); return { diff --git a/types/deno/index.d.ts b/types/deno/index.d.ts new file mode 100644 index 0000000..c37e591 --- /dev/null +++ b/types/deno/index.d.ts @@ -0,0 +1,13 @@ +declare module 'https://deno.land/x/minimatch@v3.0.4/index.js' { + export default function matches(text: string, pattern: string): boolean; +} + +declare module 'https://deno.land/std@0.200.0/wasi/snapshot_preview1.ts' { + export default class Context { + constructor(opts: Record); + + exports: WebAssembly.Exports; + start(opts: any); + initialize?(opts: any); + } +} diff --git a/wasm/wasistdout.wasm b/wasm/wasistdout.wasm new file mode 100755 index 0000000000000000000000000000000000000000..5703ca2bcccef528bba7aafe51c043ca9b6f179f GIT binary patch literal 169 zcmX|)K?=e!6h!BzQES_UiWE@h?fKN$oU)wgJ262~3RWqF`wE32P