Problem/Motivation

Twig's 3.30 released just now fails Drupal tests with TypeError: Twig\Runtime\EscaperRuntime::escape(): Argument #4 ($autoescape) must be of type bool, null given

Twig 3.30 changed how its escape filter is compiled. The filter now gets its own EscapeFilter node, which compiles to a direct call to $this->escaper->escape(...). Drupal's TwigNodeVisitor::leaveNode() only swaps that node's callable to drupal_escape. Twig still emits the direct call, but builds the arguments from drupal_escape, which needs the environment. So $this->env gets passed as an extra first argument, every argument shifts one place, and null (the charset) ends up in $autoescape.

Steps to reproduce

core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php and core/tests/Drupal/KernelTests/Core/Theme/TwigMarkupInterfaceTest.php are quick ones to check as they fail.

Proposed resolution

In core/lib/Drupal/Core/Template/TwigNodeVisitor.php, instead of changing the callable on Twig's node, leaveNode() now returns a new plain FilterExpression that uses drupal_escape with the original argument node. That way Twig's escape node is never compiled at all.

The composer.lock would need to be updated temporarily too to verify it passes on Twig 3.30. (Not in the MR).

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

LLM disclosure

LLM assistance was used to identify and propose a resolution to this issue.

Issue fork drupal-3625969

Command icon 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:

Comments

gábor hojtsy created an issue. See original summary.

gábor hojtsy’s picture

@jurgenhaas is saying its due to the "Speed up escaping by fetching the escaper runtime once per template" change in 3.30.

hfernandes’s picture

Just faced this issue while upgrading the core to 10.6.17.
In the meantime I had to lock the twig version with composer require twig/twig:~3.29.0 util this issue gets sorted out.

gábor hojtsy’s picture

My LLM says "Twig 3.30's new EscapeFilter node compiles to $this->escaper->escape(...), but Drupal's TwigNodeVisitor only swaps in drupal_escape without adjusting for the argument shift." and is experimenting with a solution :) Will report when it has it :D

gábor hojtsy’s picture

Issue summary: View changes
gábor hojtsy’s picture

Issue summary: View changes
phenaproxima’s picture

Gábor's Opus concurs with my Opus regarding both the cause and the fix.

socketwench’s picture

Can confirm that this resolves the WSOD on core 11.4.7 and Twig 3.30.

phenaproxima’s picture

Status: Active » Reviewed & tested by the community
phenaproxima’s picture

Status: Reviewed & tested by the community » Active

Hmmm...maybe not. There's a test failing in the "for testing only" branch, due to a deprecation in Twig itself.

gábor hojtsy’s picture

The fails on the updated real 3.30 MR are:

  • 2.29 deprecation firing in the phpunit test
  • Umami performance numbers worsening (which is odd)

We would fix the deprecation notice I think but the performance number should be only fixed if we actually update to 3.30.

The test only changed did not run any tests in the composer lock update MR because no tests were changed, so making a dummy change there just to verify that :D

gábor hojtsy’s picture

Status: Active » Needs review
jurgenhaas’s picture

It's been "discussed" over in the Twig issue queue as well: https://github.com/twigphp/Twig/issues/4952

godotislate’s picture

Judging from the merged PR to Twig, tt seems like we could also just do this change:

diff --git a/core/lib/Drupal/Core/Template/TwigNodeVisitor.php b/core/lib/Drupal/Core/Template/TwigNodeVisitor.php
index 2752e52f4bb..5c8c4f0f9c8 100644
--- a/core/lib/Drupal/Core/Template/TwigNodeVisitor.php
+++ b/core/lib/Drupal/Core/Template/TwigNodeVisitor.php
@@ -63,6 +63,7 @@ public function leaveNode(Node $node, Environment $env): ?Node {
       if ('escape' == $name || 'e' == $name) {
         // Use our own escape filter that is MarkupInterface aware.
         $node->setAttribute('twig_callable', $env->getFilter('drupal_escape'));
+        $node->setAttribute('template_escaper', FALSE);

         // Store that we have a filter active already that knows
         // how to deal with render arrays.

I haven't tested this, though. Also we'd have to bump the Twig version to 3.30.0 to test in CI.

catch’s picture

I'm unable to reproduce the umami performance test failure locally so far - it passes fine.

gábor hojtsy’s picture

@godotislate: yeah we have two MRs so we can test the fix in parallel with 3.30 and current 3.28 on main.

longwave made their first commit to this issue’s fork.

gábor hojtsy’s picture

The suggestion from the Twig side issue is also to

replace the whole node with your own FilterExpression node built for the node class of the drupal_escape filter

which is what we are doing here.

longwave’s picture

Alternative approach in MR!17244 that should be more performant as well - instead of swapping out the filter at parse time, we just swap out the definitions of escape and e filters with our version.

This approach was assisted by Claude Opus 5.5.

gábor hojtsy’s picture

Test only job of https://git.drupalcode.org/project/drupal/-/jobs/12447670 now reproduces the fail with dummy test changes and the composer.lock update only in that MR. 😅

@catch: Umami fail magically disappeared with the one twig file fixed that was wrong for 3.30.

f0ns’s picture

Tested both MRs on a real site running Drupal 11.4.7.

Before: with Twig 3.30.0 every page WSODs with EscaperRuntime::escape(): Argument #4 ($autoescape) must be of type bool, null given.

MR !17242: the TwigNodeVisitor.php change fixes the WSOD on Twig 3.30.0. The views-view-table.html.twig hunk doesn't apply to 11.4.x, which still has gin_is_sticky.

MR !17244: the first TwigExtension.php hunk needs a context tweak for 11.4.x, where getFilters() has no : array return type. With that, it fixes the WSOD on Twig 3.30.0 and also works on Twig 3.28.0.

For each setup I compared escaping output against stock Twig 3.28 for {{ a }}, {{ a|e }}, a Markup object with and without |escape, |escape('js') and render arrays with and without |e. The output was identical in every case.

ykhalid’s picture

Here is a reproduction with plain Twig and no Drupal, in case it helps whoever reviews the two MRs. It registers a filter the way drupal_escape is registered (needs_environment) and swaps the callable of every escape and e node the way TwigNodeVisitor does, then prints the compiled escape calls. In an empty directory run composer require twig/twig:3.29.0 (or 3.30.0), then php repro.php.

Twig 3.29.0 renders and compiles the swapped filter call:

yield (string) $this->extensions['DrupalLikeExtension']->escapeFilter($this->env, ($context["v"] ?? null), "html", null, true);

Twig 3.30.0 throws the same TypeError as the site does and compiles a direct runtime call with the swapped filter's argument list, so the Environment lands in the first slot and everything shifts:

yield (string) $this->escaper->escape($this->env, ($context["v"] ?? null), "html", null, true);

It runs in a second, so it is a quick way to check an approach against both versions without a site. I also opened the Twig side report, which is what jurgenhaas linked above.

<?php
// Plain Twig, no Drupal. A filter registered like drupal_escape (needs_environment)
// and a node visitor that swaps the callable of escape/e nodes, as TwigNodeVisitor does.
require __DIR__ . '/vendor/autoload.php';
use Twig\Environment;
use Twig\Loader\ArrayLoader;
use Twig\TwigFilter;
use Twig\Node\Node;
use Twig\Node\Expression\FilterExpression;
use Twig\NodeVisitor\NodeVisitorInterface;
use Twig\Extension\AbstractExtension;
use Twig\Runtime\EscaperRuntime;

function drupal_like_escape_is_safe(Node $filterArgs) { return ['html']; }

class DrupalLikeExtension extends AbstractExtension {
  public function getFilters(): array {
    return [new TwigFilter('drupal_escape', [$this, 'escapeFilter'], [
      'needs_environment' => TRUE, 'is_safe_callback' => 'drupal_like_escape_is_safe'])];
  }
  public function escapeFilter(Environment $env, $arg, $strategy = 'html', $charset = NULL, $autoescape = FALSE) {
    if ($arg === NULL || $arg === '') return NULL;
    return $env->getRuntime(EscaperRuntime::class)->escape($arg, $strategy, $charset, $autoescape);
  }
  public function getNodeVisitors(): array { return [new DrupalLikeVisitor()]; }
}
class DrupalLikeVisitor implements NodeVisitorInterface {
  public function enterNode(Node $node, Environment $env): Node { return $node; }
  public function leaveNode(Node $node, Environment $env): ?Node {
    if ($node instanceof FilterExpression) {
      $name = $node->getAttribute('twig_callable')->getName();
      if ('escape' == $name || 'e' == $name) {
        $node->setAttribute('twig_callable', $env->getFilter('drupal_escape'));
      }
    }
    return $node;
  }
  public function getPriority() { return 256; }
}

$cache = __DIR__ . '/cache'; @mkdir($cache);
$twig = new Environment(new ArrayLoader(['t' => 'auto: {{ v }} | explicit: {{ v|e }}']), ['cache' => $cache, 'autoescape' => 'html', 'auto_reload' => TRUE]);
$twig->addExtension(new DrupalLikeExtension());
echo "twig " . Environment::VERSION . "\n";
try {
  echo "render: " . $twig->render('t', ['v' => '<b>x</b>']) . "\n";
} catch (\Throwable $e) {
  echo get_class($e) . ": " . $e->getMessage() . "\n";
}
foreach (glob("$cache/*/*.php") as $f) {
  echo "--- compiled escape calls in " . basename($f) . "\n";
  foreach (file($f) as $n => $line) if (str_contains($line, 'scape')) echo ($n+1) . ": " . trim($line) . "\n";
}
kevinquillen’s picture

Confirmed, had to pin to a previous version for build to work.

junaidpv’s picture

I am facing this issue

ayoub.elmansouri’s picture

I tested MR !17242 on core 11.4.7 and Twig 3.30 and it works as expected. The fix resolves the issue perfectly without any regressions.

godotislate’s picture

Status: Needs review » Reviewed & tested by the community

I bumped Twig locally to 3.30.

For MR 17244:
TwigExtensionTest fails as expected without the changes to TwigExtension and TwigNodeVisitor, and the changes make the test pass.

I think 17244 is the best way forward for the performance benefits cited in #22. A Twig method is being copied to TwigExtension, and so there's a risk we could miss upstream changes, but on the other hand, the Twig method is marked @internal, so I'm not sure which way is better, but I think it's fine as is.

But it otherwise is good for RTBC.

Looks like we'll also need a separate 11.4.x MR, and maybe also for 11.3.x and 10.6.x as well?

godotislate’s picture

In the meantime while this issue is open and the fix has a release, sites can have their Twig version reverted with composer update twig/twig:3.28.0. This will set 3.28.0 as the version in composer.lock without adjusting constraints in composer.json.

  • catch committed 836a5ba6 on 12.0.x
    fix: #3625969 Twig's 3.30 TypeError: Twig\Runtime\EscaperRuntime::escape...

  • catch committed b4659e2c on main
    fix: #3625969 Twig's 3.30 TypeError: Twig\Runtime\EscaperRuntime::escape...
catch’s picture

Version: main » 11.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Agreed with taking this out of runtime.

I've committed/pushed https://git.drupalcode.org/project/drupal/-/merge_requests/17244. to main and 12.0.x

Moving to 11.x for backport.

lnunesbr’s picture

Have tested MR !17250 with drupal 10.6.x and works fine.

godotislate’s picture

Status: Patch (to be ported) » Needs review
mw4ll4c3’s picture

Status: Needs review » Reviewed & tested by the community

Confirming MR 17251 works on 11.4.7

majorrobot’s picture

I tested the MR for 11.x in 11.4.7 locally, and it worked like a charm.

majorrobot’s picture

Tested the MR for 10.6.x, and works well there, too.

godotislate’s picture

Note that I added the 10.6.x MR because in #3612448: Widen constraints in core-recommended for 11.3.x and 10.6.x, we relaxed the Twig constraint, but I'm not sure the changes here will be backported to 11.3.x or 10.6.x since they are security-only branches.

elc’s picture

If any site running 11.3.x and 10.6.x no longer works when they are updated, I would consider that to be a sufficiently serious issue to warrant back-porting to a supported branch that is meant to continue working for another 3 months.

This is a downside of the loosening of the constraints - we are no longer guaranteed a tested set of dependencies in core-recommended and a breaking release can occur between scheduled release cycles, just like a security release of a dependency used to get stuck on constraints.

If the constraints are going to be left relaxed and Twig 3.30.x is now a possible dependency, then Drupal must be compatible with it.

It makes for the case that there are circumstances when an off schedule release is warranted. This seem to be one of them as the main Drupal product is effectively broken until the next published release that fixes this. Only a small percentage of site admins are going to find their way to this issue for the fix. The rest need it to just work.

jaydip makawana’s picture

I tested the MR for 11.x in 11.4.7 locally, its working as expected.

camerongreen’s picture

Pop something like this in your composer.json and update:

"twig/twig": "^3.29.0, !=3.30.0"

  • catch committed 12ea8942 on 11.4.x
    fix: #3625969 Twig's 3.30 TypeError: Twig\Runtime\EscaperRuntime::escape...

  • catch committed 7a5138ae on 11.x
    fix: #3625969 Twig's 3.30 TypeError: Twig\Runtime\EscaperRuntime::escape...

  • catch committed d7caf396 on 10.6.x
    fix: #3625969 Twig's 3.30 TypeError: Twig\Runtime\EscaperRuntime::escape...
catch’s picture

Version: 11.x-dev » 10.6.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed the backports to 11.x/11.4.x and 10.6.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

damienmckenna’s picture

Thanks everyone!

damienmckenna’s picture

Might it be possible to get releases tagged with this fix? This bug is causing all functional tests to fail for all contrib modules using D10, D11 or D12. Thank you again.

longwave’s picture

Yes, the release managers are already discussing this, but the timing is not great with Drupalcon next week.

just_like_good_vibes’s picture

hello,
about pipelines failures on contrib modules, you can use a temporary solution if you like, with variables in the .gitlab-ci.yml file.

For example

variables:
  CORE_STABLE: '11.4.x-dev'
  CORE_PREVIOUS_MINOR: '11.3.x-dev'   # only used with OPT_IN_TEST_PREVIOUS_MINOR 
  CORE_PREVIOUS_STABLE: '10.6.x-dev'  # only used with OPT_IN_TEST_PREVIOUS_MAJOR
jonathan1055’s picture

If the actual quick releases of 11.4.8, 11.3.18 and 10.6.18 are going to be a problem, then in Gitlab Templates we could make the above changes to the CORE_ variables and release it as default and this would fix all Contrib projects right away. The values could be reverted when the real release is made.

Just for info, I tested phpunit on Core 10.6.17 (previous major) and this fails as expected, because is installs Twig v3.30.0. However Core 11.3.17 (previous minor) does not fail, because it install Twig v3.27.1.

jonathan1055’s picture

I've just tested in a CI pipeline with the three updated -dev core versions. All three load Twig 3.30 and Core 11.4.8-dev and 10.6.18-dev phpunit tests now work correctly.

But 11.3.18-dev fails, with the original
TypeError: Twig\Runtime\EscaperRuntime::escape(): Argument #4 ($autoescape) must be of type bool, null given.
So it appears that 11.3.17 wasn't broken before, but it is broken now.

Or maybe I'm missing something and have got this completely wrong. It would be good for someone else to confirm my findings.

longwave’s picture

Version: 10.6.x-dev » 11.3.x-dev
Status: Fixed » Patch (to be ported)

The fix did not get cherry picked to 11.3 so that makes sense. Let's do that as we will need to release 11.3 as well.

majorrobot’s picture

Status: Patch (to be ported) » Needs review

MR for 11.3.x with the fix cherry-picked is ready with passing pipelines.

Just needs a review!

  • catch committed d3f21930 on 11.3.x
    fix: #3625969 Twig's 3.30 TypeError: Twig\Runtime\EscaperRuntime::escape...
catch’s picture

Version: 11.3.x-dev » 10.6.x-dev
Status: Needs review » Fixed

Moving back to fixed.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch’s picture

I've tagged 11.4.8 and 10.6.18 which include the fix here.

I think 11.3 still has the patch level constraints for core-recommended so can probably wait for #3625998: Update CKEditor5 version for Drupal core 10.6, 11.3, 11.4, 11.5.

jonathan1055’s picture

Thanks. I confirm that phpunit jobs in Contrib pipelines using the new releases 11.4.8 and 10.6.18 run OK and pass. The unchanged 11.3.17 is fine, as this still uses Twig 3.27. Here's my test pipeline with override variables.

Gitlab Templates will be updated tomorrow - here is the issue and MR to update those core version variables

dadderley’s picture

A couple of days ago (Friday night), I updated a site from drupal 10.6.16 to 10.6.17.
Immediately got a WSOD with this error.

TypeError: Twig\Runtime\EscaperRuntime::escape(): Argument #4 ($autoescape) must be of type bool, null given, called in /var/www/drupal/web/sites/default/files/php/twig/6ab7292fecdbb_field

I pasted the error message info Google AI and it returned:

This error is caused by a recent update to Twig (version 3.30.0), which introduced stricter type safety on EscaperRuntime::escape() (requiring bool for the fourth argument instead of allowing null). Because your Drupal site's compiled Twig cache (/var/www/drupal/web/sites/default/files/php/twig/...) still contains older compiled templates generated before the update, they are passing null and triggering this fatal TypeError.

It gave this solution:

Bash
composer require "twig/twig:3.29.0" --no-update
composer update twig/twig
drush cr

I did this downgrade and the site immediately came back to life.

xmacinfo’s picture

Do we need Core to pin down twig versions?

elc’s picture

All constraints were loosened on purpose after the restricted constraints prevented a dependency security update from being allowed to apply. The core-recommended project is now identical to using the core project.

@see

In an ideal world we would have the tested dependency constraints of core-recommended, with the ability to update them without needing to do a full core release. Perhaps automatically updated after a CI run (in this case they would have failed and not updated); triggered by the release of a dependency. I'm happy I caught this one on the dev sites, but I wasted a lot of time working on d.o contrib modules.

xmacinfo’s picture

Thanks for the reminder about loosening constraints. This makes it easy to update security fixes of those packages (the issues documented above) or rollback the packages when a package update cause a fatal error (this issue).

I am not sure how any CI run would have caught the problem.

longwave’s picture

Yeah, we can't win here. If we pin dependencies then it's on us to update as soon as there is a security release; if we don't pin dependencies then we risk running into issues when there is any kind of BC break. On balance we think not pinning is slightly preferable as people want to be able to update as soon as there is a security issue.

The only way to catch this early would be to run tests against untagged prerelease versions of all dependencies, but currently we don't have a pipeline that does this, and even then we might not catch things in time.

mikelutz’s picture

I think 11.3 still has the patch level constraints for core-recommended so can probably wait for #3625998: Update CKEditor5 version for Drupal core 10.6, 11.3, 11.4, 11.5

@catch I've got a bunch of sites using drupal/core:~11.3.0 would be nice to get an 11.3 hotfix. Is this fix backwards compatible with twig 3.29, or If I pin my projects to 3.29 will they break again with 11.3.18 comes out with this fix and I'm still pinned to twig 3.29?

jonathan1055’s picture

Gitlab Templates has been updated to use 11.4.8 and 10.6.18. Release 1.17.1 is tagged and default-ref updated to point to this, so all Contrib using the default reference for their pipelines will now not get the Twig error and the phpunit jobs should run as before.

gábor hojtsy’s picture

Just noting here that @longwave made Drupal 11.3.18 available for those that do not use core-recommended about an hour ago at https://www.drupal.org/project/drupal/releases/11.3.18