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

Loading of ClearScriptV8.dll #175

Closed
ChrisHuebsch-FLIG opened this issue Apr 14, 2020 · 8 comments
Closed

Loading of ClearScriptV8.dll #175

ChrisHuebsch-FLIG opened this issue Apr 14, 2020 · 8 comments

Comments

@ChrisHuebsch-FLIG
Copy link

ChrisHuebsch-FLIG commented Apr 14, 2020

I want to include ClearScript into my project, for scripting purposes (of course ;-))

The project itself is a DLL in a managed environment. That environment, allegedly, uses a private ManagedLoader with a custom --AssemblyLoadEventHandler-- AssemblyResolveHandler. This handler checks DLL-Names before loading them into the App-Domain.

It does so by splitting the assembly Name at "," with the help of "IndexOf" and "Substring". But that fails with ClearScriptV8, because of a programming mistake in that MangedLoader. (It does not check for IndexOf == -1)

I traced it fourther down to V8Proxy.cs, which loads the ClearScriptV8(-64).dll into the application.
It uses Assembly.Load("ClearScriptV8").

I cannot change the ManagedLoader, because its part of the 3rd party environment. Filing a bug would also not result in a near time solution.

Is there anything I can do on ClearScript side?

The worst I could imagine, is forking a clone of the repo and change that single line.

Is there any reason, you did not specify the Assembly-Name with that full blown identification string?

(Sorry for not using the proper vocabulary. I am rather new to .NET and it is one of my least used platforms at the moment.)

Edit: Corrected Handler-Type for future readers.

@ClearScriptLib ClearScriptLib self-assigned this Apr 14, 2020
@ClearScriptLib
Copy link
Collaborator

Hi @ChrisHuebsch-FLIG,

It does so by splitting the assembly Name at "," with the help of "IndexOf" and "Substring".

An assembly's Name (its so-called simple name) is extremely unlikely to have embedded commas.

Its FullName usually does, but that's true of ClearScript assemblies as well, so we're not sure how ClearScript differs in this regard.

Can you clarify?

Thanks!

@ChrisHuebsch-FLIG
Copy link
Author

The AssemblyResolveHandler, which is installed into my ApplicationContext, executes these lines, when it needs to load an additional dll:

private Assembly AssemblyResolveHandler(object sender, ResolveEventArgs args)
{
Trace.WriteLine("Resolve failed: " + args.Name);
int length = args.Name.IndexOf(',');
string str1 = args.Name.Substring(0, length);
// ...
}

V8Proxy uses a short name (or simple name). This causes the code above to run into an ArgumentOutOfRangeException, because length is -1.

According to the webpage, Assembly.Load() allows short or long names.
https://docs.microsoft.com/en-us/dotnet/api/system.reflection.assembly.load?view=netframework-4.8#System_Reflection_Assembly_Load_System_String_

But when opening the Assembly.Load() function in ObjectBrowser in Visual Studio, the comments show this:

2020-04-14 15_47_11-JS_Tests - Microsoft Visual Studio

So there are two colliding definitions of "Load()" - one in the Help library, one in the code itself.

In my environment the short name won't load because of that mistake.

Is there any method to get it loaded anyways?

Perhaps there is some method of "preloading" the V8-Library, so it won't trigger the AssemblyResolveHandler.

@ClearScriptLib
Copy link
Collaborator

Hi @ChrisHuebsch-FLIG,

Ah, it's the assembly resolution event, not the assembly load event.

Ironically, Assembly.Load("ClearScriptV8") is expected to fail in most cases; ClearScript just doesn't handle the exception that's thrown in your case. The call was added long ago at the request of a user who needed a hook for loading the V8 assemblies from a custom location.

Anyway, we'll harden the call in the next ClearScript version, but we do recommend that you also report the IndexOf issue to its owners.

In the meantime, we're not aware of any clean way to work around the problem. However, you might be able to use a hack to disable the faulty event handler temporarily. Is that something you're willing to do? If so, let us know if you're using .NET Framework or .NET Core.

Thanks!

@ChrisHuebsch-FLIG
Copy link
Author

That's awesome! Thank you!

@ClearScriptLib
Copy link
Collaborator

Hi @ChrisHuebsch-FLIG,

We'll assume that you're on .NET Framework, as we can't reproduce your issue on .NET Core. Apparently the latter always passes a full name to the assembly resolution handler, commas and all.

Try executing the following before instantiating your first script engine. Note that this is 100% reverse-engineered hack code that's not guaranteed to work and should be jettisoned as soon as you deploy the next version of ClearScript or a fixed version of the assembly resolution handler:

var field = typeof(AppDomain).GetField("_AssemblyResolve", BindingFlags.Instance | BindingFlags.NonPublic);
var handlers = ((Delegate)field.GetValue(AppDomain.CurrentDomain)).GetInvocationList();
Array.ForEach(handlers, handler => AppDomain.CurrentDomain.AssemblyResolve -= (ResolveEventHandler)handler);
using (new V8ScriptEngine()) { }
Array.ForEach(handlers, handler => AppDomain.CurrentDomain.AssemblyResolve += (ResolveEventHandler)handler);

Also note that this code should only be executed once, as ClearScript caches the V8 assembly and bypasses the loading process thereafter.

Good luck!

@ChrisHuebsch-FLIG
Copy link
Author

Indeed, I use .NET 4.x and not Core.

It's working. I am deeply impressed. Thank you so much.
That's the kind of support, which makes using this project a great experience.

ClearScriptLib added a commit that referenced this issue May 29, 2020
…; added JavaScriptExtensions.ToTask (GitHub Issue #182); added DocumentLoader.MaxCacheSize and DocumentCategory.MaxCacheSize; added code to break event connections on engine disposal (GitHub Issue #183); improved ES6 module support, fixing cycle crash (GitHub Issue #181); added DynamicHostObject (GitHub Issue #180); added BigInt / BigInteger support for V8 (GitHub Issue #176); hardened Assembly.Load call in V8Proxy.cs (GitHub Issue #175); improved V8Update environment isolation to fix some V8 build issues (GitHub Issue #185); updated API documentation. Tested with V8 8.3.110.9.
@ClearScriptLib
Copy link
Collaborator

Fixed in Version 6.0.2.

@ChrisHuebsch-FLIG
Copy link
Author

I can confirm the fix works!

Big thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants