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

Shift + Enter between two IF conditions #340

Closed
matcobat opened this issue Dec 11, 2023 · 10 comments
Closed

Shift + Enter between two IF conditions #340

matcobat opened this issue Dec 11, 2023 · 10 comments

Comments

@matcobat
Copy link

Hi,

First, I would like to say that I really love docx-templates !
I recently found a bug, that makes the "createReport" method looping in an infinite way (so, my application do not answer anymore :().

I am actually using the NodeJS version.

Here is an example of a template that cause the bug.
image

If I replace the "Shift Enter" by a simple "Enter" after the first IF, it is working fine.
On my application, since I let users to create their own templates, I can not check all the templates. Since this is completely crashing my app, it is very annoying.

if it could work, of throw an error, it would be fantastic !

Thanks !

@KingofGnome
Copy link

On my application, since I let users to create their own templates, I can not check all the templates.

Similar in my case, was fighting endless loops aswell. I think in the end I've monkeypatched an loop-counter in there, to at least stop the loop and have a proper error handling.

Let me check tomorrow on my workaround, mby it helps you.
Would be awesome to get rid of this bug so.

@KingofGnome
Copy link

Possibly related to #154?
(IF's are only for loops with 0 or 1 iteration)

Hi all, I encountered the same issue, with two IF on the same line (version 4.11.3). Here is a template that blocks my program:

+++ IF variable1 +++ +++ IF variable2 +++
Hello
+++ END-IF +++
+++ END-IF +++

@matcobat
Copy link
Author

Hi @KingofGnome,
You're right, it is certainly the same problem.

@dseiler
Copy link

dseiler commented Feb 7, 2024

@matcobat @KingofGnome I was also able to reproduce this issue. Looks like this is a big problem for us now because we cannot guarantee that our users are not creating such a corrupt template.
@jjhbw could we treat this issue with priority? I could also help to resolve. Any idea where I should start? Thanks a lot for your support!!

@jjhbw
Copy link
Collaborator

jjhbw commented Feb 8, 2024

I don't know of an immediate solution to this. Because the result of this bug is an infinite loop, the most likely culprit is the walkTemplate function (because of its while loop)

export async function walkTemplate(

This function is very much 'core' to the application and hence challenging to change without breaking other behaviour. If you want to give this a go, we have a pretty solid suite of snapshot tests to use as guardrails.

I would start by creating a snapshot test for your template that triggers the infinite loop (using a Jest timeout or by instrumenting runJS to make it fail after e.g. 1000 executions) and then developing against that until the test case is green. I fixed a similar infinite loop earlier #213, but that didn't require changes to walkTemplate.

@dseiler
Copy link

dseiler commented Feb 8, 2024

Thanks for the hint. I did a first debugging and found that the while(true) loop at line 183 in the processTemplate.ts file is never exiting. There is only one "break" statement in this loop at line 217 which could exit this loop. This statement is never true so we have an infinite loop.

processTemplate.ts , line 215-221
} else { const parent = nodeIn._parent; if (parent == null) break; nodeIn = parent; ctx.level -= 1; move = 'UP'; }
So, it looks like the nodeIn._parent is never null if the two IF statements in the template are on the same line. Any idea why this could happen and how to fix? Thanks!

@KingofGnome
Copy link

Thanks for the hint. I did a first debugging and found that the while(true) loop at line 183 in the processTemplate.ts file is never exiting. There is only one "break" statement in this loop at line 217 which could exit this loop. This statement is never true so we have an infinite loop.

processTemplate.ts , line 215-221 } else { const parent = nodeIn._parent; if (parent == null) break; nodeIn = parent; ctx.level -= 1; move = 'UP'; } So, it looks like the nodeIn._parent is never null if the two IF statements in the template are on the same line. Any idea why this could happen and how to fix? Thanks!

Mby try to unzip two similar docx files and compare both xml's.
Never had the time to figure out the difference between return and shift return in dicx xml struct.

@dseiler
Copy link

dseiler commented Feb 16, 2024

I did some in depth debugging on this issue and I found that two nested IF commands within the same paragraph (shift enter does not create a new paragraph) is causing the walkTemplate function on the processTemplate.ts to jump to the wrong node and therefore enter in an endless loop.
After deep analysis I came to the conclusion that this issue probably cannot be solved because when two IF statements are following each other within the same w:p or w:tr tag the current logic has now way to find out that the first END-IF statement actually belongs to the second nested IF and not to the first IF as usual.
I therefore think the "fix" for this issue is to validate the template so that such a nested IF will not be allowed within the same w:p or w:tr tags.
@jjhbw I enhanced the code to throw an exception if such a nested IF is found in the template. How could I send my suggestion to you? Can I do a pull request? I created also a jest test for this issue and all tests are green.

@jjhbw
Copy link
Collaborator

jjhbw commented Feb 17, 2024

Pull requests are welcome and very much appreciated, especially if they come with tests!

@jjhbw
Copy link
Collaborator

jjhbw commented Mar 21, 2024

Should be fixed thanks to @dseiler #353

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

4 participants