microsoft / ClearScript

A library for adding scripting to .NET applications. Supports V8 (Windows, Linux, macOS) and JScript/VBScript (Windows).
https://microsoft.github.io/ClearScript/
MIT License
1.74k stars 148 forks source link

[ScriptUsage(ScriptAccess.ReadOnly)] on property don't give access to the getter when DefaultAccess = ScriptAccess.None #439

Closed vbornand closed 1 year ago

vbornand commented 1 year ago

Hello,

I have a V8ScriptEngine configured with DefaultAccess = ScriptAccess.None to have access only on some specific methods and properties.

With the following code, I'm able to access the Name property:

public class Animal
{
    [ScriptUsage(ScriptAccess.ReadOnly)]
    public string Name { [ScriptUsage(ScriptAccess.ReadOnly)]get; set; }
}

but if I remove the attribute before the get:

public class Animal
{
    [ScriptUsage(ScriptAccess.ReadOnly)]
    public string Name {get; set; }
}

I have the following error: [Exception: Error: The property get method is unavailable or inaccessible at Script:1:42<error: Error: The property get method is unavailable or inaccessible<error>]

It's not a big deal to add the attribute on the get, but in my case I use a CustomAttributeLoader to add "virtually" the ScriptUsage attributes on some properties and I don't find a way to add it on the getter too.

Here is a simplified program to reproduce the issue:

using Microsoft.ClearScript;
using Microsoft.ClearScript.V8;
using System.Reflection;

public class Animal
{
    public string Name { get; set; }
}

public class AnimalFactory
{
    [ScriptUsage]
    public Animal GetAnimal(string name)
    {
        return new Animal() { Name = name };
    }
}

internal class Program
{
    private static void Main(string[] args)
    {
        HostSettings.CustomAttributeLoader = new TestCustomAttributeLoader();
        var jsEngine = new V8ScriptEngine(V8ScriptEngineFlags.EnableDebugging | V8ScriptEngineFlags.EnableRemoteDebugging | V8ScriptEngineFlags.AwaitDebuggerAndPauseOnStart | V8ScriptEngineFlags.EnableTaskPromiseConversion | V8ScriptEngineFlags.DisableGlobalMembers);
        jsEngine.DefaultAccess = ScriptAccess.None;
        jsEngine.AddHostObject("animalFactory", new AnimalFactory());
        jsEngine.Execute("var a = animalFactory.GetAnimal('Test'); debugger;"); //a.Name => [Exception: Error: The property get method is unavailable or inaccessible at Script:1:42<error: Error: The property get method is unavailable or inaccessible<error>]
    }
}

///Give read access on all the properties
public class TestCustomAttributeLoader: CustomAttributeLoader
{
    public override T[] LoadCustomAttributes<T>(ICustomAttributeProvider resource, bool inherit)
    {
        if (typeof(T) == typeof(ScriptUsageAttribute))
        {
            var property = resource as PropertyInfo;
            if (property != null)
            {
                return new[] { new ScriptUsageAttribute(ScriptAccess.ReadOnly) } as T[];
            }
        }
        return base.LoadCustomAttributes<T>(resource, inherit);
    }
}

Is it a current limitation or did I something wrong?

Thanks

vbornand commented 1 year ago

I just found a workaround:

using Microsoft.ClearScript;
using Microsoft.ClearScript.V8;
using System.Reflection;

public class Animal
{
    public string Name { get; set; }
}

public class AnimalFactory
{
    [ScriptUsage]
    public Animal GetAnimal(string name)
    {
        return new Animal() { Name = name };
    }
}

internal class Program
{
    private static void Main(string[] args)
    {
        HostSettings.CustomAttributeLoader = new TestCustomAttributeLoader();
        var jsEngine = new V8ScriptEngine(V8ScriptEngineFlags.EnableDebugging | V8ScriptEngineFlags.EnableRemoteDebugging | V8ScriptEngineFlags.AwaitDebuggerAndPauseOnStart | V8ScriptEngineFlags.EnableTaskPromiseConversion | V8ScriptEngineFlags.DisableGlobalMembers);
        //jsEngine.DefaultAccess = ScriptAccess.None; => the default access is set to None for all the types in the CustomAttributeLoader.
        jsEngine.AddHostObject("animalFactory", new AnimalFactory());
        jsEngine.Execute("var a = animalFactory.GetAnimal('Test'); debugger;");
    }
}

public class TestCustomAttributeLoader: CustomAttributeLoader
{
    public override T[] LoadCustomAttributes<T>(ICustomAttributeProvider resource, bool inherit)
    {
        if (typeof(T) == typeof(DefaultScriptUsageAttribute))
        {
            return new[] { new DefaultScriptUsageAttribute(ScriptAccess.None) } as T[];
        }

        if (typeof(T) == typeof(ScriptUsageAttribute))
        {
            var property = resource as PropertyInfo;
            if (property != null)
            {
                return new[] { new ScriptUsageAttribute(ScriptAccess.ReadOnly) } as T[];
            }
        }
        return base.LoadCustomAttributes<T>(resource, inherit);
    }
}

Instead of set jsEngine.DefaultAccess = ScriptAccess.None;, I addded the following code in the CustomAttributeLoader:

if (typeof(T) == typeof(DefaultScriptUsageAttribute))
{
    return new[] { new DefaultScriptUsageAttribute(ScriptAccess.None) } as T[];
}

As far as I understand, the behavior should be the same with the 2 methods.

ClearScriptLib commented 1 year ago

Hi @vbornand,

We agree that a property accessor without an explicitly declared script access level should inherit it from its parent property. Our next release will include a fix.

Thanks for reporting this!

ClearScriptLib commented 1 year ago

Fixed in Version 7.3.5.