jedisct1 / libsodium.js

libsodium compiled to Webassembly and pure JavaScript, with convenient wrappers.
Other
968 stars 138 forks source link

Assigning functions to constants before ready fails #327

Open FiloSottile opened 10 months ago

FiloSottile commented 10 months ago

I know sodium.ready needs to be called before using the library, but doing so at import time is difficult in libraries (where the await should be delegated to the application) or where lacking top-level await support.

At the same time, a common import pattern, even recommended by the Node error message in the details below (which might be fixable as a separate issue), is

import sodium from 'libsodium-wrappers-sumo'
const { crypto_hash_sha256, from_hex, to_hex } = sodium
``` file:///Users/filippo/src/filippo.io/age.ts/dist/hkdf.js:2 import { from_string } from "libsodium-wrappers-sumo"; ^^^^^^^^^^^ SyntaxError: Named export 'from_string' not found. The requested module 'libsodium-wrappers-sumo' is a CommonJS module, which may not support all module.exports as named exports. CommonJS modules can always be imported via the default export, for example using: import pkg from 'libsodium-wrappers-sumo'; const { from_string } = pkg; ```

When using that pattern, crypto_hash_sha256 is referenced before ready, which leads to intermittent errors.

https://github.com/FiloSottile/age.ts/actions/runs/6743827419/job/18332547466

Would it be possible to make it so calling functions before ready fails, but not simply referencing them?

(Also, if the failure was a clearer error, it would help. Debugging mysterious errors about _malloc to incorrect use of ready is pretty hard.)

See https://github.com/FiloSottile/age.ts/pull/14/commits/0e7577fbc9557d2539c31b3e2a5742d4af18c906.

jedisct1 commented 10 months ago

Having symbols that are defined but don't work, and are overwritten later doesn't seem to be simple to do. Because the import and the promise resolution can be in completely different scopes.

The promise is required to wait for the WebAssembly code to be compiled. Having builds that only include the JavaScript version, and not the WebAssembly version, would solve that, as the promise wouldn't be required any more. Earlier version of that library didn't need the promise.

I worked on making updated packages without WebAssembly, but didn't manage to get anything working in a reasonable time frame. I'll give it a new try some day.

FiloSottile commented 10 months ago

I might be missing the complexity of the setup, but would generating wrappers like

function crypto_hash_sha256(...) {
    if (!globalIsReady) throw notReadyError;
    return _crypto_hash_sha256(...)
}

not solve the issue?

_crypto_hash_sha256 can be treated like crypto_hash_sha256 is treated now, while assigning crypto_hash_sha256 to a constant before calling ready should still lead to a function that can be called after ready successfully.

jedisct1 commented 10 months ago

But how do you find globalIsReady?

jedisct1 commented 10 months ago

libsodium.js was a nice contribution, but honestly, for crypto in JavaScript today, I'd rather use WebCrypto when possible, and Noble cryptography for everything else.

FiloSottile commented 10 months ago

I think using globalThis.libsodiumIsReady would work.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis

jedisct1 commented 10 months ago

TIL about globalThis 👍