Contrib.social

twig escape filter on stringable objects results in fatal error

Open on Drupal.org →
Created on 25 November 2024, 27 days ago

Problem/Motivation

See the original issue here: https://www.drupal.org/project/drupal/issues/3457168 🐛 Since twig/twig 3.9: error with "twig_escape_filter" function usage in /core/lib/Drupal/Core/Template/TwigExtension.php Needs work

`twig_escape_filter()` usage has been converted into `$env->getRuntime(EscaperRuntime::class)->escape()` in `core/lib/Drupal/Core/Template/TwigExtension.php` because of a code deprecation.

But this fix introduces an issue, where it can send a wrong type in the escape method. Check this commit: https://git.drupalcode.org/project/drupal/-/commit/eaa7072469e5c3cbe2b87...

We can see that `$return` argument sent to `twig_escape_filter()` has been wrongly replaced with `$arg`, and we are missing string conversion actions performed several lines above.

Steps to reproduce

Get any stringable object variable in a twig template, and apply to it any escape filter (except for 'html'), and the website ends up with a BSOD and this error (depending on what object you try to escape):

The website encountered an unexpected error. Try again later.

Error: Object of class Drupal\Core\Url could not be converted to string in Twig\Template->display() (line 350 of /app/vendor/twig/twig/src/Template.php).
Twig\Template->render(Array) (Line: 35)
Twig\TemplateWrapper->render(Array) (Line: 33)

Twig Example:

<a href="{{ any_drupal_url_object|e('html_attr') }}">test</a>

Proposed resolution

Use
return $env->getRuntime(EscaperRuntime::class)->escape($return, $strategy, $charset, $autoescape);
instead of
return $env->getRuntime(EscaperRuntime::class)->escape($arg, $strategy, $charset, $autoescape);
in `app/core/lib/Drupal/Core/Template/TwigExtension.php`.

🐛 Bug report
Status

Active

Version

10.3 ✨

Component

theme system

Created by

🇫🇷France julienjoye

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @julienjoye
  • Comment 27 days ago →
    🇫🇷France julienjoye
  • Merge request !10329issue #3489668: fix wrong variable sent to EscaperRuntime::escape() method → (Open) created by Unnamed author
  • Pipeline finished with Failed
    27 days ago
    Total: 617s
    #349578
  • Pipeline finished with Canceled
    27 days ago
    Total: 298s
    #349595
  • Pipeline finished with Success
    27 days ago
    Total: 1181s
    #349598
  • Comment 26 days ago →
    🇫🇷France julienjoye
  • Comment 23 days ago →
    🇺🇸United States smustgrave

    Thank you for reporting, as a bug next step will be to add test coverage to show the issue.

  • Comment 19 days ago →
    🇫🇷France julienjoye

    Hey!

    I don't have much time for this by now, therefore I use the MR diff in the meantime.
    Still, I checked in the tests if a quick win could be done (Unfortunately, it can take some time).

    Nevertheless, some inputs:

    By now, all this `escapeFilter()` method is covered by only one test method (`TwigExtensionTest::testEscaping()`) and seems to implement "Happy Path testing anti-pattern", because all the scenarios provided by the `providerTestEscaping` data provider, are based on `path()` twig function. And this twig function always returns a string.

    Then, the escape method always pass by `$return = (string) $arg;` path, and then the call to `$env->getRuntime(EscaperRuntime::class)->escape();` with `$arg` or `$return` argument, is pretty identical.. That's why we missed this issue.

    Unfortunately, I don't know if we can call a twig function that could return an object. I did not find any in that TwigExtension core class.

    Is there a TwigExtension that could be loaded only during tests?
    Maybe we can write an anonymous class and load it dynamically in that test method?

    Cheers.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024