Make it easier to support undefined filters and functions?

[javiereguiluz]

Problem

When using Twig in your own apps, you have full control over your dependencies, so all filters/functions are defined. However, when using Twig for third-party packages (e.g. Symfony bundles) this is not true.

E.g. in EasyAdmin bundle we use a Twig function called csrf_token() ... but that's only available if your app also has symfony/security-csrf package installed. This package is optional in EasyAdmin, so we can't use the function.

We ended up creating a ea_call_function_if_exists(string $functionName, mixed ...$functionArguments) Twig function:

{# instead of this: #}
{{ csrf_token('foo') }}

{# we must use this: #}
{{ ea_call_function_if_exists('csrf_token', 'foo') }}

This works well for some functions but not for all, because of their implementation.

Solution

We could create a try tag like this:

{% try %} {{ csrf_token('foo') }} {% endtry %}

{% try %} {{ csrf_token('foo') }} {% else %} ... {% endtry %}
{# or #}
{% try %} {{ csrf_token('foo') }} {% catch %} ... {% endtry %}

Alternatively, we could make the well known ?? operator much smarter and allow using it in expressions like this:

{# if csrf_token() doesn't exist or throws an exception, '...'  is printed #}
{{ csrf_token('foo') ?? '...' }}

{# it would work for filters too #}
{{ 'foo'|csrf_token ?? '...' }}

Thanks!

[stof]

Due to various features of Twig, the way function calls are compiled depends on the definition of the function. Making runtime conditional calls would then be difficult. It would force to replace compile-time errors with a compiled code triggering a runtime error.

[spackmat]

Hey, we just ran into that problem also with the csrf_token() function that is disabled in our test environment (as it created errors due to race conditions with tests of the same form running in parallel) and thus does not exist there.

We added a custom Twig function to check, if the CSRF-extension is loaded. First we added a generic function to test, if a function exists, but the getFunction()/getFunctions() methods of the Twig Environment are marked as internal and we did not want to use them.

Having a runtime check for existing Twig functions would be very helpful in that case. We already get a nice error when we call a non defined function at runtime(?), so a test for that should be doable, or did I get something wrong here?

[ericmorand]

@spackmat are your templates expected to work on both situations - i.e. with the CSRF extension enabled and with the CSRF extension disabled?

You say that you had to disable the extension on your test environment, but doesn't it mean that your test environment is not honoring the minimal requirements of your application - that seems to rely on the existence of the extension? If so, it probably means that your test environment can't be trusted as a legit test environment and this should be a greater concern.

Anyway, I think that a runtime check is not ideal. In my opinion, your test environment should provide its own CSRF extension - basically consisting of no-op twig filters and functions - so that your templates are guaranteed to be tested against their expectations. This way you don't have to test the existence of functions in the templates since the template is not expected to work in an environment that does not provide these functions.

This is exactly what we do when we design Drupal or Symfony themes outside of Drupal and Symfony stacks - using either twig.js or Twing as compilers: we inject into the environment all the functions, filters and tags that the templates relies on, as no-op. For example, the famous trans filter is added and all it does is outputting the operand followed by "- Translated in (...)".

As you see, we don't mind that the output is the same accross environments, since the contract of a template is "the template executes this function or filter at this point".

[spackmat]

I am not sure why the CSRF tokens got invalid in random cases so we interpreted that as race conditions: Something like form is rendered in parallel (A and B and maybe some others), then B is submitted before A for whatever reason by the test runner and the token for call A is not valid anymore. Until now I thought that the tokens are valid as long as the session is open, but then we would not get those random invalid token errors in our automated browser based tests. Nevertheless deactivating the CSRF system in the framework config for the test environment is not the best solution, there I agree with you. But I don't have a better solution for now.

I now implemented a no-op csrf_token() function returning a mock token that is only registered in the test environment. I'm still not sure if that is really a better solution, because now I have to make sure that this no-op function is registered, when the csrf-system is disabled in the framework-config for whatever reason in whatever environment.

Btw. is there a way to register functions only, if they are not yet registered?

[ericmorand]

I assume that at some point in your Symfony setup, you disable (or not inject at all) the CSRF dependency when the "environment" variable is set to "test", or something like that?

[spackmat]

We just set framework.csrf_protection to false in the config for the test environment like this:

return static function (ContainerConfigurator $containerConfigurator): void
{
    // …
    if ('test' === $containerConfigurator->env()) {
        $containerConfigurator->extension('framework', [
            'test' => true,
            'csrf_protection' => false,
            'session' => [
                'storage_factory_id' => 'session.storage.factory.mock_file',
            ],
        ]);
    }
};

That seems to disable the whole CSRF-system together with its Twig Extension. And then, in our login forms, where we set the token manually, the csrf_token() function is missing and we need a way to handle that.

[javiereguiluz]

I just faced this problem again. I want to make EasyAdmin support AssetMapper importmaps ... but, if I call {{ importmap(...) }} unconditionally, people who don't have AssetMapper installed will see an exception.

I tried using the {{ ea_call_function_if_exists('importmap', '...') }} trick that I mentioned, but it doesn't work:

An exception has been thrown during the rendering of a template:
"Non-static method Symfony\Bridge\Twig\Extension\ImportMapRuntime::importmap()
cannot be called statically".

This is because the function callable is:

 -callable: array:2 [▼
    0 => "Symfony\Bridge\Twig\Extension\ImportMapRuntime"
    1 => "importmap"
  ]

So, I'll need to recreate the Symfony Twig extension to call it conditionally 😐