From 8a8cd6625c4b22e624cd27c188e80c6487b70e82 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Wed, 29 Jul 2026 14:16:17 +0300 Subject: [PATCH] Map script variables on first read instead of eagerly Every variable was mapped when the engine was set up, once per evaluation, whether or not the script ever looked at it - WritableContext did it in its constructor for the ctx path, and Engine.SetValue did it per variable for the non-context path. Some of those mappings are not cheap: a user variable walks and groups every claim, a content data variable builds a wrapper. A typical script reads a handful of the variables available to it. Both paths now defer the mapping to the first read of the value, through the two APIs Jint 4.15.3 added for exactly this: - PropertyDescriptor.CreateLazy for the ctx object. Unlike a hand-written CustomJsValue descriptor it drops the flag once the value exists, so the descriptor rejoins the write inline cache instead of paying the indirection for the rest of its life. - Engine.Advanced.AddLazyGlobal for the non-context path. The options-time AddLazyGlobal could not serve it - the variables are only known after the engine has been built - and the descriptor a host could install itself is declined by the global-identifier cache. The Advanced overload is documented as being for exactly this case, and its factory may capture engine-affine state. In both cases the property itself is installed eagerly, so nothing about the shape changes: key order, enumeration, `in`, Object.getOwnPropertyNames, delete and the write-through to ScriptVars behave exactly as before, which is what the tests pin - including a counting principal that proves the mapping has not run for a variable the script never mentions, and has run for one it reads. MapVariable reproduces Engine.SetValue's special case for a CLR type so a deferred variable cannot project differently. One edge is worth recording: Engine.SetValue writes through [[Set]] while AddLazyGlobal replaces the descriptor, so a variable named after a non-writable built-in global (undefined, NaN, Infinity) would now shadow it where it was previously ignored. ScriptVars keys are domain names, so this is not reachable in practice. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f --- .../Scripting/Internal/JintExtensions.cs | 20 ++- .../Scripting/WritableContext.cs | 13 +- .../Scripting/JintScriptEngineTests.cs | 147 ++++++++++++++++++ 3 files changed, 177 insertions(+), 3 deletions(-) diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs index 7787448fe..2dc19097d 100644 --- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs +++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs @@ -7,6 +7,7 @@ using Jint; using Jint.Native; +using Jint.Runtime.Interop; using Squidex.Infrastructure; namespace Squidex.Domain.Apps.Core.Scripting.Internal; @@ -73,7 +74,10 @@ public static class JintExtensions { foreach (var (key, item) in vars) { - engine.SetValue(key, item); + // Deferred instead of Engine.SetValue, which maps every variable now. The global itself is + // installed eagerly, so existence checks and enumeration see the name without materializing + // anything; only the mapping waits for the first read of the value. + engine.Advanced.AddLazyGlobal(key, e => MapVariable(e, item)); } } @@ -81,4 +85,18 @@ public static class JintExtensions return context; } + + /// + /// The conversion performs, including its special case for + /// a CLR type, so deferring a variable cannot change what the script sees. + /// + private static JsValue MapVariable(Engine engine, object? item) + { + if (item is Type type) + { + return TypeReference.CreateTypeReference(engine, type); + } + + return JsValue.FromObject(engine, item); + } } diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs index a04b3a255..3ccfcab70 100644 --- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs +++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs @@ -1,4 +1,4 @@ -// ========================================================================== +// ========================================================================== // Squidex Headless CMS // ========================================================================== // Copyright (c) Squidex UG (haftungsbeschraenkt) @@ -8,6 +8,7 @@ using Jint; using Jint.Native; using Jint.Native.Object; +using Jint.Runtime.Descriptors; namespace Squidex.Domain.Apps.Core.Scripting; @@ -20,9 +21,17 @@ internal sealed class WritableContext : ObjectInstance { this.vars = vars; + // Scripts touch a fraction of the variables, but mapping one is not always cheap: a content data + // variable builds a wrapper, a user variable walks and groups every claim. The descriptors are + // installed eagerly - so key order, enumeration and existence checks are exactly what they were - + // and only the mapping waits for the first read of a value. Once it has run the descriptor drops + // back to an ordinary data property and rejoins the write inline cache, which is what a + // hand-written CustomJsValue descriptor cannot do. foreach (var (key, item) in vars) { - base.Set(key, FromObject(engine, item), this); + SetOwnProperty(key, PropertyDescriptor.CreateLazy( + (Engine: engine, Item: item), + static state => FromObject(state.Engine, state.Item))); } } diff --git a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs index 365f44cb3..fec93fd7b 100644 --- a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs @@ -1047,6 +1047,153 @@ public class JintScriptEngineTests : IClassFixture Assert.Equal(JsonValue.Create("number,json,user"), actual); } + [Fact] + public void Should_not_map_unread_variable() + { + var principal = new CountingPrincipal(); + + var vars = new ScriptVars + { + ["number"] = 13, + ["user"] = principal, + }; + + const string script = @" + number + 1; + "; + + var actual = sut.Execute(vars, script); + + Assert.Equal(JsonValue.Create(14), actual); + Assert.Equal(0, principal.Reads); + } + + [Fact] + public void Should_see_unread_variable_in_enumeration() + { + var principal = new CountingPrincipal(); + + var vars = new ScriptVars + { + ["user"] = principal, + }; + + const string script = @" + ('user' in globalThis) + ',' + (Object.getOwnPropertyNames(globalThis).indexOf('user') >= 0); + "; + + var actual = sut.Execute(vars, script); + + Assert.Equal(JsonValue.Create("true,true"), actual); + Assert.Equal(0, principal.Reads); + } + + [Fact] + public void Should_map_variable_on_first_read() + { + var principal = new CountingPrincipal(); + + var vars = new ScriptVars + { + ["user"] = principal, + }; + + const string script = @" + user.id; + "; + + var actual = sut.Execute(vars, script); + + Assert.Equal(JsonValue.Create("user1"), actual); + Assert.True(principal.Reads > 0); + } + + [Fact] + public void Should_not_map_unread_context_variable() + { + var principal = new CountingPrincipal(); + + var vars = new ScriptVars + { + ["number"] = 13, + ["user"] = principal, + }; + + const string script = @" + ctx.number + 1; + "; + + var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true }); + + Assert.Equal(JsonValue.Create(14), actual); + Assert.Equal(0, principal.Reads); + } + + [Fact] + public void Should_see_unread_context_variable_in_enumeration() + { + var principal = new CountingPrincipal(); + + var vars = new ScriptVars + { + ["number"] = 13, + ["user"] = principal, + }; + + const string script = @" + Object.keys(ctx).join(',') + '|' + ('user' in ctx); + "; + + var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true }); + + Assert.Equal(JsonValue.Create("number,user|true"), actual); + Assert.Equal(0, principal.Reads); + } + + [Fact] + public void Should_map_context_variable_on_first_read() + { + var principal = new CountingPrincipal(); + + var vars = new ScriptVars + { + ["user"] = principal, + }; + + const string script = @" + ctx.user.id; + "; + + var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true }); + + Assert.Equal(JsonValue.Create("user1"), actual); + Assert.True(principal.Reads > 0); + } + + private sealed class CountingPrincipal : ClaimsPrincipal + { + public int Reads { get; private set; } + + public CountingPrincipal() + : base(new ClaimsIdentity( + [ + new Claim(OpenIdClaims.Subject, "user1"), + new Claim(OpenIdClaims.Name, "user"), + ], "Squidex")) + { + } + + public override IEnumerable Claims + { + get + { + Reads++; + + return base.Claims; + } + } + } + private static ScriptVars CreateVars() { return new ScriptVars