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

Comments

damiankloip created an issue. See original summary.

damiankloip’s picture

Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new1.04 KB
damiankloip’s picture

StatusFileSize
new927 bytes
new1.95 KB

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
StatusFileSize
new1.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.