Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[StimulusBundle] Improve StimulusAttributes rendering performances by switching to html escaping strategy #2180

Merged
merged 1 commit into from
Sep 24, 2024
Merged

[StimulusBundle] Improve StimulusAttributes rendering performances by switching to html escaping strategy #2180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Chartjs/tests/Twig/ChartExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function testRenderChart()
);

$this->assertSame(
'<canvas data-controller="mycontroller symfony--ux-chartjs--chart" data-symfony--ux-chartjs--chart-view-value="&#x7B;&quot;type&quot;&#x3A;&quot;line&quot;,&quot;data&quot;&#x3A;&#x7B;&quot;labels&quot;&#x3A;&#x5B;&quot;January&quot;,&quot;February&quot;,&quot;March&quot;,&quot;April&quot;,&quot;May&quot;,&quot;June&quot;,&quot;July&quot;&#x5D;,&quot;datasets&quot;&#x3A;&#x5B;&#x7B;&quot;label&quot;&#x3A;&quot;My&#x20;First&#x20;dataset&quot;,&quot;backgroundColor&quot;&#x3A;&quot;rgb&#x28;255,&#x20;99,&#x20;132&#x29;&quot;,&quot;borderColor&quot;&#x3A;&quot;rgb&#x28;255,&#x20;99,&#x20;132&#x29;&quot;,&quot;data&quot;&#x3A;&#x5B;0,10,5,2,20,30,45&#x5D;&#x7D;&#x5D;&#x7D;,&quot;options&quot;&#x3A;&#x7B;&quot;showLines&quot;&#x3A;false&#x7D;&#x7D;" class="myclass"></canvas>',
'<canvas data-controller="mycontroller symfony--ux-chartjs--chart" data-symfony--ux-chartjs--chart-view-value="{&quot;type&quot;:&quot;line&quot;,&quot;data&quot;:{&quot;labels&quot;:[&quot;January&quot;,&quot;February&quot;,&quot;March&quot;,&quot;April&quot;,&quot;May&quot;,&quot;June&quot;,&quot;July&quot;],&quot;datasets&quot;:[{&quot;label&quot;:&quot;My First dataset&quot;,&quot;backgroundColor&quot;:&quot;rgb(255, 99, 132)&quot;,&quot;borderColor&quot;:&quot;rgb(255, 99, 132)&quot;,&quot;data&quot;:[0,10,5,2,20,30,45]}]},&quot;options&quot;:{&quot;showLines&quot;:false}}" class="myclass"></canvas>',
$rendered
);
}
Expand Down
10 changes: 4 additions & 6 deletions src/LiveComponent/tests/Unit/Twig/LiveComponentRuntimeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,15 @@ public function testGetLiveAction(): void
$this->assertSame('data-action="live#action" data-live-prop1-param="val1" data-live-some-prop-param="val2" data-live-action-param="action-name"', $props);

$props = $runtime->liveAction('action-name', ['prop1' => 'val1', 'prop2' => 'val2'], ['debounce' => 300]);
$this->assertSame('data-action="live#action" data-live-prop1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|action-name"', html_entity_decode($props));
$this->assertSame('data-action="live#action" data-live-prop1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce&#x28;300&#x29;&#x7C;action-name"', $props);
$this->assertSame('data-action="live#action" data-live-prop1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|action-name"', $props);

$props = $runtime->liveAction('action-name:prevent', ['pro1' => 'val1', 'prop2' => 'val2'], ['debounce' => 300]);
$this->assertSame('data-action="live#action:prevent" data-live-pro1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|action-name"', html_entity_decode($props));
$this->assertSame('data-action="live#action&#x3A;prevent" data-live-pro1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce&#x28;300&#x29;&#x7C;action-name"', $props);
$this->assertSame('data-action="live#action:prevent" data-live-pro1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|action-name"', $props);

$props = $runtime->liveAction('action-name:prevent', [], ['debounce' => 300]);
$this->assertSame('data-action="live#action:prevent" data-live-action-param="debounce(300)|action-name"', html_entity_decode($props));
$this->assertSame('data-action="live#action:prevent" data-live-action-param="debounce(300)|action-name"', $props);

$props = $runtime->liveAction('action-name', [], [], 'keydown.esc');
$this->assertSame('data-action="keydown.esc->live#action" data-live-action-param="action-name"', html_entity_decode($props));
$this->assertSame('data-action="keydown.esc->live#action" data-live-action-param="action-name"', $props);
}
}
8 changes: 4 additions & 4 deletions src/Notify/tests/Twig/NotifyRuntimeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ public function testStreamNotifications(array $params, string $expected)

public static function streamNotificationsDataProvider(): iterable
{
$publicUrl = 'http&#x3A;&#x2F;&#x2F;localhost&#x3A;9090&#x2F;.well-known&#x2F;mercure';
$publicUrl = 'http://localhost:9090/.well-known/mercure';

yield [
[['/topic/1', '/topic/2']],
'<div '.
'data-controller="symfony--ux-notify--notify" '.
'data-symfony--ux-notify--notify-topics-value="&#x5B;&quot;&#x5C;&#x2F;topic&#x5C;&#x2F;1&quot;,&quot;&#x5C;&#x2F;topic&#x5C;&#x2F;2&quot;&#x5D;" '.
'data-symfony--ux-notify--notify-topics-value="[&quot;\/topic\/1&quot;,&quot;\/topic\/2&quot;]" '.
'data-symfony--ux-notify--notify-hub-value="'.$publicUrl.'"'.
'></div>',
];
Expand All @@ -54,7 +54,7 @@ public static function streamNotificationsDataProvider(): iterable
['/topic/1'],
'<div '.
'data-controller="symfony--ux-notify--notify" '.
'data-symfony--ux-notify--notify-topics-value="&#x5B;&quot;&#x5C;&#x2F;topic&#x5C;&#x2F;1&quot;&#x5D;" '.
'data-symfony--ux-notify--notify-topics-value="[&quot;\/topic\/1&quot;]" '.
'data-symfony--ux-notify--notify-hub-value="'.$publicUrl.'"'.
'></div>',
];
Expand All @@ -63,7 +63,7 @@ public static function streamNotificationsDataProvider(): iterable
[],
'<div '.
'data-controller="symfony--ux-notify--notify" '.
'data-symfony--ux-notify--notify-topics-value="&#x5B;&quot;https&#x3A;&#x5C;&#x2F;&#x5C;&#x2F;symfony.com&#x5C;&#x2F;notifier&quot;&#x5D;" '.
'data-symfony--ux-notify--notify-topics-value="[&quot;https:\/\/symfony.com\/notifier&quot;]" '.
'data-symfony--ux-notify--notify-hub-value="'.$publicUrl.'"'.
'></div>',
];
Expand Down
4 changes: 2 additions & 2 deletions src/React/tests/Twig/ReactComponentExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function testRenderComponent()
);

$this->assertSame(
'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir&#x2F;MyComponent" data-symfony--ux-react--react-props-value="&#x7B;&quot;fullName&quot;&#x3A;&quot;Titouan&#x20;Galopin&quot;&#x7D;"',
'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir/MyComponent" data-symfony--ux-react--react-props-value="{&quot;fullName&quot;:&quot;Titouan Galopin&quot;}"',
$rendered
);
}
Expand All @@ -52,7 +52,7 @@ public function testRenderComponentWithoutProps()
$rendered = $extension->renderReactComponent('SubDir/MyComponent');

$this->assertSame(
'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir&#x2F;MyComponent"',
'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir/MyComponent"',
$rendered
);
}
Expand Down
68 changes: 25 additions & 43 deletions src/StimulusBundle/src/Dto/StimulusAttributes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,54 +107,36 @@ public function addAttribute(string $name, string $value): void

public function __toString(): string
{
$controllers = array_map(function (string $controllerName): string {
return $this->escapeAsHtmlAttr($controllerName);
}, $this->controllers);

// done separately so we can escape, but avoid escaping ->
$actions = array_map(function (array $actionData): string {
$controllerName = $this->escapeAsHtmlAttr($actionData['controllerName']);
$actionName = $this->escapeAsHtmlAttr($actionData['actionName']);
$eventName = $actionData['eventName'];

$action = $controllerName.'#'.$actionName;
if (null !== $eventName) {
$action = $this->escapeAsHtmlAttr($eventName).'->'.$action;
}

return $action;
}, $this->actions);
$attributes = [];

$targets = [];
foreach ($this->targets as $key => $targetNamesString) {
$targetNames = explode(' ', $targetNamesString);
$targets[$key] = implode(' ', array_map(function (string $targetName): string {
return $this->escapeAsHtmlAttr($targetName);
}, $targetNames));
if ($this->controllers) {
$attributes[] = 'data-controller="'.$this->escape(implode(' ', $this->controllers)).'"';
}

$attributes = [];
if ($this->actions) {
$actions = [];
foreach ($this->actions as ['controllerName' => $controllerName, 'actionName' => $actionName, 'eventName' => $eventName]) {
$action = $this->escape($controllerName.'#'.$actionName);
if (null !== $eventName) {
// done separately so we can escape, but avoid escaping ->
$action = $this->escape($eventName).'->'.$action;
}

$actions[] = $action;
}

if ($controllers) {
$attributes[] = \sprintf('data-controller="%s"', implode(' ', $controllers));
$attributes[] = 'data-action="'.implode(' ', $actions).'"';
}

if ($actions) {
$attributes[] = \sprintf('data-action="%s"', implode(' ', $actions));
foreach ($this->targets as $k => $v) {
$attributes[] = $this->escape($k, 'html_attr').'="'.$this->escape($v).'"';
}

if ($targets) {
$attributes[] = implode(' ', array_map(function (string $key, string $value): string {
return \sprintf('%s="%s"', $key, $value);
}, array_keys($targets), $targets));
foreach ($this->attributes as $k => $v) {
$attributes[] = $this->escape($k, 'html_attr').'="'.$this->escape($v).'"';
}

return rtrim(implode(' ', [
...$attributes,
...array_map(function (string $attribute, string $value): string {
return $attribute.'="'.$this->escapeAsHtmlAttr($value).'"';
}, array_keys($this->attributes), $this->attributes),
]));
return implode(' ', $attributes);
}

public function toArray(): array
Expand Down Expand Up @@ -193,7 +175,7 @@ public function toEscapedArray(): array
{
$escaped = [];
foreach ($this->toArray() as $key => $value) {
$escaped[$key] = $this->escapeAsHtmlAttr($value);
$escaped[$key] = $this->escape($value);
}

return $escaped;
Expand All @@ -212,18 +194,18 @@ private function getFormattedValue(mixed $value): string
return (string) $value;
}

private function escapeAsHtmlAttr(mixed $value): string
private function escape(mixed $value, string $strategy = 'html'): string
{
if (class_exists(EscaperRuntime::class)) {
return $this->env->getRuntime(EscaperRuntime::class)->escape($value, 'html_attr');
return $this->env->getRuntime(EscaperRuntime::class)->escape($value, $strategy);
}

if (method_exists(EscaperExtension::class, 'escape')) {
return EscaperExtension::escape($this->env, $value, 'html_attr');
return EscaperExtension::escape($this->env, $value, $strategy);
}

// since twig/twig 3.9.0: Using the internal "twig_escape_filter" function is deprecated.
return (string) twig_escape_filter($this->env, $value, 'html_attr');
return (string) twig_escape_filter($this->env, $value, $strategy);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/StimulusBundle/tests/Dto/StimulusAttributesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public function testIsTraversable()
public function testAddAttribute()
{
$this->stimulusAttributes->addAttribute('foo', 'bar baz');
$this->assertSame('foo="bar&#x20;baz"', (string) $this->stimulusAttributes);
$this->assertSame('foo="bar baz"', (string) $this->stimulusAttributes);
$this->assertSame(['foo' => 'bar baz'], $this->stimulusAttributes->toArray());
}
}
4 changes: 2 additions & 2 deletions src/StimulusBundle/tests/Twig/StimulusTwigExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ public function testAppendStimulusController(): void
$extension = new StimulusTwigExtension(new StimulusHelper($this->twig));
$dto = $extension->renderStimulusController('my-controller', ['myValue' => 'scalar-value']);
$this->assertSame(
'data-controller="my-controller another-controller" data-my-controller-my-value-value="scalar-value" data-another-controller-another-value-value="scalar-value&#x20;2"',
(string) $extension->appendStimulusController($dto, 'another-controller', ['another-value' => 'scalar-value 2']),
'data-controller="my-controller another-controller" data-my-controller-my-value-value="scalar-value" data-another-controller-another-value-value="scalar-value 2" data-another-controller-json-value-value="{&quot;key&quot;:&quot;Value with quotes &#039; and \&quot;.&quot;}"',
(string) $extension->appendStimulusController($dto, 'another-controller', ['another-value' => 'scalar-value 2', 'jsonValue' => json_encode(['key' => 'Value with quotes \' and ".'])]),
);
}

Expand Down
6 changes: 3 additions & 3 deletions src/Svelte/tests/Twig/SvelteComponentExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function testRenderComponent()
);

$this->assertSame(
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir&#x2F;MyComponent" data-symfony--ux-svelte--svelte-props-value="&#x7B;&quot;fullName&quot;&#x3A;&quot;Titouan&#x20;Galopin&quot;&#x7D;"',
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent" data-symfony--ux-svelte--svelte-props-value="{&quot;fullName&quot;:&quot;Titouan Galopin&quot;}"',
$rendered
);
}
Expand All @@ -53,7 +53,7 @@ public function testRenderComponentWithoutProps()
$rendered = $extension->renderSvelteComponent('SubDir/MyComponent');

$this->assertSame(
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir&#x2F;MyComponent"',
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent"',
$rendered
);
}
Expand All @@ -73,7 +73,7 @@ public function testRenderComponentWithIntro()
);

$this->assertSame(
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir&#x2F;MyComponent" data-symfony--ux-svelte--svelte-props-value="&#x7B;&quot;fullName&quot;&#x3A;&quot;Titouan&#x20;Galopin&quot;&#x7D;" data-symfony--ux-svelte--svelte-intro-value="true"',
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent" data-symfony--ux-svelte--svelte-props-value="{&quot;fullName&quot;:&quot;Titouan Galopin&quot;}" data-symfony--ux-svelte--svelte-intro-value="true"',
$rendered
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/TwigComponent/tests/Unit/ComponentAttributesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public function testCanAddStimulusControllerViaStimulusAttributes(): void
'data-controller' => 'foo live',
'data-live-data-value' => '{}',
'data-foo-name-value' => 'ryan',
'data-foo-some-array-value' => '&#x5B;&quot;a&quot;,&quot;b&quot;&#x5D;',
'data-foo-some-array-value' => '[&quot;a&quot;,&quot;b&quot;]',
], $attributes->all());
}

Expand Down
4 changes: 2 additions & 2 deletions src/Vue/tests/Twig/VueComponentExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function testRenderComponent()
);

$this->assertSame(
'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir&#x2F;MyComponent" data-symfony--ux-vue--vue-props-value="&#x7B;&quot;fullName&quot;&#x3A;&quot;Titouan&#x20;Galopin&quot;&#x7D;"',
'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir/MyComponent" data-symfony--ux-vue--vue-props-value="{&quot;fullName&quot;:&quot;Titouan Galopin&quot;}"',
$rendered
);
}
Expand All @@ -53,7 +53,7 @@ public function testRenderComponentWithoutProps()
$rendered = $extension->renderVueComponent('SubDir/MyComponent');

$this->assertSame(
'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir&#x2F;MyComponent"',
'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir/MyComponent"',
$rendered
);
}
Expand Down
Loading