Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong argument for twig_matches #3801

Closed
jensschaerer opened this issue Jan 12, 2023 · 15 comments
Closed

Wrong argument for twig_matches #3801

jensschaerer opened this issue Jan 12, 2023 · 15 comments

Comments

@jensschaerer
Copy link

After upgrading to TWIG v3.5.0 (coming from v3.4.3) I get a strange error and as my knowledge of the TWIG codebase is limited, I'm not able to get whats the fault.

Error:
twig_matches(): Argument #2 ($str) must be of type string, null given

Calling template part:

{% if not result is empty %}
    {% include 'my-template.html.twig' with {"result":result} %}
{% endif %}

result is an array passed to a sub template. This works in v3.4.3 but not in v3.5.0

@jdreesen
Copy link
Contributor

It comes from the usage of the matches operator, like {% if someVar matches '/some-regex/' %}.
If someVar is empty, Twig throws an error since v3.5.

@jensschaerer
Copy link
Author

the variable "result" shouldn't be empty in any way because of the if statement around the include block

@stof
Copy link
Member

stof commented Jan 13, 2023

I don't see any usage of matches in your code snippet. So I'm quite sure you are showing the wrong part of your template.

@jensschaerer
Copy link
Author

If I remove the code snippet shown above the template renders as expected. There is no other code involved

@stof
Copy link
Member

stof commented Jan 13, 2023

Then please share the content of the included template. The error is probably somewhere inside it.

@jensschaerer
Copy link
Author

The included template was made empty to test if the error is inside it, so the error is within the include

@stof
Copy link
Member

stof commented Jan 13, 2023

Then please share the stack trace of the exception to see where this twig_matches call is.

@jensschaerer
Copy link
Author

After investigating the error and rollback to TWIG 3.4.3 and now re-upgrade to TWIG 3.5.0 the error seems gone. Strange behavior anyway. I cleared cache, deleted all cache related files, so something went wrong while installing TWIG 3.5.0 over 3.4.3

Sorry for the back and forth and this false report. I think this issue can be closed. If I encounter any more errors after deployment I'll get back

@stof stof closed this as completed Jan 13, 2023
@jensschaerer
Copy link
Author

@stof I now have same error. Please see stack trace:

TypeError
--
TypeError: twig_matches(): Argument #2 ($str) must be of type string, null given, called in /var/www/html/var/cache/dev/twig/12/120b453c0c68443f04ae729510c089ca.php on line 72    at vendor/twig/twig/src/Extension/CoreExtension.php:1030   at twig_matches('/^\\d+$/', null)      (var/cache/dev/twig/12/120b453c0c68443f04ae729510c089ca.php:72)   at __TwigTemplate_8e8f6ec760020205dea88db15c00e3e6->doDisplay('r', 'S')      (vendor/twig/twig/src/Template.php:394)   at Twig\Template->displayWithErrorHandling('r', 'S')      (vendor/twig/twig/src/Template.php:367)   at Twig\Template->display(array('r', 'S'))      (var/cache/dev/twig/95/9581a4bef78726dd9085689fb9aa283a.php:71)   at __TwigTemplate_a7b058c22557f81ab07ba5b8ec5260e2->doDisplay('r', 'S')      (vendor/twig/twig/src/Template.php:394)   at Twig\Template->displayWithErrorHandling('r', 'S')      (vendor/twig/twig/src/Template.php:367)   at Twig\Template->display(array('r', 'S'))      (vendor/twig/twig/src/Template.php:379)   at Twig\Template->render('r', 'S')      (vendor/twig/twig/src/TemplateWrapper.php:40)   at Twig\TemplateWrapper->render(array('r', 'S'))      (vendor/twig/twig/src/Environment.php:280)   at Twig\Environment->render('bundles/MSSqlBundle/Queries/DownloadQuery.csv.twig', array('r', 'S'))      (vendor/symfony/framework-bundle/Controller/AbstractController.php:258)   at Symfony\Bundle\FrameworkBundle\Controller\AbstractController->renderView('bundles/MSSqlBundle/Queries/DownloadQuery.csv.twig', array('r', 'S'))      (vendor/pimcore/pimcore/lib/Controller/Controller.php:81)   at Pimcore\Controller\Controller->renderView('bundles/MSSqlBundle/Queries/DownloadQuery.csv.twig', array('r', 'S'))      (vendor/symfony/framework-bundle/Controller/AbstractController.php:266)   at Symfony\Bundle\FrameworkBundle\Controller\AbstractController->render('r', 'S')      (vendor/pimcore/pimcore/lib/Controller/Controller.php:43)   at Pimcore\Controller\Controller->render('bundles/MSSqlBundle/Queries/DownloadQuery.csv.twig', array('r', 'S'))      (src/Controller/QueriesController.php:109)   at App\Controller\QueriesController->downloadQueryAction(89, object(Request), object(SqlServerQueryService))      (vendor/symfony/http-kernel/HttpKernel.php:163)   at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)      (vendor/symfony/http-kernel/HttpKernel.php:75)   at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)      (vendor/symfony/http-kernel/Kernel.php:202)   at Symfony\Component\HttpKernel\Kernel->handle(object(Request))      (public/index.php:36)

<!--EndFragment-->

call in Controller:

                $fields = array_keys($results[0]);
                $data = [
                    'title' => $query->getName(),
                    'results' => $results,
                    'fields' => $fields,
                    'fieldsList' => $query->getFieldslist(),
                    'csvFieldDelimiter' => self::DEFAULT_CSV_FIELD_DELIMITER,
                    'csvRowDelimiter' => self::DEFAULT_CSV_ROW_DELIMITER,
                    'csvStringDelimiter' => self::DEFAULT_CSV_STRING_DELIMITER,
                    'hideTitle' => $request->get('hidetitle', false)
                ];
                $response = $this->render('bundles/MSSqlBundle/Queries/DownloadQuery.csv.twig', $data);

Template:

{% if results is empty %}
	No db results
{% else %}
{% include 'bundles/MSSqlBundle/Partials/Query/FieldsRow.csv.twig' with {'fields':fields} %}
{% for result in results %}
	{% if not result is empty %}
		{% include 'bundles/MSSqlBundle/Partials/Query/AllFields.csv.twig' with {'result':result} %}
	{% endif %}
{% endfor %}

AllFields.csv.twig is empty for testing, error occurs before

@stof
Copy link
Member

stof commented Jan 13, 2023

An error happening in an empty template does not make sense to me.
try removing the Twig cache entirely (rm -rf var/cache/dev/twig/) to be sure that it is not a case of a cache corruption (having a cache for an older version of the template).
alternatively, provide the content of var/cache/dev/twig/12/120b453c0c68443f04ae729510c089ca.php where the error happens.

@jensschaerer
Copy link
Author

Cache was cleared several times, vendor folder deleted and reinstalled, no change. Please see content of var/cache/dev/twig/12/120b453c0c68443f04ae729510c089ca.php

elseif (twig_matches("/^\\d+\$/", twig_get_attribute($this->env, $this->source,             // line 8
(isset($context["result"]) || array_key_exists("result", $context) ? $context["result"] : (function () { throw new RuntimeError('Variable "result" does not exist.', 8, $this->source); })()), $context["field"], [], "array", false, false, true, 8))) {

@jensschaerer
Copy link
Author

Seems to be cache related issue in combination with empty and not empty templates, where twig_matches takes place. Doesn't matter if I'm in dev or prod environment. For now it is working again after fixing the "matches" part in the sub template. Somehow it was not recognized by TWIG at all that the template was empty and the previous code was executed... Sorry for this confusion :/

@uwej711
Copy link
Contributor

uwej711 commented Jan 19, 2023

Imho this change in 3.5.0 might break BC, preg_match accepted null for the second parameter (PHP 7.4). Now you get the TypeError. Easy to fix with |default("") but I wasn't expecting that.

@drjayvee
Copy link
Contributor

drjayvee commented Feb 2, 2023

I also ran into this BC-breaking change.

Just to be super clear, here's the trivial reproduce case:

{{ null matches '' }}

As pointed out by uwej711, it's easy to fix, but unexpected. Given that Twig is usually very lenient, I think this should be changed.

This was introduced in #3687 to guard against invalid regex patterns.

@stof I'd be happy to create a PR with

- function twig_matches(string $regexp, string $str)
+ function twig_matches(string $regexp, ?string $str)

What do you think?

fabpot added a commit that referenced this issue Feb 8, 2023
…942)

This PR was merged into the 3.x branch.

Discussion
----------

Restores the leniency of the `matches` twig comparison

Restores the leniency of the `matches` twig comparison, allowing null subject to result in a non-match.

Resolves BC break introduced in PR #3687 ref in #3801 (comment)

As per pattern in #3617

Commits
-------

f136668 Restores the leniency of the `matches` twig comparison, allowing null subject to result in a non-match.
@fabpot
Copy link
Contributor

fabpot commented Feb 8, 2023

BC break fixed in #3809

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants