I have just updated twig/twig (v1.37.1 => v1.38.0) via composer and get a fatal error with the following in the Apache error log:

Got error 'PHP message: PHP Fatal error: Declaration of Drupal\\Core\\Template\\TwigTransTokenParser::parse(Twig_Token $token) must be compatible with Twig\\TokenParser\\TokenParserInterface::parse(Twig\\Token $token) in /var/www/html/web/core/lib/Drupal/Core/Template/TwigTransTokenParser.php on line 0\nPHP message: PHP Stack trace:\nPHP message: PHP 1. {main}() /var/www/html/web/index.php:0\nPHP message: PHP 2. Drupal\\Core\\DrupalKernel->handle() /var/www/html/web/index.php:19\nPHP message: PHP 3. Stack\\StackedHttpKernel->handle() /var/www/html/web/core/lib/Drupal/Core/DrupalKernel.php:693\nPHP message: PHP 4. Drupal\\Core\\StackMiddleware\\NegotiationMiddleware->handle() /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php:23\nPHP message: PHP 5. Drupal\\Core\\StackMiddleware\\ReverseProxyMiddleware->handle() /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php:52\nPHP message: PHP 6. Asm89\\Stack\\Cors->handle() /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php:47\nPHP message: PHP 7. Drupal\\Core\\StackMiddleware\\KernelPreHandle->handle() /var/www/html/vendor/asm89/stack-cors/src/Asm89/Stack/Cors.php:49\nPHP message: PHP 8. Drupal\\Core\\StackMiddleware\\Session->handle() /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php:47\nPHP message: PHP 9. Symfony\\Component\\HttpKernel\\HttpKernel->handle() /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/Session.php:57\nPHP message: PHP 10. Symfony\\Component\\HttpKernel\\HttpKernel->handleRaw() /var/www/html/vendor/symfony/http-kernel/HttpKernel.php:68\nPHP message: PHP 11. Drupal\\Component\\EventDispatcher\\ContainerAwareEventDispatcher->dispatch() /var/www/html/vendor/symfony/http-kernel/HttpKernel.php:156\nPHP message: PHP 12. call_user_func:{/var/www/html/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111}() /var/www/html/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111\nPHP message: PHP 13. Drupal\\Core\\EventSubscriber\\MainContentViewSubscriber->onViewRenderArray() /var/www/html/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111\nPHP message: PHP 14. Drupal\\Core\\Render\\MainContent\\HtmlRenderer->renderResponse() /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/MainContentViewSubscriber.php:90\nPHP message: PHP 15. Drupal\\Core\\Render\\MainContent\\HtmlRenderer->prepare() /var/www/html/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php:117\nPHP message: PHP 16. Drupal\\Core\\Render\\Renderer->executeInRenderContext() /var/www/html/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php:227\nPHP message: PHP 17. Drupal\\Core\\Render\\MainContent\\HtmlRenderer->Drupal\\Core\\Render\\MainContent\\{closure}() /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:582\nPHP message: PHP 18. Drupal\\Core\\Render\\Renderer->render() /var/www/html/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php:226\nPHP message: PHP 19. Drupal\\Core\\Render\\Renderer->doRender() /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:195\nPHP message: PHP 20. call_user_func:{/var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:378}() /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:378\nPHP message: PHP 21. Drupal\\node\\Controller\\NodeViewController->buildTitle() /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:378\nPHP message: PHP 22. Drupal\\Core\\Render\\Renderer->render() /var/www/html/web/core/lib/Drupal/Core/Entity/Controller/EntityViewController.php:73\nPHP message: PHP 23. Drupal\\Core\\Render\\Renderer->doRender() /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:195\nPHP message: PHP 24. Drupal\\Core\\Theme\\ThemeManager->render() /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:437\nPHP message: PHP 25. twig_render_template() /var/www/html/web/core/lib/Drupal/Core/Theme/ThemeManager.php:384\nPHP message: PHP 26. Drupal\\Core\\Template\\TwigEnvironment->loadTemplate() /var/www/html/web/core/themes/engines/twig/twig.engine:64\nPHP message: PHP 27. Drupal\\Core\\Template\\TwigEnvironment->loadClass() /var/www/html/vendor/twig/twig/src/Environment.php:446\nPHP message: PHP 28. Drupal\\Core\\Template\\TwigEnvironment->compileSource() /var/www/html/vendor/twig/twig/src/Environment.php:482\nPHP message: PHP 29. Drupal\\Core\\Template\\TwigEnvironment->tokenize() /var/www/html/vendor/twig/twig/src/Environment.php:792\nPHP message: PHP 30. Twig\\Lexer->__construct() /var/www/html/vendor/twig/twig/src/Environment.php:692\nPHP message: PHP 31. Twig\\Lexer->getOperatorRegex() /var/www/html/vendor/twig/twig/src/Lexer.php:72\nPHP message: PHP 32. Drupal\\Core\\Template\\TwigEnvironment->getUnaryOperators() /var/www/html/vendor/twig/twig/src/Lexer.php:389\nPHP message: PHP 33. Drupal\\Core\\Template\\TwigEnvironment->initExtensions() /var/www/html/vendor/twig/twig/src/Environment.php:1454\nPHP message: PHP 34. Drupal\\Core\\Template\\TwigEnvironment->initExtension() /var/www/html/vendor/twig/twig/src/Environment.php:1533\nPHP message: PHP 35. Drupal\\Core\\Template\\TwigExtension->getTokenParsers() /var/www/html/vendor/twig/twig/src/Environment.php:1579\nPHP message: PHP 36. spl_autoload_call() /var/www/html/web/core/lib/Drupal/Core/Template/TwigExtension.php:207\nPHP message: PHP 37. Composer\\Autoload\\ClassLoader->loadClass() /var/www/html/web/core/lib/Drupal/Core/Template/TwigExtension.php:207\nPHP message: PHP 38. Composer\\Autoload\\includeFile() /var/www/html/vendor/composer/ClassLoader.php:322\n'

CommentFileSizeAuthor
#103 3039408-8.5.x-103.patch9.98 KBalexpott
#101 3039408-83.patch6.89 KBalexpott
#101 3039408-81.patch10.32 KBalexpott
#99 3039408-99.patch7.11 KBpdenooijer
#98 3039408-92-do-not-test.patch7.84 KBkfritsche
#84 3039408-83.patch6.89 KBalexpott
#81 3039408-interdiff-68-81.txt1.75 KBWidgetsBurritos
#81 3039408-81.patch10.32 KBWidgetsBurritos
#77 3039408-interdiff-73-77.txt1.68 KBWidgetsBurritos
#77 3039408-77-withlock.patch10.13 KBWidgetsBurritos
#77 3039408-77-nolock.patch7.7 KBWidgetsBurritos
#73 3039408-73.patch7.37 KBWidgetsBurritos
#68 3039408-interdiff-67-68.txt1.69 KBWidgetsBurritos
#68 3039408-68.patch9.8 KBWidgetsBurritos
#67 3039408-diff-62-67.txt1.36 KBvijaycs85
#67 3039408-67.patch8.75 KBvijaycs85
#65 3039408-65.patch388 bytespandaski
#62 3039408-diff-61-62.txt2.52 KBvijaycs85
#62 3039408-62.patch7.4 KBvijaycs85
#61 3039408-diff-57-61.txt682 bytesvijaycs85
#61 3039408-61.patch4.87 KBvijaycs85
#57 3039408-diff-50-57.txt652 bytesvijaycs85
#57 3039408-57.patch4.21 KBvijaycs85
#50 3039408-diff-42-50.txt718 bytesvijaycs85
#50 3039408-50.patch3.82 KBvijaycs85
#42 3039408-42-hasAttribute.patch3.05 KBphenaproxima
#42 3039408-42-CheckToStringNode.patch3.12 KBphenaproxima
#38 3039408-38.patch1001 bytesvijaycs85
#37 3039408-36.patch1.9 KBmikelutz
#21 core-twig_fatal_with_1.38.0-3039408-20.patch386 byteshuzooka
#4 3039408-4.patch4.38 KBtstoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webfaqtory created an issue. See original summary.

rmpereira’s picture

Same problem, i exec the composer command :

composer update drupal/core

The twig/twig component has been updated from version 1.37.1 to 1.38.0.

I come back temporarily to 1.37.1 version and it's work for me ...

lisagodare’s picture

I'm also having the same issue. As I have a very limited amount of time to perform maintenance updates, I have temporarily set the twig version in composer to 1.37.1 as a workaround.

tstoeckler’s picture

Status: Active » Needs review
FileSize
4.38 KB

Here's an initial patch that updates to 1.38 in composer.lock and fixes the typehints. I'm not sure what this means in terms of backwards compatibility, i.e. I think this patch makes us incompatible with 1.35, but not sure.

Gábor Hojtsy’s picture

Woah, how did this happen in Twig 1.38 in the first place?

Ben25’s picture

rolling back to 1.37.1 is the quickest fix.

rgpublic’s picture

It seems they renamed the class name "Twig_Token" to "Twig\Token". Which makes sense to me in some way, because the namespaced variant instead of the prefixed version is more standards-conformant. Or, I guess, they removed the aliasing. The Twig_Token class name seems to be deprecated for some time. Why they changed this in a 1.37 => 1.38 minor version update I dont know. Of course, all code that relied on this is failing now. That's my take on this. If someone as more info on this, I'd be glad to know.

Eli-T’s picture

lluisandreu’s picture

Hello, how can I rollback Twig in a Composer project Drupal?

Thanks.

matzAB’s picture

@lluisandreu

composer require twig/twig:1.37.1

hkirsman’s picture

@lluisandreu I did:
composer require twig/twig:1.37.1
and removed if from composer.json

iuana’s picture

Indeed, I rolled back the version of twig/twig by running composer require "twig/twig:1.37.1"

EliasPapa’s picture

Yup running composer require twig/twig:1.37.1 seems to fix the issue by downgrading twig to that version. Lets hope they fix it soon. Thanks for creating the thread, was going crazy there for a sec.

huzooka’s picture

Status: Needs review » Needs work

Since the issue is with Twig (see https://github.com/twigphp/Twig/issues/2886 ), the solution is conflicting with twig/twig 1.38.0 IMHO

And since the issue exists in the 8.7.x branch as well (i was working with that), the first step is check that the issue exists in 8.8.x

Gábor Hojtsy’s picture

@rgpublic well these kind of changes would be perfectly valid in Twig 3 (unreleased). For Twig 1 and 2 (released), backwards compatibility should be kept. Work is happening in this pull request to fix it for Twig 1: https://github.com/twigphp/Twig/pull/2887/commits/65c0559125c5d11b96162b...

hiramanpatil’s picture

roll back the version "twig/twig:1.37.1" worked...thanks!

ajay547’s picture

Confirmed #16, its working.

donaldp’s picture

I can confirm that rolling back to twig 1.37.1 fixes the issue:
The problem seems to this:

In Drupal: public function parse(\Twig_Token $token) {..}
vs
in twig v1.38.0: public function parse(Token $token);
in twig v1.37.1: it's defined very differently as :

class_exists('Twig_TokenParserInterface');

if (\false) {
    interface TokenParserInterface extends \Twig_TokenParserInterface
    {
    }
} 
alunyov’s picture

That worked for me as temporary fix:
composer require twig/twig:1.37.1

Lukas von Blarer’s picture

Downgrading Twig to 1.37.1 solves the issue. The patch in #4 doesn't apply to 8.6.10. So if anyone needs a urgent fix go with changing the composer version constraints for twig/twig.

huzooka’s picture

Version: 8.6.10 » 8.8.x-dev
Status: Needs work » Needs review
FileSize
386 bytes

Untestable patch against 8.8.x.

tstoeckler’s picture

Version: 8.8.x-dev » 8.6.10

Sorry, my patch was on 8.7.x, not 8.6.x.

I think #21, as well as pinning 1.37, is problematic, because 1.38 is actually a security release per https://github.com/FriendsOfPHP/security-advisories/commit/e5783d3116527...

As a temporary measure I did this for my site as well, but I think we should update as soon as possible in core.

tstoeckler’s picture

Version: 8.6.10 » 8.6.x-dev

Oops, version change was unintended. Although I think 8.6.x is the correct version to target here.

WidgetsBurritos’s picture

Looks like they've just merged and bumped the release that fixes this issue:
https://github.com/twigphp/Twig/pull/2887

Seems to work for me locally.

Ben25’s picture

pstewart’s picture

I picked up the 1.38.1 twig upgrade but then ran into a different error viewing the previously working homepage of my d8 site:

Uncaught PHP Exception Twig\Error\SyntaxError: "An exception has been thrown during the compilation of a template ("Attribute "name" does not exist for Node "Twig\Node\CheckToStringNode".") in "themes/contrib/zurb_foundation/templates/node.html.twig"." at vendor/twig/twig/src/Environment.php line 797

I'm guessing the template in question has a fault but something may have gone wrong with exception handling following the namespace change that seems to have been introduced with 1.38.0 despite the fix with 1.38.1. I've reverted to 1.37.1 as per earlier comments and everything is now working again for me.

timmillwood’s picture

I am seeing the same issue as #26.

Uncaught PHP Exception Twig\Error\SyntaxError: "An exception has been thrown during the compilation of a template ("Attribute "name" does not exist for Node "Twig\Node\CheckToStringNode".") in "core/themes/bartik/templates/block--system-menu-block.html.twig"." at /mnt/www/html/mysite/vendor/twig/twig/src/Environment.php line 797
kfritsche’s picture

I can confirm this, with 1.38.1 and the seven core theme I get the same error on the node/add page.

URL: /node/add
Theme: seven
Error:

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Twig\Error\SyntaxError</em>: An exception has been thrown during the compilation of a template (&quot;Attribute &quot;name&quot; does not exist for Node &quot;Twig\Node\CheckToStringNode&quot;.&quot;) in &quot;core/themes/seven/templates/node-add-list.html.twig&quot;. in <em class="placeholder">Twig\Environment-&gt;compileSource()</em> (line <em class="placeholder">797</em> of <em class="placeholder">.../vendor/twig/twig/src/Environment.php</em>). <pre class="backtrace">Drupal\Core\Template\TwigNodeTrans-&gt;compileString(Object) (Line: 37)
Gábor Hojtsy’s picture

Title: Updating twig/twig to v1.38.0 causes fatal error » Updating twig/twig to v1.38.0 or v1.38.1 causes fatal error
kfritsche’s picture

That class (CheckToStringNode ) which throws an error in Twig was introduced with 1.38.0 and was part of the security fix mentioned by #22 (https://github.com/twigphp/Twig/commits/1.x/src/Node/CheckToStringNode.php).

material_admin theme is affected too.

phenaproxima’s picture

Confirming this is still broken for me as well, with the same error @kfristche reports in #28. In my case, the errors are coming from Views, but I think it's likely the same problem (see https://travis-ci.org/acquia/headless-lightning/jobs/505306749#L2353 for a partial backtrace).

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Twig\Error\SyntaxError</em>: An exception has been thrown during the compilation of a template (&quot;Attribute &quot;name&quot; does not exist for Node &quot;Twig\Node\CheckToStringNode&quot;.&quot;) in &quot;core/themes/seven/templates/node-add-list.html.twig&quot;. in <em class="placeholder">Twig\Environment-&gt;compileSource()</em> (line <em class="placeholder">797</em> of <em class="placeholder">.../vendor/twig/twig/src/Environment.php</em>). <pre class="backtrace">Drupal\Core\Template\TwigNodeTrans-&gt;compileString(Object) (Line: 37)
vijaycs85’s picture

fabpot’s picture

I would need to debug this issue, I have added some questions on the related Twig issue.

fabpot’s picture

I would need help to debug this issue, I have added some questions on the related Twig issue.

fabpot’s picture

Can you tell me how to reproduce? I've checkout-ed Drupal, updated Twig, everything seem to work. I've then updated everything (composer up), and it does not break for me.

Corn696’s picture

composer create-project drupal-composer/drupal-project:8.x-dev some-dir --stability dev --no-interaction

create database

install drupal following the setup wizard. (I used english as language)

visit page -> get unexpected error

On a existing site with stable as base theme in use the site is accessible. But seven admin theme throws errors on node/add and in the frontend the mini pager of a view as well.

mikelutz’s picture

Patch against 8.6 for testing.

vijaycs85’s picture

Not sure what this would break in terms of functionality, but it doesn't throw the exception anymore.

WidgetsBurritos’s picture

@vijaycs85 obviously we need to see what happens with the tests, but there is another reference to $args->getAttribute('name') further up in that file. Does that one need a try/catch as well?

fabpot’s picture

Here is a patch for you to test:

Add

if ($n instanceof \Twig\Node\CheckToStringNode) {
$n = $n->getNode('expr');
}

Before

$args = $n;

in TwigNodeTrans.php at line 116

vijaycs85’s picture

Here is the patch with composer.lock file changes as well (thanks to @mikelutz).

phenaproxima’s picture

Here is another possible fix I came up with (calling hasAttribute() before getAttribute()), and another patch that implements @fabpot's solution. Let's see how these do. Lucky 42!

dnebrich’s picture

I just tested fabpot's patch from comment #40. It worked for me. I did not apply any other patches referenced in this issue.

I have drupal 8.6.10, I upgraded twig to 1.38.0, saw the issue, then upgraded twig to 1.38.1, got the new exception mentioned in #31, then applied fabpot's patch and all is well now.

ressa’s picture

Thanks @fabpot, your suggestion in #40 fixed the problem for me.

phenaproxima’s picture

Sounds like 3039408-42-CheckToStringNode.patch in #42 (which implements @fabpot's solution) is tentatively RTBC...

The last submitted patch, 37: 3039408-36.patch, failed testing. View results

ressa’s picture

Thanks @phenaproxima, I just tried your patches from #42, and they also work well.

vijaycs85’s picture

Found few failures for patch in #42(3039408-42-CheckToStringNode.patch):

18:55:17 Drupal\Tests\system\Functional\Theme\TwigRegistryLoaderTest    0 passes   1 fails                                                              
18:55:29 Drupal\Tests\system\Functional\Theme\TwigSettingsTest          0 passes   1 fails                            
18:55:34 Drupal\Tests\system\Functional\Theme\TwigTransTest             0 passes             1 exceptions             
18:55:34 FATAL Drupal\Tests\system\Functional\Theme\TwigTransTest: test runner returned a non-zero error code (2).
18:55:34 Drupal\Tests\system\Functional\Theme\TwigTransTest             0 passes   1 fails                            
18:55:34 Drupal\Tests\system\Functional\Theme\TwigExtensionTest         0 passes   1 fails                            
18:56:00 Drupal\Tests\update\Functional\UpdateUploadTest                0 passes   1 fails      

Checking locally.

Status: Needs review » Needs work

The last submitted patch, 42: 3039408-42-hasAttribute.patch, failed testing. View results

vijaycs85’s picture

Fixed Drupal\Tests\system\Functional\Theme\TwigRegistryLoaderTest. All others all locally green, not sure if the errors are because of deprication notices.

Status: Needs review » Needs work

The last submitted patch, 50: 3039408-50.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

websiteworkspace’s picture

I'm seeing this major and critical error/problem too!

composer automatically updated twig to version 1.38.2 and it is breaking all updated Drupal 8 websites!
(thank goodness for nightly automated backups ... )

There is a note in the change log for twig!


* 1.38.2 (2019-03-12)

 * added TemplateWrapper::getTemplateName()

* 1.38.1 (2019-03-12)

 * fixed class aliases

* 1.38.0 (2019-03-12)

 * fixed sandbox security issue (under some circumstances, calling the
   __toString() method on an object was possible even if not allowed by the
   security policy)
 * fixed batch filter clobbers array keys when fill parameter is used
 * added preserveKeys support for the batch filter
 * fixed "embed" support when used from "template_from_string"
 * added the possibility to pass a TemplateWrapper to Twig\Environment::load()
 * improved the performance of the sandbox
 * added a spaceless filter
 * added max value to the "random" function
 * made namespace classes the default classes (PSR-0 ones are aliases now)
 * removed duplicated directory separator in FilesystemLoader
 * added Twig\Loader\ChainLoader::getLoaders()
 * changed internal code to use the namespaced classes as much as possible

The fatal error and thrown exception occurs in the following class and method

{site}/vendor/twig/twig/src/Node/CheckToStringNode.php


namespace Twig\Node;

use Twig\Compiler;
use Twig\Node\Expression\AbstractExpression;

/**
 * Checks if casting an expression to __toString() is allowed by the sandbox.
 *
 * For instance, when there is a simple Print statement, like {{ article }},
 * and if the sandbox is enabled, we need to check that the __toString()
 * method is allowed if 'article' is an object. The same goes for {{ article|upper }}
 * or {{ random(article) }}
 *
 * @author Fabien Potencier <fabien@symfony.com>
 */
class CheckToStringNode extends Node
{
    public function __construct(AbstractExpression $expr)
    {
        parent::__construct(['expr' => $expr], [], $expr->getTemplateLine(), $expr->getNodeTag());
    }

    public function compile(Compiler $compiler)
    {
        $compiler
            ->raw('$this->sandbox->ensureToStringAllowed(')
            ->subcompile($this->getNode('expr'))
            ->raw(')')
        ;
    }
}



Consider the following ...
Thousands of Drupal 8 users ran a routine code update check script and update code script today, and discovered this critical error.
Then each of those thousands of Drupal 8 users each lost and hour, or hours, of their day, tracking down this problem.
... tens of thousands of person work hours squandered, down the drain ...

phenaproxima’s picture

Thousands of Drupal 8 users ran a routine code update check script and update code script today, and discovered this critical error.
Then each of those thousands of Drupal 8 users each lost and hour, or hours, of their day, tracking down this problem.
... tens of thousands of person work hours squandered, down the drain ...

We can't always anticipate every possible change that our upstream dependencies might make. The risk of things breaking is part and parcel of software development, but detecting these kinds of problems is exactly why we write tests, and when things do break, we try to fix them as quickly as possible. (That's why this issue is marked critical.) We're all aware of this problem and we're doing our best to fix it, and not all of us are being paid for that work. Please try to be understanding.

Gábor Hojtsy’s picture

@websiteworkspace: if you are automatically updating to third party dependencies when their new versions come out without testing them, then worse problems could arise. Drupal core includes a composer.lock file that specifies tested dependencies. We are running test suites on them every minute of every day. When we update our composer.lock-ed dependencies, we test them first. Since you are not relying on core's composer.lock, you can also use https://github.com/webflo/drupal-core-strict to achieve the same.

websiteworkspace’s picture

Please try to be understanding.

The purpose of my comment was/is to express "understanding", as well as empathy, for the likely thousands of people who've found themselves fighting this emergency fire/problem today.

Please try to be understanding.

websiteworkspace’s picture

Since you are not relying on core's composer.lock ...

How does one configure a Drupal 8 site's composer.json to "rely on core's composer.lock" file?

Where on drupal.org is there an article about how to implement such a configuration for a Drupal 8 site and its composer.json file?

Might it make sense for the following approach:


composer require webflo/drupal-core-strict

to be a standard aspect/component of Drupal 8 deployments, and its associated scripts, requirements, and dependencies?

Please try to be understanding.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
4.21 KB
652 bytes

after a couple of hours of xdebug, found another missing scenario. automated_tests++

pandaski’s picture

#57 works well, doing more testing now

Status: Needs review » Needs work

The last submitted patch, 57: 3039408-57.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

vijaycs85’s picture

We got 2 fails left.

22:51:15 Drupal\Tests\system\Functional\Theme\TwigSettingsTest          0 passes   1 fails                            
22:51:25 Drupal\Tests\system\Functional\Theme\TwigExtensionTest         0 passes   1 fails      

I am running locally and getting fails on both, but no error at all. e.g:

php core/scripts/run-tests.sh --verbose --url http://d8 --class "Drupal\Tests\system\Functional\Theme\TwigSettingsTest"

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\system\Functional\Theme\TwigSettingsTest

Test run started:
  Tuesday, March 12, 2019 - 22:59

Test summary
------------

Drupal\Tests\system\Functional\Theme\TwigSettingsTest          0 passes   1 fails

Test run duration: 27 sec

Detailed test results
---------------------


---- Drupal\Tests\system\Functional\Theme\TwigSettingsTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Fail      Other      phpunit-13.xml       0 Drupal\Tests\system\Functional\Them
    PHPunit Test failed to complete; Error: PHPUnit 6.5.14 by Sebastian
    Bergmann and contributors.

    Testing Drupal\Tests\system\Functional\Theme\TwigSettingsTest
    ....                                                                4 / 4
    (100%)

    Time: 27.42 seconds, Memory: 6.00MB

    OK (4 tests, 18 assertions)

    HTML output was generated
    http://d8/sites/simpletest/browser_output/Drupal_Tests_system_Functional_Theme_TwigSettingsTest-7-55788078.html
    http://d8/sites/simpletest/browser_output/Drupal_Tests_system_Functional_Theme_TwigSettingsTest-8-32313748.html
    http://d8/sites/simpletest/browser_output/Drupal_Tests_system_Functional_Theme_TwigSettingsTest-9-32313748.html

    Remaining deprecation notices (1)

      1x: The Twig\Environment::getCacheFilename method is deprecated since
    version 1.22 and will be removed in Twig 2.0.
        1x in TwigSettingsTest::testTwigCacheOverride from
    Drupal\Tests\system\Functional\Theme
vijaycs85’s picture

Status: Needs work » Needs review
FileSize
4.87 KB
682 bytes

Another test fix.

vijaycs85’s picture

FileSize
7.4 KB
2.52 KB

Fixing all 4 depreciation notices.

The last submitted patch, 61: 3039408-61.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jibran’s picture

pandaski’s picture

The quickest/temp fix is to add the trouble version to the conflict section of `core/composer.json`

The last submitted patch, 62: 3039408-62.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

vijaycs85’s picture

Fixing one last fail in #62

#65: worth noting, It might not work, if you use roave/SecurityAdvisories

WidgetsBurritos’s picture

FileSize
9.8 KB
1.69 KB

Noticed #67 had a few minor code style issues.

Adding this patch to fix them, to help move this along.

pandaski’s picture

Quick question:

Do we have a reason to update other packages in `composer.lock` file? Are they required by Twig?

mikelutz’s picture

Status: Needs review » Needs work

Unfortunately, #68 will fail when applied without the composer lock changes, as CheckToStringNode did not exist before twig 1.38.0. The patch needs to work with the current dependencies and future versions of twig. We need passing tests both with and without the composer lock changes.

joelpittet’s picture

@Joseph Zhao, yes because that is when the CheckToStringNode was introduced.

@mikelutz The tests pass already without the lock changes if you don't update Twig, do we really need to get them passing for when "you don't update Twig" because to get these changes you'd need to update Drupal already and you only run into this problem when you update Twig.

We should bump the minimum version of Twig though for this in composer.json and I assume @vijaycs85 was just doing the lock changes to ensure the testbot was testing the right version of Twig.

WidgetsBurritos’s picture

I don't think the comment in #70 is true. Since CheckToStringNode is only ever being used with instanceof, I don't think the library doesn't actual needs it. See http://php.net/manual/en/internals2.opcodes.instanceof.php#108164

Please note, that you get no warnings on non-existent classes:

<?php
class A() {
}

$a = new A();

$exists = ($a instanceof A); //TRUE
$exists = ($a instanceof NonExistentClass); //FALSE

As it would return false in that instance, it will behave like it did already.

WidgetsBurritos’s picture

Status: Needs work » Needs review
FileSize
7.37 KB

For testing purposes, here's the patch without the composer.lock changes.

mikelutz’s picture

Twig 1.38 was a security update, so maybe the committers and security team will want to force the dependency update with the next release, in which case you do need to bump the minimum version in the composer json and get buy-in from RM/FM/security team folks.

WidgetsBurritos: tricky. USEing a non-existent file will throw an exception in the symfony4 test thread, but it might not here. Curious now if it passes.

WidgetsBurritos’s picture

@mikelutz yeah, I dunno for sure. I guess we'll find out soon. :-)

WidgetsBurritos’s picture

Status: Needs review » Needs work

It's not quite done yet, but taking an earlier peak at the test runner, it looks like the changes made to TwigNamespaceTest are going to be problematic as its returning a Twig_Template instance instead of a TemplateWrapper instance.

WidgetsBurritos’s picture

This patch should resolve the issues with both TwigNamespaceTest and TwigRegistryLoaderTest. For testing purposes, I've included the patch both with and without the composer.lock file updates.

alexpott’s picture

Thanks everyone for jumping on this.

@mikelutz is right we need to update core/composer.json to have "twig/twig": "^1.38.1",. We need to change the lock file on this issue too.

pandaski’s picture

Looks like Twig had three releases v1.38.0, v1.38.1 and v1.38.2 this morning (https://github.com/twigphp/Twig/releases)

And v1.38.2 is the latest one (https://github.com/twigphp/Twig/releases/tag/v1.38.2)

Changelog: https://github.com/twigphp/Twig/blob/2.x/CHANGELOG

WidgetsBurritos’s picture

Per @Joseph Zhao's feedback, my last patch actually updated the lock file to 1.38.2 instead of 1.38.1 anyway. Any objections here?

Also should the update to composer.json be based on #68 instead of #77 then?

WidgetsBurritos’s picture

I went ahead and based the patch on #68 as it's a forced dependency here.

alexpott’s picture

We're going to need an 8.7.x/8.8.x patch too and they have some changes for Twig 2 compatibility already so the fix might become smaller.

pandaski’s picture

#81,

             ],
+            "abandoned": true,
             "time": "2015-10-02T06:51:40+00:00"

Shall we leave this line to "phpunit/phpunit-mock-objects" issue?

alexpott’s picture

Here's a patch for 8.7.x / 8.8.x

@Joseph Zhao nope we should not hand craft our composer.lock file.

alexpott’s picture

+++ b/core/modules/system/tests/modules/twig_extension_test/src/TwigExtension/TestExtension.php
@@ -2,6 +2,9 @@
+use Twig\TwigFilter;
+use Twig\TwigFunction;
+

@@ -21,7 +24,7 @@ class TestExtension extends \Twig_Extension {
-      new \Twig_SimpleFunction('testfunc', [$this, 'testFunction']),
+      'testfunc' => new TwigFunction('testfunc', ['Drupal\twig_extension_test\TwigExtension\TestExtension', 'testFunction']),
     ];
   }
 
@@ -39,7 +42,7 @@ public function getFunctions() {

@@ -39,7 +42,7 @@ public function getFunctions() {
    */
   public function getFilters() {
     return [
-      new \Twig_SimpleFilter('testfilter', [$this, 'testFilter']),
+      'testfilter' => new TwigFilter('testfilter', ['Drupal\twig_extension_test\TwigExtension\TestExtension', 'testFilter']),

This changes are not strictly necessary for 8.7.x/8.8.x but keeping them inline with 8.6.x is a good thing and using the namespaced function is a good idea.

Nick Hope’s picture

Re. #81, Composer is telling me it cannot apply https://www.drupal.org/files/issues/2019-03-12/3039408-81.patch to D8.6.10.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Patch looks good now.

jibran’s picture

pandaski’s picture

We have another file `TwigExtension.php` containing many usages of `new \Twig_SimpleFunction` and `new \Twig_SimpleFilter`.

Maybe we can put a post-task so we can keep the same style for the functions and styles.

Also tested the patch for 8.8.x in the local instance, looks good to me RTBC +1

dtv_rb’s picture

Patch #81 cannot be applied to my Drupal 8.6.10 core. Updated from Twig 1.37.1 to 1.38.2 a few hours ago.

jurgenhaas’s picture

Also 8.6.10 and patch from #81: composer outputs an error message about not being able to apply the patch (same as #86, but when checking the files, the patch got applied correctly and it does solve the problem.

dtv_rb’s picture

@jurgenhaas

It's because the changes to the composer.lock and core/composer.json files could not be applied. Mine looked different than these in the patch.

What I did: I saved the patch file #81 to my system and removed the changes to composer.lock and core/composer.json.

Now the patch applies and there are no Twig Errors any more.

Hope that fixes it.

@WidgetsBurritos

It would be great if you could provide a separate patch without the changes to the composer files!

Nick Hope’s picture

+1 #91.

However the case of "email": "BackEndTea@gmail.com" is failing to change to "email": "backendtea@gmail.com", so perhaps that might be triggering the "could not apply" error in Composer.

jurgenhaas’s picture

@dtv_rb thanks for the explanation. In my case, I just ignored the composer error because it did what was necessary and the process doesn't fail. It's just the irritating message but everything else is fine.

xjm’s picture

xjm’s picture

This will need a separate patch for the 8.6.x backport, which should resolve a lot of the questions above about it not applying to 8.6. (Composer config changes almost always need separate patches per minor branch.)

pdenooijer’s picture

FileSize
7.32 KB

Removed the local composer.lock changes from #81 as this will give the errors about not being able to apply the patch. This assumes that the composer.lock is in the docroot and it should be updated with composer it self not with a patch.

kfritsche’s picture

FileSize
7.84 KB

Patch from #81 seems to work for 8.6.10.

I attach a new patch (as do not test) which should work for people using composer and/or drupal-composer/drupal-scaffold. As there you only have the core directory and do not have the composer.lock. Thats why the patch fails. Its the same as #81 just without the composer.lock changes. For core to commit #81 is still correct.

As I did the work now for myself, just wanted to share for people to use, till 8.6.11(?) is released.

pdenooijer’s picture

Updated #97 with the right patch paths.

frondeau’s picture

Twig 1.38.2 doasn't resolve issue in Drupal 8. My version is back to 1.37.1.

alexpott’s picture

Thanks everyone for supplying patches for your local sites but doing so creates problems because then the rtbc retest is not going to be testing the correct thing. Also it make the issue super confusing. Another solution is to create another issue and link to it from here - mark in a duplicate but still use it.

Uploading the correct patches again so they are last on the issue.

alexpott’s picture

@informaid after applying the patch you have to run composer update twig/twig - if you are not using composer to manager your code then you need to wait for a release.

alexpott’s picture

Here's a patch for 8.5.x as well.

dtv_rb’s picture

@alexpott Patch #81 doesn't apply correctly for people with different composer.json or composer.lock files than in the patch. I was upgrading from Twig 1.37.1 to 1.38.2. The patch relies on upgrading from 1.35.4 to 1.38.2 in core/composer.json. So it can't really be marked as RTBC.

alexpott’s picture

@dtv_rb the patch changes our root composer.lock if you have made customisations to your root composer.json which will result in a new composer.lock then it is on you to manage applying the patch. We have to update the composer.lock in this or Drupal's tarball will be not be updated. In an ideal world we would not ship a composer.lock file but that's for a completely separate issue and has a whole initiative involved in trying to resolve it. You can apply patches with git and use the --exclude=composer.lock option and then run composer update twig/twig and everything will work for you.

dtv_rb’s picture

@alexpott Thanks for the clarification! :)

  • catch committed 82fed91 on 8.8.x
    Issue #3039408 by vijaycs85, WidgetsBurritos, alexpott, phenaproxima,...

  • catch committed 2966903 on 8.7.x
    Issue #3039408 by vijaycs85, WidgetsBurritos, alexpott, phenaproxima,...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed and cherry-picked to 8.8.x, 8.7.x, 8.6.x and 8.5.x, thanks!

  • catch committed 2685447 on 8.6.x
    Issue #3039408 by vijaycs85, WidgetsBurritos, alexpott, phenaproxima,...

  • catch committed 5cbcd4b on 8.5.x
    Issue #3039408 by vijaycs85, WidgetsBurritos, alexpott, phenaproxima,...
nerdstein’s picture

Is there any way this can get added into previous released versions? This is causing sites on simplytest.me to not provision properly.

xjm’s picture

@nerdstein: It's impossible to add anything to a release that's already been tagged.

There will be new patch releases soon with these fixes and they will also be included in 8.7.0-alpha1.

nor sairi’s picture

can i ask question? where location i need to put this patch and apply patch.

severin-bruhat’s picture

Will you create 8.6.11 for this? If so, can you communicate the delay?
Thanks

Eli-T’s picture

@nor sairi see https://www.drupal.org/patch/apply for help on how to apply patches

mikelutz’s picture

@nor sari also see alexpott's comments in #105 about applying this specific patch.

@severin-bruhat - Xjm is a release manager and just said there will be new patch releases soon, this means 8.6.11. There is no delay, these releases require tests and signoffs and coordination and take a little time, but it is coming.

severin-bruhat’s picture

@mikelutz, thank you

Ben25’s picture

Can anyone access `/admin/modules/uninstall` when using twig 1.38.2 ?

WidgetsBurritos’s picture

@Ben25,

With the patched code from above, yes I can access /admin/modules/uninstall.

Rajab Natshah’s picture

Sorry to report back
Still, we do have issues after 1.38.2 as we need a patch
but it's not only node, media and other entity types

The following is a temp to keep going until the release
composer require twig/twig:1.37.1

huzooka’s picture

Apply the matching patch instead of downgrading, see https://symfony.com/blog/twig-sandbox-information-disclosure

carma03’s picture

I was getting this error on 8.5.11 version:

Twig\Error\SyntaxError: An exception has been thrown during the compilation of a template ("Attribute "name" does not exist for Node "Twig\Node\CheckToStringNode".") in "core/themes/classy

..and #103 patch worked for me.

Thanks!

xjm’s picture

Hi everyone,

The above update to Drupal's composer.json Twig requirement was included in today's Drupal 8.6.11 and 8.5.12 releases, which should help sites avoid this error. I recommend you update Drupal core rather than trying to apply a patch to an existing Composer-built site. Thanks!

dtv_rb’s picture

@xjm Thank you! The update to 8.6.11 fixed the whole issue!

Rajab Natshah’s picture

Thank you for your hard work!! :)
Updating to Drupal 8.6.11

hctom’s picture

Be aware, that there might be an issue with Twig 1.38.2, when using {% embed %} tags in your template:

https://github.com/twigphp/Twig/issues/2898

Follow that issue to get updates/clarifications for it.

See also: #3040210: Updating twig/twig to v1.38.2 (included in Drupal 8.6.11) causes fatal error when embed tags are used in templates

scott_euser’s picture

If anyone is having issues related to this with devel (eg, devel_breakpoint() / kint()), issue open here:
https://www.drupal.org/project/devel/issues/3040310#comment-13024326

nor sairi’s picture

where is twig directory to apply patch?

yuseferi’s picture

Running

composer require twig/twig:1.37.1

resolved it for me

scott_euser’s picture

For the benefit of anyone who stumbles on this issue, as per https://www.drupal.org/project/drupal/issues/3039408#comment-13018250 please instead update your Drupal Core to the latest which fixes this issue; do not force Twig 1.37.1 as that has a security issue.

micheljp’s picture

Getting this error when upgrading to security release 8.6.13

[Wed Mar 20 17:31:31.347132 2019] [php7:notice] [pid 1597] [client 192.168.33.1:56457] Uncaught PHP Exception Twig\\Error\\SyntaxError: "An exception has been thrown during the compilation of a template ("Attribute "name" does not exist for Node "Twig\\Node\\CheckToStringNode".") in "core/themes/classy/templates/views/views-mini-pager.html.twig"." at /var/www/public/drupal8/vendor/twig/twig/src/Environment.php line 797,

a-fro’s picture

This bug also brought our site down when deploying the 8.6.13 security release.

We fixed it by removing the twig translate tag {% trans %}String translation works if not rendering a {{ variable }}{% endtrans %} as a hotfix.

sitepioneers’s picture

I was able to fix the issue by updating core to the latest release.

guardiola86’s picture

I'm getting this error as well when updating core to 8.6.13

cilefen’s picture

@guardiola86 Please provide some detail about the upgrade process followed, and the context in which the error occurs, or we have nothing to go on.

guardiola86’s picture

I've upgraded to core 8.6.13 and twig version 1.38.4. That's pretty much all I've done and I'm getting this error:

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Twig\Error\SyntaxError</em>: An exception has been thrown during the compilation of a template (&quot;Attribute &quot;name&quot; does not exist for Node &quot;Twig\Node\CheckToStringNode&quot;.&quot;) in &quot;core/themes/classy/templates/content/node.html.twig&quot;. in <em class="placeholder">Twig\Environment-&gt;compileSource()</em> (line <em class="placeholder">807</em> of <em class="placeholder">/vagrant/repos/charged/vendor/twig/twig/src/Environment.php</em>). <pre class="backtrace">Project_trans_Node-&gt;compileString(Object) (Line: 32)
Project_trans_Node-&gt;compile(Object) (Line: 121)
Twig\Node\Node-&gt;compile(Object) (Line: 103)
Twig\Compiler-&gt;subcompile(Object) (Line: 53)
Twig\Node\IfNode-&gt;compile(Object) (Line: 121)
Twig\Node\Node-&gt;compile(Object) (Line: 121)
Twig\Node\Node-&gt;compile(Object) (Line: 103)
Twig\Compiler-&gt;subcompile(Object) (Line: 329)
Twig\Node\ModuleNode-&gt;compileDisplay(Object) (Line: 106)
Twig\Node\ModuleNode-&gt;compileTemplate(Object) (Line: 78)
Twig\Node\ModuleNode-&gt;compile(Object) (Line: 92)
Twig\Compiler-&gt;compile(Object) (Line: 781)
Twig\Environment-&gt;compile(Object) (Line: 802)
Twig\Environment-&gt;compileSource(Object) (Line: 482)
Twig\Environment-&gt;loadClass(&#039;__TwigTemplate_bee36a7b6b2952c4b71c29fa6d71daff4e8a79a2bff87441ac3d7b455ed1c570&#039;, &#039;core/themes/classy/templates/content/node.html.twig&#039;, NULL) (Line: 446)
Twig\Environment-&gt;loadTemplate(&#039;core/themes/classy/templates/content/node.html.twig&#039;) (Line: 64)
twig_render_template(&#039;core/themes/classy/templates/content/node.html.twig&#039;, Array) (Line: 384)
Drupal\Core\Theme\ThemeManager-&gt;render(&#039;node&#039;, Array) (Line: 437)
Drupal\Core\Render\Renderer-&gt;doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer-&gt;render(Array, ) (Line: 226)
Drupal\Core\Render\MainContent\HtmlRenderer-&gt;Drupal\Core\Render\MainContent\{closure}() (Line: 582)
Drupal\Core\Render\Renderer-&gt;executeInRenderContext(Object, Object) (Line: 227)
Drupal\Core\Render\MainContent\HtmlRenderer-&gt;prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer-&gt;renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber-&gt;onViewRenderArray(Object, &#039;kernel.view&#039;, Object)
call_user_func(Array, Object, &#039;kernel.view&#039;, Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher-&gt;dispatch(&#039;kernel.view&#039;, Object) (Line: 156)
Symfony\Component\HttpKernel\HttpKernel-&gt;handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel-&gt;handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session-&gt;handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle-&gt;handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache-&gt;pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache-&gt;handle(Object, 1, 1) (Line: 119)
Drupal\cdn\StackMiddleware\DuplicateContentPreventionMiddleware-&gt;handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware-&gt;handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware-&gt;handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel-&gt;handle(Object, 1, 1) (Line: 693)
Drupal\Core\DrupalKernel-&gt;handle(Object) (Line: 19)
</pre>
cilefen’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Andy_D’s picture

Updated core to latest release and got this...

Drupal version : 8.6.15

Drupal-FAEL git:(000-latest-8.6-lightning) ✗ composer show twig/twig
name     : twig/twig
descrip. : Twig, the flexible, fast, and secure template language for PHP
keywords : templating
versions : * v1.40.1
type     : library
license  : BSD 3-Clause "New" or "Revised" License (BSD-3-Clause) (OSI approved) https://spdx.org/licenses/BSD-3-Clause.html#licenseText
source   : [git] https://github.com/twigphp/Twig.git 35889516bbd6bbe46a600c2c33b03515df4a076e
dist     : [zip] https://api.github.com/repos/twigphp/Twig/zipball/35889516bbd6bbe46a600c2c33b03515df4a076e 35889516bbd6bbe46a600c2c33b03515df4a076e
path     : /Users/andrewdempster/Projects/Drupal-FAEL/vendor/twig/twig
names    : twig/twig

autoload
psr-0
Twig_ => lib/
psr-4
Twig\ => src/

requires
php >=5.4.0
symfony/polyfill-ctype ^1.8

requires (dev)
psr/container ^1.0
symfony/debug ^2.7
symfony/phpunit-bridge ^3.4.19|^4.1.8

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Twig\Error\SyntaxError</em>: An exception has been thrown during the compilation of a template (&quot;Attribute &quot;name&quot; does not exist for Node &quot;Twig\Node\CheckToStringNode&quot;.&quot;). in <em class="placeholder">Twig\Environment-&gt;compileSource()</em> (line <em class="placeholder">1</em> of <em class="placeholder">themes/custom/focusrite/components/_patterns/03-organisms/_node.twig</em>). <pre class="backtrace">Project_trans_Node-&gt;compileString(Object) (Line: 34)

guardiola86’s picture