Fix double escaping of the decision attributes in the profiler #21387

Merged
merged 1 commit into from Jan 24, 2017

Projects

None yet

5 participants

@stof
Member
stof commented Jan 24, 2017
Q A
Branch? 3.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #21384
License MIT
Doc PR n/a

A ternary operator is considered safe by the Twig auto-escaping only when both branches are safe. But this ternary was safe only in the ELSE branch, causing it to be unsafe. This triggered a double-escaping of the value (escaping the output of the dump). The fix is to use a {% if %} and 2 separate output statements, allowing them to be auto-escaped separately.

@stof stof Fix double escaping of the decision attributes in the profiler
A ternary operator is considered safe by the Twig auto-escaping only when
both branches are safe. But this ternary was safe only in the ELSE branch,
causing it to be unsafe. This triggered a double-escaping of the value
(escaping the output of the dump). The fix is to use a {% if %} and 2 separate
output statements, allowing them to be auto-escaped separately.
bc1f084
@stof
Member
stof commented Jan 24, 2017

Note that 3.1 is not affected, because profiler_dump is not safe there

@iltar
Contributor
iltar commented Jan 24, 2017

Okay, I have to admit, I would not have guessed this was even possible 😆

@stof stof changed the base branch to symfony:3.2 from symfony:master Jan 24, 2017
@nicolas-grekas nicolas-grekas added this to the 3.2 milestone Jan 24, 2017
@nicolas-grekas
Member

Good catch, thanks @stof.

@nicolas-grekas nicolas-grekas merged commit bc1f084 into symfony:3.2 Jan 24, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor was unable to build non-mergeable pull request
Details
fabbot.io Your code looks good.
Details
@nicolas-grekas nicolas-grekas added a commit that referenced this pull request Jan 24, 2017
@nicolas-grekas nicolas-grekas bug #21387 Fix double escaping of the decision attributes in the prof…
…iler (stof)

This PR was merged into the 3.2 branch.

Discussion
----------

Fix double escaping of the decision attributes in the profiler

| Q             | A
| ------------- | ---
| Branch?       | 3.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21384
| License       | MIT
| Doc PR        | n/a

A ternary operator is considered safe by the Twig auto-escaping only when both branches are safe. But this ternary was safe only in the ELSE branch, causing it to be unsafe. This triggered a double-escaping of the value (escaping the output of the dump). The fix is to use a {% if %} and 2 separate output statements, allowing them to be auto-escaped separately.

Commits
-------

bc1f084 Fix double escaping of the decision attributes in the profiler
2cf8452
@stof stof deleted the stof:fix_double_escaping branch Jan 24, 2017
@javiereguiluz
Member

@stof super nice catch! Thanks for the fix and the explanation.

@fabpot fabpot referenced this pull request Feb 6, 2017
Merged

Release v3.2.3 #21544

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment