Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

index out of bounds bug #29

Open
raisinrand opened this issue Jan 1, 2025 · 2 comments
Open

index out of bounds bug #29

raisinrand opened this issue Jan 1, 2025 · 2 comments

Comments

@raisinrand
Copy link

hi, there is a bug when parsing this template in non-comptime mode
{{#a}} {{#b}}{{/b}}{{/a}}

this panics when rendering:

thread 148525 panic: index out of bounds: index 3, len 2
/home/randy/.cache/zig/p/1220897a76b954e9811b760ba8e465d6c2c41ad3f69c8ce30c708e24af8ff12cb182/src/rendering/rendering.zig:1189:62: 0x10a2b0a in levelCapacityHint (codegen)
                            const section_children = elements[index .. index + section.children_count];
                                                             ^
/home/randy/.cache/zig/p/1220897a76b954e9811b760ba8e465d6c2c41ad3f69c8ce30c708e24af8ff12cb182/src/rendering/rendering.zig:804:69: 0x10a26fa in render (codegen)
                        const capacity_hint = self.levelCapacityHint(elements);
                                                                    ^
/home/randy/.cache/zig/p/1220897a76b954e9811b760ba8e465d6c2c41ad3f69c8ce30c708e24af8ff12cb182/src/rendering/rendering.zig:1375:35: 0x10a6758 in bufRender__anon_9310 (codegen)
            try data_render.render(template.elements);
                                  ^
/home/randy/.cache/zig/p/1220897a76b954e9811b760ba8e465d6c2c41ad3f69c8ce30c708e24af8ff12cb182/src/rendering/rendering.zig:655:31: 0x10a68d6 in internalAllocRender__anon_9309 (codegen)
    try RenderEngine.bufRender(list.writer(), template, data, PartialsMap.init(partials));
                              ^
/home/randy/.cache/zig/p/1220897a76b954e9811b760ba8e465d6c2c41ad3f69c8ce30c708e24af8ff12cb182/src/rendering/rendering.zig:148:35: 0x107144c in allocRenderPartialsWithOptions__anon_5943 (codegen)
    return try internalAllocRender(allocator, template, partials, data, render_options, null);
                                  ^
/home/randy/.cache/zig/p/1220897a76b954e9811b760ba8e465d6c2c41ad3f69c8ce30c708e24af8ff12cb182/src/rendering/rendering.zig:108:46: 0x106151c in allocRender__anon_5440 (codegen)
    return try allocRenderPartialsWithOptions(allocator, template, {}, data, .{});
                                             ^

it looks like this happens because in parser.zig we omit empty nodes in produceNodes, but if this occurs in a section, we don't update the children_count for the section
the space in the template is empty after trimming whitespace

for (nodes.items) |*node| {
    if (!node.text_part.isEmpty()) {
        list.appendAssumeCapacity(try self.createElement(node));
    }
}

i think an easy fix is to just get rid of this check, unless it's necessary for some reason?

@batiati
Copy link
Owner

batiati commented Jan 2, 2025

Hey, @raisinrand, can you write a unit test to reproduce it?
I tried this one, but it passed:

            test "Sections: Empty nodes" {
                const template_text = "{{#section}}A{{#empty}}{{/empty}}{{/section}}";
                const expected = "A";

                const Data = struct { section: bool, empty: bool };
                const data = Data{ .section = true, .empty = false };
                try expectRender(template_text, &data, expected);
            }

IIRC, empty nodes are omitted mostly because of the streamed render. To fix this bug we could just check section.children_count == 0.

@raisinrand
Copy link
Author

ah, the bug happens if there is a whitespace-only text node in a section, not if there is an empty section

            test "Sections: Empty node in section" {
                const template_text = "{{#section}} {{node}}{{/section}}";
                const expected = "";

                const Data = struct { section: bool, node: bool };
                const data = Data{ .section = true, .node = false };
                try expectRender(template_text, &data, expected);
            }

this version fails

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants