Problem/Motivation

If assert.active is enabled (default PHP behaviour), and you open a view with non-standard parameters, Views will add the invalid parameters as twig tokens, and will fail the assertion later down the line, and throw the following exception:

The website encountered an unexpected error. Please try again later.

AssertionError: Tokens need to be valid Twig variables. in assert() (line 390 of core/modules/views/src/Plugin/views/PluginBase.php).
Drupal\views\Plugin\views\PluginBase->viewsTokenReplace('<a href="{{ view_node }}">{{ type }}</a>', Array) (Line: 1396)
Drupal\views\Plugin\views\field\FieldPluginBase->renderAltered(Array, Array) (Line: 1289)
Drupal\views\Plugin\views\field\FieldPluginBase->renderText(Array) (Line: 1243)
Drupal\views\Plugin\views\field\FieldPluginBase->advancedRender(Object) (Line: 238)
template_preprocess_views_view_field(Array, 'views_view_field', Array)
call_user_func_array('template_preprocess_views_view_field', Array) (Line: 285)
Drupal\Core\Theme\ThemeManager->render('views_view_field', Array) (Line: 433)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 204)
Drupal\Core\Render\Renderer->render(Array) (Line: 1788)
Drupal\views\Plugin\views\field\FieldPluginBase->theme(Object) (Line: 758)
Drupal\views\Plugin\views\style\StylePluginBase->elementPreRenderRow(Array)

We should only add valid twig tokens in the list.

Steps to reproduce

PHP assertions might be on by default, but if they aren't, you may enforce it by adding the following to your local settings.php:

assert_options(ASSERT_ACTIVE, TRUE);
\Drupal\Component\Assertion\Handle::register();

Attached a copy of a sample

views.view.list_of_users.yml

view config to replicate the issue.

  • Create a view which includes a field which incorporates token rewrites
  • Add the following query string to the view's URL: ?amp;page=0&amp;category[64]=64&page=0
  • Views will attempt to add invalid tokens such as: {{ arguments.amp;page }} into the list and subsequently fail its assertions

Proposed resolution

Validate that the query string parameters injected are actually valid twig variable tokens, skipping them otherwise.

Or alternatively detect those non-standard parameters, and replace with {{ arguments['amp;page'] }} type syntax.

Remaining tasks

Provide issue fork/patch.

User interface changes

N/A

API changes

N/A

Issue fork drupal-3414144

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

codebymikey created an issue. See original summary.

codebymikey’s picture

Assigned: Unassigned » codebymikey
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new2.95 KB
new4.56 KB
new4.69 KB

Attached a patch which stops invalid query parameters from being added in.

I also updated the documentation for the \Drupal\views\Plugin\views\field\FieldPluginBase::getTokenValuesRecursive function.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

MR seems to have failures,

FYI don't need to upload test-only patches, MRs have a test-only feature run.

Left some nitpicky comments also.

codebymikey’s picture

Status: Needs work » Needs review

Wasn't aware of the test-only pipelines, I've updated the coding standard issues and tests seem to pass.

codebymikey’s picture

StatusFileSize
new4.76 KB
smustgrave’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Appears you ran the test-only feature so posting results here for quick glance.

Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
     '{{ arguments.foo.b }}' => 'value'
     '{{ arguments.bar.a }}' => 'value'
     '{{ arguments.bar.b.c }}' => 'value'
+    '{{ arguments.foo.c.d }}' => 'invalid argument'
+    '{{ arguments.foo.&invalid }}' => 'invalid argument'
 )
/builds/issue/drupal-3414144/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
/builds/issue/drupal-3414144/core/modules/views/tests/src/Unit/Plugin/field/FieldPluginBaseTest.php:739
/builds/issue/drupal-3414144/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
/builds/issue/drupal-3414144/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-3414144/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-3414144/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/builds/issue/drupal-3414144/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/builds/issue/drupal-3414144/vendor/phpunit/phpunit/src/TextUI/Command.php:97
FAILURES!

Issue summary appears complete.

Change seems small enough with the additional check that I can mark it now.

  • catch committed 0f25cc2f on 10.2.x
    Issue #3414144 by codebymikey, smustgrave: Invalid twig token variables...

  • catch committed b4f3d7fa on 11.x
    Issue #3414144 by codebymikey, smustgrave: Invalid twig token variables...
catch’s picture

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

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

Status: Fixed » Closed (fixed)

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