Skip to content

Commit

Permalink
Fix cyclic input argument evaluation (elsa-workflows#3755)
Browse files Browse the repository at this point in the history
* Prevent stack overflow when evaluating input arguments

* Shuffle test folders
  • Loading branch information
sfmskywalker committed Mar 2, 2023
1 parent 67efc51 commit 1e407a9
Show file tree
Hide file tree
Showing 35 changed files with 138 additions and 52 deletions.
37 changes: 23 additions & 14 deletions Elsa.sln
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.MassTransit", "src\mod
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{90031D64-CA0F-46D0-9AF4-8DC023A5FFCD}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.IntegrationTests", "test\Elsa.IntegrationTests\Elsa.IntegrationTests.csproj", "{B066ED0A-86E0-4025-85C2-7E8F7139D8A4}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.UnitTests", "test\Elsa.UnitTests\Elsa.UnitTests.csproj", "{85A6862B-E523-45FA-A26B-BF98BE140E89}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Labels", "src\modules\Elsa.Labels\Elsa.Labels.csproj", "{80FF5821-E831-450D-AA0C-C76D07D878F4}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "bundles", "bundles", "{F06B9573-DF68-4606-866C-A7546A10A05A}"
Expand Down Expand Up @@ -161,6 +157,16 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Samples.ElasticsearchS
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Samples.OutboundHttpRequests", "src\samples\console\Elsa.Samples.OutboundHttpRequests\Elsa.Samples.OutboundHttpRequests.csproj", "{39AB433B-F7A6-4DD3-873A-9527946B7BD8}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "unit", "unit", "{18453B51-25EB-4317-A4B3-B10518252E92}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "integration", "integration", "{1B8D5897-902E-4632-8698-E89CAF3DDF54}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "component", "component", "{08B41FFA-CEE3-46A7-B5C0-3EB65D37A16C}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.JavaScript.UnitTests", "test\unit\Elsa.JavaScript.UnitTests\Elsa.JavaScript.UnitTests.csproj", "{0BB927FB-8C12-49B6-9150-64B733B2EBFD}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.IntegrationTests", "test\integration\Elsa.IntegrationTests\Elsa.IntegrationTests.csproj", "{E9652738-2B3D-4357-B84B-54F0EA161382}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -199,14 +205,6 @@ Global
{BB983D0B-A939-4008-B9E3-C7C172BCF83A}.Debug|Any CPU.Build.0 = Debug|Any CPU
{BB983D0B-A939-4008-B9E3-C7C172BCF83A}.Release|Any CPU.ActiveCfg = Release|Any CPU
{BB983D0B-A939-4008-B9E3-C7C172BCF83A}.Release|Any CPU.Build.0 = Release|Any CPU
{B066ED0A-86E0-4025-85C2-7E8F7139D8A4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{B066ED0A-86E0-4025-85C2-7E8F7139D8A4}.Debug|Any CPU.Build.0 = Debug|Any CPU
{B066ED0A-86E0-4025-85C2-7E8F7139D8A4}.Release|Any CPU.ActiveCfg = Release|Any CPU
{B066ED0A-86E0-4025-85C2-7E8F7139D8A4}.Release|Any CPU.Build.0 = Release|Any CPU
{85A6862B-E523-45FA-A26B-BF98BE140E89}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{85A6862B-E523-45FA-A26B-BF98BE140E89}.Debug|Any CPU.Build.0 = Debug|Any CPU
{85A6862B-E523-45FA-A26B-BF98BE140E89}.Release|Any CPU.ActiveCfg = Release|Any CPU
{85A6862B-E523-45FA-A26B-BF98BE140E89}.Release|Any CPU.Build.0 = Release|Any CPU
{80FF5821-E831-450D-AA0C-C76D07D878F4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{80FF5821-E831-450D-AA0C-C76D07D878F4}.Debug|Any CPU.Build.0 = Debug|Any CPU
{80FF5821-E831-450D-AA0C-C76D07D878F4}.Release|Any CPU.ActiveCfg = Release|Any CPU
Expand Down Expand Up @@ -411,15 +409,21 @@ Global
{39AB433B-F7A6-4DD3-873A-9527946B7BD8}.Debug|Any CPU.Build.0 = Debug|Any CPU
{39AB433B-F7A6-4DD3-873A-9527946B7BD8}.Release|Any CPU.ActiveCfg = Release|Any CPU
{39AB433B-F7A6-4DD3-873A-9527946B7BD8}.Release|Any CPU.Build.0 = Release|Any CPU
{0BB927FB-8C12-49B6-9150-64B733B2EBFD}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{0BB927FB-8C12-49B6-9150-64B733B2EBFD}.Debug|Any CPU.Build.0 = Debug|Any CPU
{0BB927FB-8C12-49B6-9150-64B733B2EBFD}.Release|Any CPU.ActiveCfg = Release|Any CPU
{0BB927FB-8C12-49B6-9150-64B733B2EBFD}.Release|Any CPU.Build.0 = Release|Any CPU
{E9652738-2B3D-4357-B84B-54F0EA161382}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{E9652738-2B3D-4357-B84B-54F0EA161382}.Debug|Any CPU.Build.0 = Debug|Any CPU
{E9652738-2B3D-4357-B84B-54F0EA161382}.Release|Any CPU.ActiveCfg = Release|Any CPU
{E9652738-2B3D-4357-B84B-54F0EA161382}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(NestedProjects) = preSolution
{155227F0-A33B-40AA-A4B4-06F813EB921B} = {61017E64-6D00-49CB-9E81-5002DC8F7D5F}
{C6658DE0-2B2F-47F0-BB61-2CA66D435C09} = {61017E64-6D00-49CB-9E81-5002DC8F7D5F}
{873BFC3E-63C2-4495-A503-5EC05DCD84E4} = {155227F0-A33B-40AA-A4B4-06F813EB921B}
{56C2FFB8-EA54-45B5-A095-4A78142EB4B5} = {155227F0-A33B-40AA-A4B4-06F813EB921B}
{5BA4A8FA-F7F4-45B3-AEC8-8886D35AAC79} = {61017E64-6D00-49CB-9E81-5002DC8F7D5F}
{B066ED0A-86E0-4025-85C2-7E8F7139D8A4} = {90031D64-CA0F-46D0-9AF4-8DC023A5FFCD}
{85A6862B-E523-45FA-A26B-BF98BE140E89} = {90031D64-CA0F-46D0-9AF4-8DC023A5FFCD}
{F06B9573-DF68-4606-866C-A7546A10A05A} = {61017E64-6D00-49CB-9E81-5002DC8F7D5F}
{B2049499-D384-46DF-8837-F1180107DD54} = {5BA4A8FA-F7F4-45B3-AEC8-8886D35AAC79}
{FC5D2CE9-FFB0-478D-8D54-E4109D1FD202} = {F06B9573-DF68-4606-866C-A7546A10A05A}
Expand Down Expand Up @@ -481,5 +485,10 @@ Global
{C25CFDCF-8E06-4DD8-A83E-FF7EF9FDA02E} = {56C2FFB8-EA54-45B5-A095-4A78142EB4B5}
{4A54379F-4775-41B6-9752-FF300288916A} = {56C2FFB8-EA54-45B5-A095-4A78142EB4B5}
{39AB433B-F7A6-4DD3-873A-9527946B7BD8} = {873BFC3E-63C2-4495-A503-5EC05DCD84E4}
{18453B51-25EB-4317-A4B3-B10518252E92} = {90031D64-CA0F-46D0-9AF4-8DC023A5FFCD}
{1B8D5897-902E-4632-8698-E89CAF3DDF54} = {90031D64-CA0F-46D0-9AF4-8DC023A5FFCD}
{08B41FFA-CEE3-46A7-B5C0-3EB65D37A16C} = {90031D64-CA0F-46D0-9AF4-8DC023A5FFCD}
{0BB927FB-8C12-49B6-9150-64B733B2EBFD} = {18453B51-25EB-4317-A4B3-B10518252E92}
{E9652738-2B3D-4357-B84B-54F0EA161382} = {1B8D5897-902E-4632-8698-E89CAF3DDF54}
EndGlobalSection
EndGlobal
13 changes: 11 additions & 2 deletions src/modules/Elsa.JavaScript/Features/JavaScriptFeature.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ public JavaScriptFeature(IModule module) : base(module)
/// <inheritdoc />
public override void Configure()
{
Module.UseWorkflowManagement(management => management.AddActivitiesFrom<JavaScriptFeature>());
}

/// <inheritdoc />
public override void Apply()
{
// JavaScript services.
Services
.AddSingleton<IExpressionSyntaxProvider, JavaScriptExpressionSyntaxProvider>()
.AddSingleton<IJavaScriptEvaluator, JintJavaScriptEvaluator>()
Expand All @@ -41,6 +48,7 @@ public override void Configure()
.AddExpressionHandler<JavaScriptExpressionHandler, JavaScriptExpression>()
;

// Type definition services.
Services
.AddSingleton<ITypeDefinitionService, TypeDefinitionService>()
.AddSingleton<ITypeDescriber, TypeDescriber>()
Expand All @@ -50,7 +58,8 @@ public override void Configure()
.AddSingleton<ITypeDefinitionProvider, CommonTypeDefinitionProvider>()
.AddSingleton<ITypeDefinitionProvider, VariableTypeDefinitionProvider>()
;

Module.UseWorkflowManagement(management => management.AddActivitiesFrom<JavaScriptFeature>());

// Handlers.
Services.AddNotificationHandlersFrom<JavaScriptFeature>();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
using Elsa.Expressions.Models;
using Elsa.Expressions.Services;
using Elsa.Extensions;
using Elsa.JavaScript.Notifications;
using Elsa.Mediator.Services;
using Elsa.Workflows.Core.Models;
using Elsa.Workflows.Management.Activities;
using Elsa.Workflows.Management.Extensions;
using Elsa.Workflows.Management.Services;
using Humanizer;
using JetBrains.Annotations;
using Jint;

namespace Elsa.JavaScript.Handlers;

/// <summary>
/// Configures the JavaScript engine with workflow input getters.
/// </summary>
[PublicAPI]
public class WorkflowDefinitionActivityJavaScriptHandler : INotificationHandler<EvaluatingJavaScript>
{
private readonly IActivityRegistry _activityRegistry;
private readonly IExpressionEvaluator _expressionEvaluator;

/// <summary>
/// Constructor.
/// </summary>
public WorkflowDefinitionActivityJavaScriptHandler(IActivityRegistry activityRegistry, IExpressionEvaluator expressionEvaluator)
{
_activityRegistry = activityRegistry;
_expressionEvaluator = expressionEvaluator;
}

/// <inheritdoc />
public async Task HandleAsync(EvaluatingJavaScript notification, CancellationToken cancellationToken)
{
var engine = notification.Engine;
var context = notification.Context;

// If we are already evaluating inputs, then we're in a circular evaluation loop. In this case, we should not attempt to evaluate the inputs.
if(context.TransientProperties.TryGetValue("EvaluatingInputs", out var evaluatingInputs) && (bool)evaluatingInputs)
return;

// To prevent a circular evaluation loop, set a flag on the context to indicate that we're currently evaluating the inputs.
context.TransientProperties["EvaluatingInputs"] = true;

// Create input getters.
await CreateInputAccessorsAsync(engine, context);
}

private async Task CreateInputAccessorsAsync(Engine engine, ExpressionExecutionContext context)
{
var workflowDefinitionActivity = GetFirstWorkflowDefinitionActivity(context);

if (workflowDefinitionActivity == null)
return;

var descriptor = _activityRegistry.Find(workflowDefinitionActivity.Type, workflowDefinitionActivity.Version)!;
var inputDefinitions = descriptor.Inputs;

foreach (var inputDefinition in inputDefinitions)
{
var inputPascalName = inputDefinition.Name.Pascalize();
var input = workflowDefinitionActivity.SyntheticProperties.TryGetValue(inputDefinition.Name, out var inputValue) ? (Input?)inputValue : default;
var evaluatedExpression = input != null ? await _expressionEvaluator.EvaluateAsync(input, context) : input;

engine.SetValue($"get{inputPascalName}", (Func<object?>)(() => evaluatedExpression));
}
}

private static WorkflowDefinitionActivity? GetFirstWorkflowDefinitionActivity(ExpressionExecutionContext context) =>
context.GetActivityExecutionContext().GetFirstWorkflowDefinitionActivity();
}
25 changes: 0 additions & 25 deletions src/modules/Elsa.JavaScript/Services/JintJavaScriptEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ private async Task<Engine> GetConfiguredEngine(Action<Engine>? configureEngine,
// Create variable & input setters and getters for each variable.
CreateMemoryBlockAccessors(engine, context);

// Create input getters.
await CreateInputAccessorsAsync(engine, context);

engine.SetValue("isNullOrWhiteSpace", (Func<string, bool>)(value => string.IsNullOrWhiteSpace(value)));
engine.SetValue("isNullOrEmpty", (Func<string, bool>)(value => string.IsNullOrEmpty(value)));
Expand All @@ -93,29 +91,6 @@ private async Task<Engine> GetConfiguredEngine(Action<Engine>? configureEngine,

return engine;
}

private async Task CreateInputAccessorsAsync(Engine engine, ExpressionExecutionContext context)
{
var workflowDefinitionActivity = GetFirstWorkflowDefinitionActivity(context);

if (workflowDefinitionActivity == null)
return;

var descriptor = _activityRegistry.Find(workflowDefinitionActivity.Type, workflowDefinitionActivity.Version)!;
var inputDefinitions = descriptor.Inputs;

foreach (var inputDefinition in inputDefinitions)
{
var inputPascalName = inputDefinition.Name.Pascalize();
var input = workflowDefinitionActivity.SyntheticProperties.TryGetValue(inputDefinition.Name, out var inputValue) ? (Input?)inputValue : default;
var evaluatedExpression = input != null ? await _expressionEvaluator.EvaluateAsync(input, context) : input;

engine.SetValue($"get{inputPascalName}", (Func<object?>)(() => evaluatedExpression));
}
}

private static WorkflowDefinitionActivity? GetFirstWorkflowDefinitionActivity(ExpressionExecutionContext context) =>
context.GetActivityExecutionContext().GetFirstWorkflowDefinitionActivity();

private static void CreateMemoryBlockAccessors(Engine engine, ExpressionExecutionContext context)
{
Expand Down
11 changes: 0 additions & 11 deletions test/Elsa.UnitTests/UnitTest1.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.5.0" />
<PackageReference Include="Moq" Version="4.18.4" />
<PackageReference Include="xunit" Version="2.4.2" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand All @@ -19,4 +20,8 @@
</PackageReference>
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\modules\Elsa.JavaScript\Elsa.JavaScript.csproj" />
</ItemGroup>

</Project>
26 changes: 26 additions & 0 deletions test/unit/Elsa.JavaScript.UnitTests/UnitTest1.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using Elsa.Expressions.Services;
using Elsa.JavaScript.Handlers;
using Elsa.Workflows.Management.Services;
using Moq;
using Xunit;

namespace Elsa.JavaScript.UnitTests;

public class WorkflowDefinitionActivityJavaScriptHandlerTests
{
private readonly WorkflowDefinitionActivityJavaScriptHandler _handler;

public WorkflowDefinitionActivityJavaScriptHandlerTests()
{
var activityRegistryMock = new Mock<IActivityRegistry>();
var expressionEvaluatorMock = new Mock<IExpressionEvaluator>();
_handler = new WorkflowDefinitionActivityJavaScriptHandler(activityRegistryMock.Object, expressionEvaluatorMock.Object);
}

[Fact]
public void Test1()
{

}

}

0 comments on commit 1e407a9

Please sign in to comment.