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

Check start tags were matched to an end tag #923

Merged
merged 3 commits into from
Dec 2, 2024

Conversation

paulharris
Copy link
Contributor

Template:

{#username}
{^username}

Context:
{ "username": "" }

The problem appears to be in ActionType::ElseBlock, which find the context, but doesn't have a switch case for json::type::String, and instead sets current=action.pos

current = action.pos;

For this template, there are 2 actions.
At this point we are on the second action (current=1)
But this action's "pos" is = 0.
So, current is reset to zero, and it loops through again and again, never progressing.

This seems related to #899
I think the problem is pos is never set to a valid point... this is done in case'/', ie when the parser sees the end tag.
But, if there is never an end-tag, it never checks if there are unmatched pairs, and pos is never set.

So, I've adjusted the code so it always expects a non-negative pos.
pos is initialised by default to -1 for start tags.
And end tag will match it and set it to a valid pos.

For all other normal tags, it is initialised to zero.

THIS was a quick first-pass patch, as I'm not familiar with mustache, so I imagine this patch is NOT COMPLETE.
It needs to be run through a test suite.

@gittiver
Copy link
Member

Are there already tests for this cases?

@gittiver
Copy link
Member

actually some tests fail

@paulharris paulharris force-pushed the fix-mustache-unmatched branch from 3055ee8 to b90a739 Compare October 18, 2024 02:17
@paulharris
Copy link
Contributor Author

Sorry about that, I didn't check the tests.
I've forced-pushed, start again...
I've fixed some unsigned/signed warnings (be consistent if using static_cast() ... in C++20 can switch to std::ssize()).
I've added a test for the mustache - in a new JSON test as I assume the existing json files are from the mustache spec test dataset?
I've adjusted my method for detecting missing end tags, improved the error message, and it now works.

You can see the infinite loop by checking out this branch, but wind back one commit to before the fix.

@paulharris
Copy link
Contributor Author

I've been using my fix with success, just need a review to push this through.

@gittiver gittiver merged commit a8ca7ed into CrowCpp:master Dec 2, 2024
11 checks passed
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

Successfully merging this pull request may close these issues.

3 participants