From 6c92d014ecb75252e1c1005eaaf3f728bba94db9 Mon Sep 17 00:00:00 2001 From: Hugo Alliaume Date: Wed, 18 Sep 2024 15:16:42 +0200 Subject: [PATCH] [StimulusBundle] Improve `StimulusAttributes` rendering performances by switching to `html` escaping strategy --- src/Chartjs/tests/Twig/ChartExtensionTest.php | 2 +- .../Unit/Twig/LiveComponentRuntimeTest.php | 10 +-- src/Notify/tests/Twig/NotifyRuntimeTest.php | 8 +- .../Twig/ReactComponentExtensionTest.php | 4 +- .../src/Dto/StimulusAttributes.php | 73 ++++++++----------- .../tests/Dto/StimulusAttributesTest.php | 2 +- .../tests/Twig/StimulusTwigExtensionTest.php | 4 +- .../Twig/SvelteComponentExtensionTest.php | 6 +- .../tests/Unit/ComponentAttributesTest.php | 2 +- .../tests/Twig/VueComponentExtensionTest.php | 4 +- 10 files changed, 51 insertions(+), 64 deletions(-) diff --git a/src/Chartjs/tests/Twig/ChartExtensionTest.php b/src/Chartjs/tests/Twig/ChartExtensionTest.php index 03472690cbf..0f3ae34e8b0 100644 --- a/src/Chartjs/tests/Twig/ChartExtensionTest.php +++ b/src/Chartjs/tests/Twig/ChartExtensionTest.php @@ -56,7 +56,7 @@ public function testRenderChart() ); $this->assertSame( - '', + '', $rendered ); } diff --git a/src/LiveComponent/tests/Unit/Twig/LiveComponentRuntimeTest.php b/src/LiveComponent/tests/Unit/Twig/LiveComponentRuntimeTest.php index 30c3ba8cbbf..01f067c08fa 100644 --- a/src/LiveComponent/tests/Unit/Twig/LiveComponentRuntimeTest.php +++ b/src/LiveComponent/tests/Unit/Twig/LiveComponentRuntimeTest.php @@ -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(300)|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:prevent" data-live-pro1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|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); } } diff --git a/src/Notify/tests/Twig/NotifyRuntimeTest.php b/src/Notify/tests/Twig/NotifyRuntimeTest.php index 7f21970fa61..8c3b3786337 100644 --- a/src/Notify/tests/Twig/NotifyRuntimeTest.php +++ b/src/Notify/tests/Twig/NotifyRuntimeTest.php @@ -39,13 +39,13 @@ public function testStreamNotifications(array $params, string $expected) public static function streamNotificationsDataProvider(): iterable { - $publicUrl = 'http://localhost:9090/.well-known/mercure'; + $publicUrl = 'http://localhost:9090/.well-known/mercure'; yield [ [['/topic/1', '/topic/2']], '
', ]; @@ -54,7 +54,7 @@ public static function streamNotificationsDataProvider(): iterable ['/topic/1'], '
', ]; @@ -63,7 +63,7 @@ public static function streamNotificationsDataProvider(): iterable [], '
', ]; diff --git a/src/React/tests/Twig/ReactComponentExtensionTest.php b/src/React/tests/Twig/ReactComponentExtensionTest.php index aae2d6b6b6a..9fbf8c8b0a3 100644 --- a/src/React/tests/Twig/ReactComponentExtensionTest.php +++ b/src/React/tests/Twig/ReactComponentExtensionTest.php @@ -36,7 +36,7 @@ public function testRenderComponent() ); $this->assertSame( - 'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir/MyComponent" data-symfony--ux-react--react-props-value="{"fullName":"Titouan Galopin"}"', + 'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir/MyComponent" data-symfony--ux-react--react-props-value="{"fullName":"Titouan Galopin"}"', $rendered ); } @@ -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/MyComponent"', + 'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir/MyComponent"', $rendered ); } diff --git a/src/StimulusBundle/src/Dto/StimulusAttributes.php b/src/StimulusBundle/src/Dto/StimulusAttributes.php index 1b477172260..5f8466aaeb2 100644 --- a/src/StimulusBundle/src/Dto/StimulusAttributes.php +++ b/src/StimulusBundle/src/Dto/StimulusAttributes.php @@ -107,54 +107,43 @@ 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); + $attributes = []; - // 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']; + if ($this->controllers) { + $attributes[] = 'data-controller="'.$this->escape(implode(' ', $this->controllers), 'html').'"'; + } - $action = $controllerName.'#'.$actionName; - if (null !== $eventName) { - $action = $this->escapeAsHtmlAttr($eventName).'->'.$action; - } + if ($this->actions) { + // done separately so we can escape, but avoid escaping -> + $actions = array_map(function (array $actionData): string { + $controllerName = $actionData['controllerName']; + $actionName = $actionData['actionName']; + $eventName = $actionData['eventName']; - return $action; - }, $this->actions); - - $targets = []; - foreach ($this->targets as $key => $targetNamesString) { - $targetNames = explode(' ', $targetNamesString); - $targets[$key] = implode(' ', array_map(function (string $targetName): string { - return $this->escapeAsHtmlAttr($targetName); - }, $targetNames)); - } + $action = $this->escape($controllerName.'#'.$actionName, 'html'); + if (null !== $eventName) { + $action = $this->escape($eventName, 'html').'->'.$action; + } - $attributes = []; + return $action; + }, $this->actions); - if ($controllers) { - $attributes[] = \sprintf('data-controller="%s"', implode(' ', $controllers)); + $attributes[] = 'data-action="'.implode(' ', $actions).'"'; } - if ($actions) { - $attributes[] = \sprintf('data-action="%s"', implode(' ', $actions)); + if ($this->targets) { + $attributes[] = implode(' ', array_map(function (string $key, string $value): string { + return $this->escape($key, 'html_attr').'="'.$this->escape($value, 'html').'"'; + }, array_keys($this->targets), $this->targets)); } - if ($targets) { - $attributes[] = implode(' ', array_map(function (string $key, string $value): string { - return \sprintf('%s="%s"', $key, $value); - }, array_keys($targets), $targets)); + if ($this->attributes) { + $attributes[] = implode(' ', array_map(function (string $attribute, string $value): string { + return $this->escape($attribute, 'html_attr').'="'.$this->escape($value, 'html').'"'; + }, array_keys($this->attributes), $this->attributes)); } - 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 @@ -193,7 +182,7 @@ public function toEscapedArray(): array { $escaped = []; foreach ($this->toArray() as $key => $value) { - $escaped[$key] = $this->escapeAsHtmlAttr($value); + $escaped[$key] = $this->escape($value, 'html'); } return $escaped; @@ -212,18 +201,18 @@ private function getFormattedValue(mixed $value): string return (string) $value; } - private function escapeAsHtmlAttr(mixed $value): string + private function escape(mixed $value, string $strategy): 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); } /** diff --git a/src/StimulusBundle/tests/Dto/StimulusAttributesTest.php b/src/StimulusBundle/tests/Dto/StimulusAttributesTest.php index 63393d99cd9..17527a3491c 100644 --- a/src/StimulusBundle/tests/Dto/StimulusAttributesTest.php +++ b/src/StimulusBundle/tests/Dto/StimulusAttributesTest.php @@ -145,7 +145,7 @@ public function testIsTraversable() public function testAddAttribute() { $this->stimulusAttributes->addAttribute('foo', 'bar baz'); - $this->assertSame('foo="bar baz"', (string) $this->stimulusAttributes); + $this->assertSame('foo="bar baz"', (string) $this->stimulusAttributes); $this->assertSame(['foo' => 'bar baz'], $this->stimulusAttributes->toArray()); } } diff --git a/src/StimulusBundle/tests/Twig/StimulusTwigExtensionTest.php b/src/StimulusBundle/tests/Twig/StimulusTwigExtensionTest.php index 41793ed5efc..0e8cdbf8893 100644 --- a/src/StimulusBundle/tests/Twig/StimulusTwigExtensionTest.php +++ b/src/StimulusBundle/tests/Twig/StimulusTwigExtensionTest.php @@ -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 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="{"key":"Value with quotes ' and \"."}"', + (string) $extension->appendStimulusController($dto, 'another-controller', ['another-value' => 'scalar-value 2', 'jsonValue' => json_encode(['key' => 'Value with quotes \' and ".'])]), ); } diff --git a/src/Svelte/tests/Twig/SvelteComponentExtensionTest.php b/src/Svelte/tests/Twig/SvelteComponentExtensionTest.php index 8231c0282db..7a9a6d03263 100644 --- a/src/Svelte/tests/Twig/SvelteComponentExtensionTest.php +++ b/src/Svelte/tests/Twig/SvelteComponentExtensionTest.php @@ -37,7 +37,7 @@ public function testRenderComponent() ); $this->assertSame( - 'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent" data-symfony--ux-svelte--svelte-props-value="{"fullName":"Titouan Galopin"}"', + 'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent" data-symfony--ux-svelte--svelte-props-value="{"fullName":"Titouan Galopin"}"', $rendered ); } @@ -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/MyComponent"', + 'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent"', $rendered ); } @@ -73,7 +73,7 @@ public function testRenderComponentWithIntro() ); $this->assertSame( - 'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent" data-symfony--ux-svelte--svelte-props-value="{"fullName":"Titouan Galopin"}" 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="{"fullName":"Titouan Galopin"}" data-symfony--ux-svelte--svelte-intro-value="true"', $rendered ); } diff --git a/src/TwigComponent/tests/Unit/ComponentAttributesTest.php b/src/TwigComponent/tests/Unit/ComponentAttributesTest.php index 5e22ac737c8..8ed7aa4f004 100644 --- a/src/TwigComponent/tests/Unit/ComponentAttributesTest.php +++ b/src/TwigComponent/tests/Unit/ComponentAttributesTest.php @@ -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' => '["a","b"]', + 'data-foo-some-array-value' => '["a","b"]', ], $attributes->all()); } diff --git a/src/Vue/tests/Twig/VueComponentExtensionTest.php b/src/Vue/tests/Twig/VueComponentExtensionTest.php index 3c618cca2e6..6a539395d4d 100644 --- a/src/Vue/tests/Twig/VueComponentExtensionTest.php +++ b/src/Vue/tests/Twig/VueComponentExtensionTest.php @@ -37,7 +37,7 @@ public function testRenderComponent() ); $this->assertSame( - 'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir/MyComponent" data-symfony--ux-vue--vue-props-value="{"fullName":"Titouan Galopin"}"', + 'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir/MyComponent" data-symfony--ux-vue--vue-props-value="{"fullName":"Titouan Galopin"}"', $rendered ); } @@ -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/MyComponent"', + 'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir/MyComponent"', $rendered ); }