From 17fb28a12369987f708219db505bb2f4a97af5c1 Mon Sep 17 00:00:00 2001 From: jsneedles Date: Thu, 8 Oct 2026 11:54:41 -0400 Subject: [PATCH] Block parameter fix fix: resolve block params named like @-variables by name Unrelated to the JsonNode changes in this PR. The new JSON array iteration tests use `as |item index|`, which exposed this bug as an intermittent CI failure. A block param that shares a name with a context variable (e.g. `{{#each items as |item index|}}`) was looked up in BlockParamsObject using the entry index of the matching @-variable in ContextDataObject. The two dictionaries usually place the key in the same slot, so this mostly worked, but when a hash collision moved the @-variable's slot, the lookup returned another block param's value or nothing. String hashes are randomized per process, so the failure was intermittent on every OS. Look block params up by segment, as TryGetContextVariable already does. Adds a regression test covering index, key, first, last, root and parent. Because the bug depends on the process hash seed, the test cannot fail deterministically without the fix. --- source/Handlebars.Test/BasicIntegrationTests.cs | 15 +++++++++++++++ source/Handlebars/BindingContext.cs | 11 +++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/source/Handlebars.Test/BasicIntegrationTests.cs b/source/Handlebars.Test/BasicIntegrationTests.cs index d5211f82..5890f4cb 100644 --- a/source/Handlebars.Test/BasicIntegrationTests.cs +++ b/source/Handlebars.Test/BasicIntegrationTests.cs @@ -991,6 +991,21 @@ public void DictionaryEnumeratorWithBlockParams(IHandlebars handlebars) Assert.Equal("hello foo world bar ", result); } + [Theory, ClassData(typeof(HandlebarsEnvGenerator))] + public void BlockParamsNamedLikeContextVariables(IHandlebars handlebars) + { + // Block params sharing a name with @-variables used to be looked up by the @-variable's slot in a + // different dictionary, so whether they resolved depended on the process's string hash seed. + var data = new { items = new List { "a", "b" } }; + + foreach (var name in new[] { "index", "key", "first", "last", "root", "parent" }) + { + var template = handlebars.Compile($"{{{{#each items as |item {name}|}}}}{{{{{name}}}}}:{{{{item}}}};{{{{/each}}}}"); + + Assert.Equal("0:a;1:b;", template(data)); + } + } + [Theory, ClassData(typeof(HandlebarsEnvGenerator))] public void DictionaryWithLastEnumerator(IHandlebars handlebars) { diff --git a/source/Handlebars/BindingContext.cs b/source/Handlebars/BindingContext.cs index 42b1b38b..0240f1e7 100644 --- a/source/Handlebars/BindingContext.cs +++ b/source/Handlebars/BindingContext.cs @@ -147,14 +147,9 @@ out WellKnownVariables[(int) WellKnownVariable.Parent] internal bool TryGetVariable(ChainSegment segment, [MaybeNullWhen(false)] out object value) { - if (segment.WellKnownVariable != WellKnownVariable.None) - { - var wellKnownVariable = WellKnownVariables[(int) segment.WellKnownVariable]; - return BlockParamsObject.TryGetValue(wellKnownVariable, out value) - || (Descriptor.Value.MemberAccessor?.TryGetValue(Value!, segment, out value) ?? false); - } - - return BlockParamsObject.TryGetValue(segment, out value) + // Block params are looked up by segment even for well-known names (e.g. `as |item index|`): + // WellKnownVariables holds entry indexes into ContextDataObject, not BlockParamsObject. + return BlockParamsObject.TryGetValue(segment, out value) || (Descriptor.Value.MemberAccessor?.TryGetValue(Value!, segment, out value) ?? false); }