Skip to content

Add support for recursive loops#4153

Merged
fabpot merged 8 commits into
twigphp:4.xfrom
fabpot:recursive-loops
Jul 28, 2024
Merged

Add support for recursive loops#4153
fabpot merged 8 commits into
twigphp:4.xfrom
fabpot:recursive-loops

Conversation

@fabpot

@fabpot fabpot commented Jul 27, 2024

Copy link
Copy Markdown
Contributor

Fixes #3416
Fixes #4144

@smnandre

Copy link
Copy Markdown
Contributor

Should loop(loop.parent) be forbidden / prevented ?

(or any infinite récursion if this syntax has not the effect i anticipate)

@fabpot fabpot force-pushed the recursive-loops branch 3 times, most recently from e0ebf28 to 8ba877f Compare July 27, 2024 15:09
@fabpot

fabpot commented Jul 27, 2024

Copy link
Copy Markdown
Contributor Author

Should loop(loop.parent) be forbidden / prevented ?

(or any infinite récursion if this syntax has not the effect i anticipate)

My first reaction was that we should not do anything (PHP does nothing to prevent such infinite loops for instance). But as this is a templating engine that can be used by end users that you don't control, I've limited the depth to 50 (50 is kind of arbitrary, but maybe a good balance between valid use cases and malicious usage of a recursive loop).

@fabpot fabpot force-pushed the recursive-loops branch from 42319bc to c9eb8cb Compare July 27, 2024 15:17
Comment thread doc/tags/for.rst Outdated
@fabpot fabpot force-pushed the recursive-loops branch from c9eb8cb to a5fe3de Compare July 28, 2024 08:07
@fabpot fabpot force-pushed the recursive-loops branch from a5fe3de to 481e7f0 Compare July 28, 2024 08:08
@fabpot fabpot merged commit e19f056 into twigphp:4.x Jul 28, 2024
@fabpot fabpot deleted the recursive-loops branch July 28, 2024 08:21
Comment thread src/Runtime/LoopContext.php
fabpot added a commit that referenced this pull request Jul 31, 2024
This PR was merged into the 4.x branch.

Discussion
----------

add type-hints to the LoopContext constructor

see #4153 (comment)

Commits
-------

27e137a add type-hints to the LoopContext constructor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants