Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable emscripten dynamic execution #159

Merged

Conversation

sguimmara
Copy link
Contributor

This PR disables emitting dynamic execution code with Emscripten. This makes it possible to use the laz-perf WASM package in a context where the Content Security Policy (CSP) prohibits running dynamically evaluated code, such as code evaluated by the eval() function, or code passed to the Function constructor.

This is done by passing the -s DYNAMIC_EXECUTION=0 flag in the CMakeLists.txt for Emscripten.

fix #158

@sguimmara sguimmara force-pushed the disable-emscripten-dynamic-execution branch from d4e169e to 40b40f4 Compare February 10, 2025 14:40
@sguimmara sguimmara marked this pull request as ready for review February 10, 2025 15:09
@abellgithub
Copy link
Collaborator

Are there any downsides to this for people not using this in the context of a browser? Do we care?

@hobu
Copy link
Collaborator

hobu commented Feb 10, 2025

No downsides, and this probably should have been the default.

@sguimmara
Copy link
Contributor Author

Here is the relevant Emscripten doc: https://emscripten.org/docs/tools_reference/settings_reference.html#dynamic-execution

When this flag is set, the following features (linker flags) are unavailable:

   RELOCATABLE: the function loadDynamicLibrary would need to eval().

and some features may fall back to slower code paths when they need to: Embind: uses eval() to jit functions for speed.

In the worst case scenario, we could have 2 different builds, one for the node environment and one for worker/web (currently they all use the exact same generated files). I don't think it's really worth it though.

@abellgithub abellgithub merged commit d0d3047 into hobuinc:master Feb 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The laz-perf JS package trigger CSP violations
3 participants