Stop attaching Array.prototype to dictionary-like objects in JavaScript expressions (#7890)
* fix(javascript): stop attaching Array.prototype to dictionary-like objects The custom `WrapObjectDelegate` installed by `JintJavaScriptEvaluator` duplicated what Jint already does, and got it wrong in two ways. Jint's default wrap handler is `ObjectWrapper.Create(engine, target, type)`, and `ObjectWrapper` attaches `Array.prototype` to array-like wrappers by itself when `Options.Interop.AttachArrayPrototype` is enabled (the default). Jint's own array-likeness test deliberately excludes dictionary-like types, including string-keyed generic dictionaries. The handler we installed instead: * Called `ObjectWrapper.Create(engine, target)`, dropping the declared `type` argument, so members were resolved against the runtime type rather than the declared one. * Used `ObjectArrayHelper.DetermineIfObjectIsArrayLikeClrCollection`, which only excludes the non-generic `IDictionary`. `ExpandoObject` does not implement that interface, so it came out array-like. Both the `variables` container and the `args` container are `ExpandoObject` instances, which meant `Object.getPrototypeOf(variables) === Array.prototype` was true and `variables.map`, `variables.filter`, `variables.reduce` and friends were all visible on them, with `variables.length` reporting `0` instead of `undefined`. Removing the handler restores Jint's default, which handles every case the custom one was written for: `List<T>`, `T[]`, `HashSet<T>`, `ImmutableArray<T>`, `Queue<T>` and `Stack<T>` all still get `Array.prototype`, while dictionaries and `ExpandoObject` no longer do. `ObjectArrayHelper` is public, so it is marked obsolete rather than deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik * test(javascript): cast the ExpandoObject to its dictionary interface `new ExpandoObject() as IDictionary<string, object>` reads as a conversion that might fail and gives the variable a nullable declared type, when `ExpandoObject` implements the interface unconditionally. A direct cast states that, and matches the BCL's `IDictionary<string, object?>` annotation exactly so the value type argument lines up too. The two other `as IDictionary<string, object>` uses in this test project (JintJavaScriptFunctionBehaviorTests) are deliberately left alone: there the operand is the untyped result of a script evaluation, so the `as` is a genuine type test paired with `Assert.NotNull`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bc41fa9957
commit
ddc95d734d
|
|
@ -5,6 +5,7 @@ namespace Elsa.Expressions.JavaScript.Helpers;
|
|||
/// <summary>
|
||||
/// Contains helper methods for working with object arrays.
|
||||
/// </summary>
|
||||
[Obsolete("Jint decides array-likeness itself and attaches Array.prototype to array-like wrappers when Options.Interop.AttachArrayPrototype is enabled (the default). This helper is no longer used and will be removed in a future version.")]
|
||||
public static class ObjectArrayHelper
|
||||
{
|
||||
/// <summary>
|
||||
|
|
|
|||
|
|
@ -4,13 +4,11 @@ using Acornima.Ast;
|
|||
using Elsa.Expressions.Helpers;
|
||||
using Elsa.Expressions.Models;
|
||||
using Elsa.Expressions.JavaScript.Contracts;
|
||||
using Elsa.Expressions.JavaScript.Helpers;
|
||||
using Elsa.Expressions.JavaScript.Notifications;
|
||||
using Elsa.Expressions.JavaScript.ObjectConverters;
|
||||
using Elsa.Expressions.JavaScript.Options;
|
||||
using Elsa.Mediator.Contracts;
|
||||
using Jint;
|
||||
using Jint.Runtime.Interop;
|
||||
using Microsoft.Extensions.Caching.Memory;
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
|
@ -70,7 +68,6 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification
|
|||
engineOptions.Interop.EnumConversion = EnumConversionMode.String;
|
||||
|
||||
ConfigureClrAccess(engineOptions);
|
||||
ConfigureObjectWrapper(engineOptions);
|
||||
ConfigureObjectConverters(engineOptions);
|
||||
ConfigureExecutionConstraints(engineOptions, cancellationToken);
|
||||
|
||||
|
|
@ -93,19 +90,6 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification
|
|||
options.AllowClr();
|
||||
}
|
||||
|
||||
private void ConfigureObjectWrapper(Jint.Options options)
|
||||
{
|
||||
options.SetWrapObjectHandler((engine, target, type) =>
|
||||
{
|
||||
var instance = ObjectWrapper.Create(engine, target);
|
||||
|
||||
if (ObjectArrayHelper.DetermineIfObjectIsArrayLikeClrCollection(target.GetType()))
|
||||
instance.Prototype = engine.Intrinsics.Array.PrototypeObject;
|
||||
|
||||
return instance;
|
||||
});
|
||||
}
|
||||
|
||||
private void ConfigureExecutionConstraints(Jint.Options options, CancellationToken cancellationToken)
|
||||
{
|
||||
// An expression that never returns would otherwise occupy the calling thread forever.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,79 @@
|
|||
using System.Dynamic;
|
||||
using Elsa.Expressions.JavaScript.Contracts;
|
||||
using Elsa.Expressions.Models;
|
||||
using Elsa.Testing.Shared;
|
||||
using Jint;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Xunit;
|
||||
using Xunit.Abstractions;
|
||||
|
||||
namespace Elsa.JavaScript.IntegrationTests;
|
||||
|
||||
/// <summary>
|
||||
/// Verifies how CLR objects are exposed to JavaScript: array-like collections should behave like arrays,
|
||||
/// while dictionary-like objects (such as the <c>variables</c> and <c>args</c> containers) should behave
|
||||
/// like plain objects.
|
||||
/// </summary>
|
||||
public class ObjectWrappingTests
|
||||
{
|
||||
private readonly IServiceProvider _serviceProvider;
|
||||
private readonly IJavaScriptEvaluator _evaluator;
|
||||
|
||||
public ObjectWrappingTests(ITestOutputHelper testOutputHelper)
|
||||
{
|
||||
_serviceProvider = new TestApplicationBuilder(testOutputHelper).Build();
|
||||
_evaluator = _serviceProvider.GetRequiredService<IJavaScriptEvaluator>();
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "The variables container is a plain object, not an array")]
|
||||
public async Task VariablesContainerIsNotArrayLike()
|
||||
{
|
||||
Assert.Equal("false", await EvaluateAsync<string>("return '' + (Object.getPrototypeOf(variables) === Array.prototype);"));
|
||||
Assert.Equal("undefined", await EvaluateAsync<string>("return typeof variables.map;"));
|
||||
Assert.Equal("undefined", await EvaluateAsync<string>("return typeof variables.filter;"));
|
||||
Assert.Equal("undefined", await EvaluateAsync<string>("return typeof variables.length;"));
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "A dictionary-like object is a plain object, not an array")]
|
||||
public async Task DictionaryLikeObjectsAreNotArrayLike()
|
||||
{
|
||||
var expando = (IDictionary<string, object?>)new ExpandoObject();
|
||||
expando["greeting"] = "hello";
|
||||
|
||||
Assert.Equal("undefined", await EvaluateAsync<string>("return typeof subject.map;", engine => engine.SetValue("subject", expando)));
|
||||
Assert.Equal("hello", await EvaluateAsync<string>("return subject.greeting;", engine => engine.SetValue("subject", expando)));
|
||||
Assert.Equal("undefined", await EvaluateAsync<string>("return typeof subject.map;", engine => engine.SetValue("subject", new Dictionary<string, object> { ["greeting"] = "hello" })));
|
||||
}
|
||||
|
||||
[Theory(DisplayName = "Array-like CLR collections expose the array prototype")]
|
||||
[InlineData("list")]
|
||||
[InlineData("set")]
|
||||
[InlineData("array")]
|
||||
public async Task ArrayLikeCollectionsExposeArrayPrototype(string name)
|
||||
{
|
||||
Assert.Equal("function", await EvaluateAsync<string>($"return typeof {name}.map;", ConfigureCollections));
|
||||
Assert.Equal("true", await EvaluateAsync<string>($"return '' + (Object.getPrototypeOf({name}) === Array.prototype);", ConfigureCollections));
|
||||
}
|
||||
|
||||
[Theory(DisplayName = "Indexable CLR collections support array iteration methods")]
|
||||
[InlineData("list")]
|
||||
[InlineData("array")]
|
||||
public async Task IndexableCollectionsSupportArrayMethods(string name)
|
||||
{
|
||||
Assert.Equal("2,4,6", await EvaluateAsync<string>($"return {name}.map(x => x * 2).join(',');", ConfigureCollections));
|
||||
Assert.Equal("6", await EvaluateAsync<string>($"return '' + {name}.reduce((a, b) => a + b, 0);", ConfigureCollections));
|
||||
}
|
||||
|
||||
private static void ConfigureCollections(Engine engine)
|
||||
{
|
||||
engine.SetValue("list", new List<int> { 1, 2, 3 });
|
||||
engine.SetValue("set", new HashSet<int> { 1, 2, 3 });
|
||||
engine.SetValue("array", new[] { 1, 2, 3 });
|
||||
}
|
||||
|
||||
private async Task<T?> EvaluateAsync<T>(string script, Action<Engine>? configureEngine = null)
|
||||
{
|
||||
var expressionExecutionContext = new ExpressionExecutionContext(_serviceProvider, new());
|
||||
return (T?)await _evaluator.EvaluateAsync(script, typeof(T), expressionExecutionContext, configureEngine: configureEngine);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue