Problem/Motivation

- Create a view of nodes, add an exposed filter on the author (uid)
- Add enough nodes that it will need to use the pager
- Autocomplete the user that created the nodes above, submit the filter form
- Try to go to the next page, or any pager link
- Assertion Error (fatal if configured to do so e.g. settings.local.php config)

AssertionError: Tokens need to be valid Twig variables. in Drupal\Component\Assertion\Handle::Drupal\Component\Assertion\{closure}() (line 377 of core/modules/views/src/Plugin/views/PluginBase.php).

Proposed resolution

Tokens created from exposed input, like the autocomplete element (as it creates the nested structure with [['target_id' => VALUE]]) will generate tokens like arguments.author.0.target_id. In PluginBase::viewsTokenReplace() the tokens are validated with assertions. Most of the time this is fine, but when tokens contains dots, they are split and each piece is evaluated with the same assertion:

assert('preg_match(\'/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/\', $key) === 1', 'Tokens need to be valid Twig variables.');

So using the example token above, one of the parts would be '0' which would fail here. We should check is_numeric() || preg_match() but only in the case each $part is being checked.

Remaining tasks

Tests?

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip created an issue. See original summary.

damiankloip’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
1.04 KB
damiankloip’s picture

And with a test. Test only patch is also the interdiff.

damiankloip’s picture

The last submitted patch, 3: 2718717-3-tests-only-FAIL.patch, failed testing.

catch’s picture

Looks sensible.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Looks ideal for me

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 2718717-3.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
1.95 KB

Reroll.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

RTBC as per #7

+++ b/core/modules/views/src/Plugin/views/PluginBase.php
--- a/core/modules/views/src/Tests/Plugin/PluginBaseTest.php
+++ b/core/modules/views/src/Tests/Plugin/PluginBaseTest.php

It might need another reroll after #2734663: Update deprecation message for old KernelTestBase in simpletest.

dawehner’s picture

It might need another reroll after #2734663: Remove deprecated KernelTestBase from simpletest.

Maybe git can actually figure that out.

  • catch committed 3b64b0d on 8.1.x
    Issue #2718717 by damiankloip: Assertion error when paging a view using...

  • catch committed 3f52754 on 8.2.x
    Issue #2718717 by damiankloip: Assertion error when paging a view using...
catch’s picture

Committed/pushed to both 8.x branches, thanks!

catch’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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