Handlebars-Net / Handlebars.Net

A real .NET Handlebars engine
MIT License
1.26k stars 217 forks source link

IndexOutOfRangeException in HandlebarsDotNet.PathStructure.PathResolver.ResolvePath for nested object path block expression #442

Closed swijnands closed 2 years ago

swijnands commented 3 years ago

Describe the bug

Trying to enumerate an array via a nested object path:

{{#bundles.styles.vendor}}{{this}}{{/bundles.styles.vendor}}

Results in an IndexOutOfRangeException exception.

Expected behavior:

I would expect the same result as when not using a nested path, i.e.

{{#arr}}....{/arr}}

Or, alternatively, if this is not supported: a clearer exception message.

Test to reproduce

var context = new
{
  bundles = new
  {
      styles = new
      {
          vendor = new[] { "a", "b", "c" }
      }
  },
  arr = new[] { "d", "e", "f"}
};

// Works: Output = "abc"
Handlebars.Compile("{{#each bundles.styles.vendor}}{{this}}{{/each}}")(context).Dump();

// Works: Output = "def"
Handlebars.Compile("{{#arr}}{{this}}{{/arr}}")(context).Dump();

// Fails: IndexOutOfRangeException in HandlebarsDotNet.PathStructure.PathResolver.ResolvePath
// On line #36: if (!TryResolveValue(pathInfo.IsVariable, context, pathChain[0], instance, out instance))
Handlebars.Compile("{{#bundles.styles.vendor}}{{this}}{{/bundles.styles.vendor}}")(context).Dump();```
oformaniuk commented 3 years ago

This should definitely be supported. Looks like something weird is happening when we're trying to resolve an actual array to enumerate on.

sushantduggal commented 3 years ago

Facing the same issue, is there a resolution on this?

devzaak commented 3 years ago

I am not 100% sure but this could be a fix for issue: In: PathResolver.cs replacing

            var segments = pathInfo.Segments;

with

            var segments = pathInfo.Segments.Where(segment => segment.IsNotEmpty).ToArray();

I am unsure if this will cause other problems though as I am not familiar with lib yet.

oformaniuk commented 3 years ago

@aslezak , it's hard to tell without testing. I'd really appreciate if you can have a look closer.

nit: Try to avoid Linq as this is performance critical path. In this case the check can be placed inside of for that is right below the line you mentioned.

devzaak commented 3 years ago

Will do as soon as I have some spare time, and yes, I noticed I can do continue; in the loop instead of prefiltering them.

abraham-fox commented 2 years ago

No updates for 5 month - I assume I can grab this :) and it seems I've found a simple solution for the issue. @zjklee please take a look at #488.