Problem/Motivation
Since updating phpstan and twig via Composer, a blank page appears when editing any node.
Enabling xdebug reveals that the new recursive logic in twig is hitting the recursion limit in Drupal render arrays, particularly forms.
Error: Xdebug has detected a possible infinite loop, and aborted your script with a stack depth of '1000' frames in Twig\Extension\SandboxExtension->isSandboxed() (line 56 of /var/www/html/vendor/twig/twig/src/Extension/SandboxExtension.php).
Twig\Extension\SandboxExtension->ensureToStringAllowed() (Line: 124)
Twig\Extension\SandboxExtension->ensureToStringAllowed() (Line: 93)
__TwigTemplate_13b75ab4e98e9076c88c6eb888b04ed5->doDisplay() (Line: 393)
Twig\Template->yield() (Line: 349)
Twig\Template->display() (Line: 364)
Twig\Template->render() (Line: 35)
Twig\TemplateWrapper->render() (Line: 33)
twig_render_template() (Line: 348)
Drupal\Core\Theme\ThemeManager->render() (Line: 446)
Drupal\Core\Render\Renderer->doRender() (Line: 459)
Drupal\Core\Render\Renderer->doRender() (Line: 459)
Drupal\Core\Render\Renderer->doRender() (Line: 203)
Drupal\Core\Render\Renderer->render() (Line: 238)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 593)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 231)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare() (Line: 128)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse() (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray() (Line: 246)
Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}() (Line: 206)
Symfony\Component\EventDispatcher\EventDispatcher->callListeners() (Line: 56)
Symfony\Component\EventDispatcher\EventDispatcher->dispatch() (Line: 188)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 709)
Drupal\Core\DrupalKernel->handle() (Line: 19)
Steps to reproduce
user@server:~/public_html$ composer update -W
Loading composer repositories with package information
Updating dependencies
Lock file operations: 0 installs, 2 updates, 0 removals
- Upgrading phpstan/phpstan (1.12.7 => 1.12.8)
- Upgrading twig/twig (v3.14.0 => v3.14.1)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 0 installs, 2 updates, 0 removals
- Downloading twig/twig (v3.14.1)
- Downloading phpstan/phpstan (1.12.8)
- Upgrading twig/twig (v3.14.0 => v3.14.1): Extracting archive
- Upgrading phpstan/phpstan (1.12.7 => 1.12.8): Extracting archive
Generating autoload files
48 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
No security vulnerability advisories found.
Using D10.3.6 with PHP 8.3.13
Proposed resolution
Update twig/twig dependency to 3.14.2 when it is released since 3.14.0 is out-of-date anyway.
Issue fork branches:
- 10.3.x: 3485956-11.3.x
- 10.4.x: 3485956-11.4.x
- 10.5.x: 3485956-10.5.x
- 11.x: 3485956-recursion-limit-exceeded
- 11.0.x: 3485956-11.0.x
- 11.1.x: 3485956-11.1.x
Remaining tasks
- Update dependency in core.
API changes
No
Data model changes
No
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #34 | 3485956-twig.diff | 1.39 KB | jan kellermann |
Issue fork drupal-3485956
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3485956-recursion-limit-exceeded
changes, plain diff MR !10085
- 3485956-10.3.x
changes, plain diff MR !10090
- 3485956-10.4.x
changes, plain diff MR !10093
- 3485956-10.5.x
changes, plain diff MR !10094
- 3485956-11.0.x
changes, plain diff MR !10095
- 3485956-11.1.x
changes, plain diff MR !10096
Comments
Comment #2
gillesbailleuxComment #3
gillesbailleuxProblem is related only to twig because the very same problem occurs on another website when updating v3.14.0 => v3.14.1
Comment #4
gillesbailleuxComment #5
gillesbailleuxRemoving the drupal/twig_tweak (3.4.0) module does not solve the problem.
Comment #6
gillesbailleuxComment #7
gillesbailleuxSame blank page appears when updating a block.
Comment #8
raphaelbertrand commentedsame problem, related error seem to be:
PHP Fatal error: Allowed memory size of *** bytes exhausted (tried to allocate 262144 bytes) in ***/vendor/twig/twig/src/Extension/SandboxExtension.php on line 130
Comment #9
raphaelbertrand commentedUsing drupal 10.3.6 and 8.3.13
Reverting change in src/Extension/SandboxExtension.php in this commit of twig/tiwg have same effect than downgrade to 3.14.0
Fix sandbox handling for __toString()
https://github.com/twigphp/Twig/commit/2bb8c2460a2c519c498df9b643d527711...
Comment #10
gillesbailleux@raphaelbertrand: any advice to increase PHP memory? If yes, where?
Comment #11
raphaelbertrand commentedi tried with differents settings (on differents servers), it doesn't seem to solve the problem.
Comment #12
mradcliffeI also ran into this updating my local ddev development environment from 10.3 to 11.0, but prior to updating to 11.0 I did run composer update -W per upgrade instructions.
I added the empty stracktrace from #8 that I also got into the issue summary.
This seems like a bug report.
Comment #13
mradcliffeWe may be doing something that's triggering this based on the BC BREAK note here.
Comment #14
raphaelbertrand commentedsame problem on taxonomy and many other edit form.
In text format config, it occur on text format with ckeditor5 enabled or when trying to enable it.
Maybe it is related to ckeditor5 ?
Comment #15
raphaelbertrand commented@mradcliffe
it seem to be related to this looking at the commit of twig/twig causing the error:
* Fix a security issue in the sandbox mode allowing an attacker to be able to call `toString()`
under some circumstances on an object even if the `__toString()` method is not allowed by the security policy
Comment #16
gillesbailleux@raphaelbertrand: when trying to edit a block which does not use ckeditor5, the same blank page appears.
Comment #17
mradcliffeFollowing Issue Priority > Critical > Bug, I think this qualifies as a Critical bug report as the site becomes unusable when applying the release for content editors. Either render system or theme system.
Comment #18
gillesbailleuxIncreasing the PHP APCu memory from 32 to 350 MB does not solve the problem.
Comment #19
mradcliffeEnabling xdebug provides a better stack trace.
Comment #20
gillesbailleux@mradcliffe: the infinite loop you spotted can only by fixed by the twig maintainer, right?
Comment #21
gillesbailleuxThe Symfony founder and project lead posted this:
https://github.com/twigphp/Twig/security/advisories/GHSA-6377-hfv9-hqf6
Comment #22
mradcliffeI don't know if this should be addressed upstream or not, @gillesbailleux. Reporting an issue upstream might help to get someone more knowledgeable about the right approach though.
Comment #23
raphaelbertrand commentedmaybe reporting this to twig maintainers can help them to know that the sanbox can cause an infinite loop in certain case an write a new exception case to prevent this and throw debuging info to dev?
Comment #24
gillesbailleux@mradcliffe: nothing can be done from the Drupal community so I guess the Symfony community will fix this.
Comment #25
mradcliffeI think I spoke too soon about it being infinite in our case, but it does hit the recursion limit. Sorry.
Checking the is_object($v) helps to reduce that.
Comment #26
mradcliffeI filed an issue upstream - https://github.com/twigphp/Twig/issues/4439
Comment #27
gillesbailleuxThank you very much @mradcliffe
Comment #28
larowlanIs this because our objects reference themselves and we don't have recursion protection?
Comment #29
mradcliffeAdding some tags, updated issue summary with sections from template.
I don't feel like only checking \is_object($v) would be acceptable upstream since the point of the method ensureToStringAllowed() method checks arrays as well, but as soon as we recurse via arrays too, we run into the recursion limit.
A potential Drupal-only solution would be to create a custom DrupalSandboxExtension that copies the code in SandboxExtension, and maybe checks $policy::$allowed_methods somehow to ensure that nobody has overridden core's TwigSecurityPolicy to remove __toString as an allowed method. If it is still allowed, then we can bypass the call to that method entirely and return $obj, and if somehow someone is doing something funky in custom land, then it would try its best with the original code.
We cannot extend SandboxExtension because it is final so this would require maintaining the code separately.
And then maybe we can work with twig/twig:4.x to allow for a way to override ensureToStringAllowed on the policy level rather on the extension level so we can get rid of it.
Comment #30
andypostComment #31
jan kellermann commentedWe had this problem, too. The update of twig/twig was triggered by roave/security-advisories:dev-latest.
We hat to downgrade and fix twig/twig to 3.14.0 in composer.json
Comment #32
amaria commented@jan we had to do the same
Comment #33
larowlanhttps://github.com/twigphp/Twig/pull/4441 may help
Comment #34
jan kellermann commentedThank you larowlan, But the white page persists.
We changes serialize() with json_encode() and removed the first unset() befor the return statement to prevent recursion in different trees of array.
And for Drupal we had to remove the patches for test.
Comment #35
cilefen commentedComment #36
elc commentedForced downgrading to 3.14.0 has fixed this in D10.3.6 production sites.
Is also causing testing failures in contrib without a useful error indication.
Comment #37
olkoeller commentedHow can I pin twig/twig to 3.14.0?
I am using Drupal 10 via Docker container with Dockerfile:
...
FROM drupal:10.2.3-php8.3-apache-bullseye
...
Comment #38
cilefen commentedThe usual way is to add a conflict.
Comment #39
olkoeller commentedso I added
"conflict": {
"drupal/drupal": "*",
"twig/twig": "3.14.1"
},
to my composer.json.
This seems to work, I do not get the error, when trying to add new content.
Thanks
Comment #40
elc commentedtwig 3.14.2 has been released:
https://github.com/twigphp/Twig/releases/tag/v3.14.2
Comment #41
kaszarobertIn the meantime v3.14.2 was released. Does this problem affect that version also?
Comment #42
jan kellermann commentedA new twig version is released:
https://github.com/twigphp/Twig/releases/tag/v3.14.2
Big thanks to larowlan !
Comment #43
mradcliffetwig/twig:3.14.2 and twig/twig:3.11.3 are released now by Fabien.
This needs to be in 11.x-dev since it needs to go into 11.0.x, 11.1.x, 10.3.x and 10.4.x.
Comment #44
elc commentedConfirm that 3.14.2 fixes the issue for us here.
Does there need to be a change in Drupal (core-recommends)? I've just run composer update for twig everywhere and we're good to go.
The core-recommends is
twig/twig": "~v3.14.0"so most everyone will now bypass 3.14.1 to .2 and never know this happened. Would it be safer to change that totwig/twig": "~v3.14.2"? Or specifically add a conflict for a known bad version?The last change to the twig include in 10.3.x was 2 months ago, so this just bad timing for us who happened to update in the last 18 hours to 20 minutes ago.
https://github.com/drupal/core-recommended/commit/01ef17e35b70b6db38cefc...
Comment #46
mradcliffeUpdating twig/twig caused some Functional failures so setting this to Needs work.
Comment #47
jonathan1055 commentedPipelines for Contrib at 10.3.6 and 11.0.5 at the start of the week both had
Locking twig/twig (v3.14.0)and ran fine.Earlier today both jobs had
Locking twig/twig (v3.14.1)which caused many test failures.Re-running now gives
Locking twig/twig (v3.14.2)and the tests pass, so in principle this has solved it for some contrib at least.Comment #48
mradcliffeI ran the pipeline again on the merge request, and it passed.
Comment #49
heddnThis is now resolved with the 3.14.2 release. See https://github.com/twigphp/Twig/pull/4441
Comment #50
remco hoeneveld commentedGreat stuf! can confirm the site i worked on was also fixed by adding `twig/twig": "~v3.14.2"`
Comment #51
xtazJust running
composer updateand it's ok for me, twig was updated to 3.14.2Comment #52
mradcliffeI'm going to re-open this as Normal since there is a workaround, but we should still update the dependency in core because 3.14.0 has security vulnerabilities.
I'm removing the performance and security tags since that is no longer relevant. And maybe it's probably a task now.
Comment #53
gillesbailleuxThe composer update -W command has installed the latest Twig version: v3.14.2.
I notice that nodes and blocks are now editable.
Many thanks to @mradcliffe for the tremendous support on this issue.
Comment #54
longwaveThanks, the MR looks good - because of the composer lock hash changes unfortunately I think we need MRs for all active branches (11.x, 11.1.x, 11.0.x, 10.5.x, 10.4.x, 10.3.x)
Comment #56
mradcliffe10.3.x, 10.4.x and 10.5.x composer.lock files match previous release of composer so I cherry-picked the 10.3.x commit into branches for 10.4.x and 10.5.x.
Edit: 11.0.x and 11.1.x were also last run with the same version of composer and it is only 11.x that is run on the 2.8 stable release of composer.
Comment #61
mradcliffeI cleaned up the issue summary a little bit and added the branches to the issue summary.
Comment #62
godotislateChanges look good on all 6 MRs (11.x, 11.1.x, 11.0.x, 10.5.x, 10.4.x, 10.3.x).
Comment #69
longwaveThank you for creating all the MRs and to everyone who helped out here with debugging and testing.
Committed and pushed the new minimum version to all six active branches.
Comment #76
themodularlabI know this probably requires a new issue to be raised, but after updating to twig 3.14.2, I'm seeing extremely slow load times that did not exist prior to 3.14.1 or 3.14.2. Just curious if anyone else is also encountering performance issues.
Comment #77
longwaveIt's certainly possible that the security fix has a performance hit; please do open a new issue to discuss as others with the same problem are not necessarily going to find it here.
Comment #78
acvellio commented@themodularlab
On my site, performance deteriorated dramatically after updating from twig 3.14.0 to twig 3.14.2.
With twig 3.14.0, the time required to open the Node edit screen was a few seconds. However, after updating to twig 3.14.2, it now takes more than 30 seconds.
Comment #79
cilefen commentedThe changes: https://github.com/twigphp/Twig/compare/v3.14.0..v3.14.2
Comment #80
cilefen commentedComment #81
senzaesclusiva commentedI encountered the same problem in a from scratch D 10.3.7 installation. The #52 suggestion “composer update -W” solved the problem.
For the moment I do not notice any performance slowdown with the Twig update to 3.14.2, but it should be considered that I have few test nodes and with few fields per node. Maybe as I continue to develop the site the problem may arise.
One possible issue to keep monitored
My 2 cents
Comment #82
senzaesclusiva commentedI wanted to add an aspect that puzzles me with respect to this issue.
I am not a programmer and am unfamiliar with the code, but I am quite skeptical that the problem that occurred is caused by Twig.
Why?
I am managing the development of 3 sites installed starting with D 10.3.6 and later upgraded to D 10.3.7, in none of them did I run 'composer update -W' , which maintain Twig version 3.14.0.
So I am wondering why, if D10.3.7 installs|updates|requires Twig to version 3.14.1 , composer does not warn of the need to update it.
I am a little confused, about this
Comment #83
longwave@senzaesclusiva This only affects people who download the zip/tar.gz version of Drupal and do not use Composer; those files contain the latest compatible versions of all dependencies, which happened to be Twig 3.14.1 for Drupal 10.3.7.
Comment #84
senzaesclusiva commentedAhhh...that's the mystery I had missed :-)
Thank you very much