[TwigExtra] Escape potential html in test attributes #251
Conversation
|
|
||
| foreach ($attributes as $name => $value) { | ||
| $result[sprintf('data-test-%s', $name)] = (string) $value; | ||
| $escapedValue = htmlspecialchars((string) $value, \ENT_QUOTES | \ENT_SUBSTITUTE, 'UTF-8'); |
There was a problem hiding this comment.
My main opinion is that it'd be better to escape this in Twig itself to use escaper strategy. but maybe it'd be overkilled.
There was a problem hiding this comment.
This won't be escaped in Twig because we're adding ['is_safe' => ['html']]. This tells Twig's escaper that the content is safe HTML, so it won't apply automatic escaping
There was a problem hiding this comment.
other option is to add in every twig
{{ sylius_test_html_attribute('test-button', value|escape) }}
There was a problem hiding this comment.
I'm wondering if there are a lot test html that have this value. A lot of them are just empty data test attributes
There was a problem hiding this comment.
I'm out of the loop and when and why this change is needed, but code looks good.
Escape used 3 times across two classes, so maybe escaping strategy could be extracted into separate class/function, but given it's just a twig test classes current calls looks good enough.
No description provided.