Follow-up to #2492839: Views replacement token bc layer allows for Twig template injection via arguments

Problem/Motivation

% is a reserved indicator and can't start a token.

http://yaml.org/spec/1.2/spec.html#id2774228

Proposed resolution

Quote the value.

Remaining tasks

grumble about symfony's yaml parser/encoder.

User interface changes

n/a

API changes

n/a

Data model changes

n/a

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

neclimdul created an issue. See original summary.

neclimdul’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
651 bytes

Patch.

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

@neclimdul thank you. Do you know if our exporter will properly export it quoted?

neclimdul’s picture

I don't actually, I assumed this was made with the exporter and that it wouldn't quote it hence the grumbling in the IS. If it was manually written, then maybe. I would guess most cases are probably going to be quoted anyways because they'll be longer and trigger some other check in Symfony. This case is short without spaces or anything and starts with the invalid character so it falls in a narrow use case that causes this problem.

mikeker’s picture

Round-tripped test_token_view and the exporter does correctly quote that value, though with single- instead of double-quotes (no escaped characters like "\n" in the string). It makes no difference in terms of parsing, but perhaps in terms of future diffs?

mikeker’s picture

Sorry, x-posted with #4 because I'm trying to eat and post at the same time... :)

To clarify, I added that string in the original issue and did it by editing the existing file which @dawehner originally added so it may have not gone through the exporter... Also, this SO answer does a good job specifying when to use no-, single-, or double-quotes.

neclimdul’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
651 bytes
653 bytes

Yeah, I generally have been single quoting, I don't know what lead me to double quote this.

Bonus, what an interdiff!

mikeker’s picture

Love it when the interdiff is larger than the patch! :)

mikeker’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7df9b06 and pushed to 8.0.x. Thanks!

  • alexpott committed 7df9b06 on 8.0.x
    Issue #2568415 by neclimdul: 2492839 added invalid yaml
    

Status: Fixed » Closed (fixed)

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