Git rid of eval() in WASM wrapper

[Aragas]

Hi again! I used DotNetJS to create a library that exposes some common functions that we share across C# executables and an Electron app to ensure the behaviour is identical. The problem is, the electron's app (Vortex) is blocking eval execution due to it's CSP. I did earlier some investigation about this, but I would like to double check! Is eval critical for the runtime to work or is it used only for debugging? Could we strip it's usage with some conditionals?

• [x] Waiting for https://github.com/dotnet/runtime/pull/74441

[Aragas]

https://github.com/dotnet/runtime/issues/68374 https://github.com/dotnet/runtime/issues/59416 https://github.com/dotnet/aspnetcore/issues/37787

Based on those issues, it's fixable, but not done yet, it seems

[elringus]

VS Code (which is built on Electron as well, iirc) had a similar limitation, but they've removed it: https://github.com/microsoft/vscode/issues/138413

Here are the 3 eval calls I was able to find in the .NET's wasm wrapper:

https://github.com/Elringus/DotNetJS/blob/cad9a87854fd860203d25991a55bb3a68fa11229/JavaScript/dotnet-runtime/native/dotnet.js#L578 https://github.com/Elringus/DotNetJS/blob/cad9a87854fd860203d25991a55bb3a68fa11229/JavaScript/dotnet-runtime/native/dotnet.js#L640 https://github.com/Elringus/DotNetJS/blob/cad9a87854fd860203d25991a55bb3a68fa11229/JavaScript/dotnet-runtime/native/dotnet.js#L3840

— but there are probably more.

Not sure it's possible to patch it the same way we're patching out other stuff here: https://github.com/Elringus/DotNetJS/blob/cad9a87854fd860203d25991a55bb3a68fa11229/JavaScript/dotnet-runtime/scripts/compile-runtime.sh#L13-L16

I also hope to opt-out of custom .NET runtime build as soon as possible (so we can support AOT and stripping #20), which will be problematic should we further patch the wrapper.

Guess we'll have to wait until this is solved in .NET runtime.

[Aragas]

I'll report at least my findings then.

I think that there's an additional eval call in function Function.apply(Function, i); https://github.com/Elringus/DotNetJS/blob/cad9a87854fd860203d25991a55bb3a68fa11229/JavaScript/dotnet-runtime/native/dotnet.js#L4760 After replacing all evals with { } as you did, Vortex still had an issue with that function. After replacing it with (x, y) => () => { }, CSP wasn't triggering anymore and Vortex stared to load the module, but now there's this error Error: System.NullReferenceException: Arg_NullReferenceException, pretty sure it breaks the runtime.

From what I understand somehow an eval function is injected here https://github.com/Elringus/DotNetJS/blob/cad9a87854fd860203d25991a55bb3a68fa11229/JavaScript/dotnet-runtime/native/dotnet.js#L4760

I manually enabled unsafe-eval for better debugging and tried some shenanigans like new Function('"use strict";return (' + keys+ ')'), but nope. Replacing with new Function('return ' + keys); or new Function(keys) yielded an interesting error and stacktrace: ReferenceError: converter is not defined at eval (https://mono-wasm.invalid/variadic_converter_ii_result_unmarshaled:3:1) which confirms that eval is used internally, as I understand.

I'm still not sure what Function.apply() does. It's safe to replace the first arg with null instead of Function, not sure why it wasn't done. Replacing the whole construction with eval(keys) works too. But as said earlier, the alternative with new Function() isn't working.

[elringus]

I think that there's an additional eval call in function Function.apply(Function, i);

Right, that aligns with the mono issue you've mentioned previously (https://github.com/dotnet/runtime/issues/68374); they've also mentioned the same line and a couple of others. Guess we'd have to wait when it's solved.

[Aragas]

Oh, right, forgot about that issue. Yep, agree, can't do much here.

[Aragas]

https://github.com/dotnet/runtime/issues/68374#issuecomment-1149742636 I guess the variable could be set at build time?

[elringus]

dotnet/runtime#68374 (comment) I guess the variable could be set at build time?

Probably here: https://github.com/Elringus/DotNetUMD/blob/release/6.0/src/mono/wasm/wasm.proj#L74 Though, I don't think just setting this var will strip all the evals, as they've mentioned they're using them to generate interop functions.

[Aragas]

As I understand, will be fixed once this is done https://github.com/dotnet/runtime/pull/74441

[Aragas]

Okay, the deed is done, they merged the PR! Is it possible to switch to the new code?

[elringus]

I'll look into this once the change is available in a shipped .NET, which will probably be .NET 8 (judging by the milestone associated with the PR).