diff --git a/src/LiveComponent/composer.json b/src/LiveComponent/composer.json index ee296ded93f..8f3e4d9b8db 100644 --- a/src/LiveComponent/composer.json +++ b/src/LiveComponent/composer.json @@ -31,7 +31,7 @@ "symfony/property-access": "^5.4.5|^6.0|^7.0", "symfony/property-info": "^5.4|^6.0|^7.0", "symfony/stimulus-bundle": "^2.9", - "symfony/ux-twig-component": "^2.25", + "symfony/ux-twig-component": "^2.25.1", "twig/twig": "^3.10.3" }, "require-dev": { diff --git a/src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php b/src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php index da90c863329..5fedbf522a0 100644 --- a/src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php +++ b/src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php @@ -15,7 +15,6 @@ use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; -use Symfony\Component\DependencyInjection\Argument\AbstractArgument; use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument; use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -110,7 +109,7 @@ function (ChildDefinition $definition, AsLiveComponent $attribute) { new Reference('ux.live_component.metadata_factory'), new Reference('serializer', ContainerInterface::NULL_ON_INVALID_REFERENCE), $config['secret'], // defaults to '%kernel.secret%' - new Reference('ux.twig_component.component_attributes_factory'), + new Reference('twig'), ]) ; @@ -158,7 +157,7 @@ function (ChildDefinition $definition, AsLiveComponent $attribute) { ->setArguments([ new Reference('ux.live_component.fingerprint_calculator'), new Reference('ux.live_component.attribute_helper_factory'), - new Reference('ux.twig_component.component_attributes_factory'), + new Reference('twig'), ]) ->addTag('container.service_subscriber', ['key' => ComponentFactory::class, 'id' => 'ux.twig_component.component_factory']) ->addTag('container.service_subscriber', ['key' => LiveComponentMetadataFactory::class, 'id' => 'ux.live_component.metadata_factory']) @@ -219,7 +218,7 @@ function (ChildDefinition $definition, AsLiveComponent $attribute) { ->setArguments([ new Reference('ux.twig_component.component_stack'), new Reference('ux.live_component.twig.template_mapper'), - new Reference('ux.twig_component.component_attributes_factory'), + new Reference('twig'), ]) ->addTag('kernel.event_subscriber') ->addTag('container.service_subscriber', ['key' => LiveControllerAttributesCreator::class, 'id' => 'ux.live_component.live_controller_attributes_creator']) diff --git a/src/LiveComponent/src/EventListener/AddLiveAttributesSubscriber.php b/src/LiveComponent/src/EventListener/AddLiveAttributesSubscriber.php index d424c3b93d4..fdcdf4e4426 100644 --- a/src/LiveComponent/src/EventListener/AddLiveAttributesSubscriber.php +++ b/src/LiveComponent/src/EventListener/AddLiveAttributesSubscriber.php @@ -17,11 +17,12 @@ use Symfony\UX\LiveComponent\Twig\TemplateMap; use Symfony\UX\LiveComponent\Util\LiveControllerAttributesCreator; use Symfony\UX\TwigComponent\ComponentAttributes; -use Symfony\UX\TwigComponent\ComponentAttributesFactory; use Symfony\UX\TwigComponent\ComponentMetadata; use Symfony\UX\TwigComponent\ComponentStack; use Symfony\UX\TwigComponent\Event\PreRenderEvent; use Symfony\UX\TwigComponent\MountedComponent; +use Twig\Environment; +use Twig\Runtime\EscaperRuntime; /** * Adds the extra attributes needed to activate a live controller. @@ -37,7 +38,7 @@ final class AddLiveAttributesSubscriber implements EventSubscriberInterface, Ser public function __construct( private ComponentStack $componentStack, private TemplateMap $templateMap, - private readonly ComponentAttributesFactory $componentAttributesFactory, + private readonly Environment $twig, private ContainerInterface $container, ) { } @@ -107,6 +108,6 @@ private function getLiveAttributes(MountedComponent $mounted, ComponentMetadata $this->componentStack->hasParentComponent() ); - return $this->componentAttributesFactory->create($attributesCollection->toArray()); + return new ComponentAttributes($attributesCollection->toArray(), $this->twig->getRuntime(EscaperRuntime::class)); } } diff --git a/src/LiveComponent/src/LiveComponentHydrator.php b/src/LiveComponent/src/LiveComponentHydrator.php index bbf125e0363..b0014ab19df 100644 --- a/src/LiveComponent/src/LiveComponentHydrator.php +++ b/src/LiveComponent/src/LiveComponentHydrator.php @@ -32,7 +32,8 @@ use Symfony\UX\LiveComponent\Metadata\LivePropMetadata; use Symfony\UX\LiveComponent\Util\DehydratedProps; use Symfony\UX\TwigComponent\ComponentAttributes; -use Symfony\UX\TwigComponent\ComponentAttributesFactory; +use Twig\Environment; +use Twig\Runtime\EscaperRuntime; /** * @author Kevin Bond @@ -53,7 +54,7 @@ public function __construct( private LiveComponentMetadataFactory $liveComponentMetadataFactory, private NormalizerInterface|DenormalizerInterface|null $serializer, #[\SensitiveParameter] private string $secret, - private readonly ComponentAttributesFactory $componentAttributesFactory, + private readonly Environment $twig, ) { if (!$secret) { throw new \InvalidArgumentException('A non-empty secret is required.'); @@ -146,7 +147,7 @@ public function hydrate(object $component, array $props, array $updatedProps, Li $dehydratedOriginalProps = $this->combineAndValidateProps($props, $updatedPropsFromParent); $dehydratedUpdatedProps = DehydratedProps::createFromUpdatedArray($updatedProps); - $attributes = $this->componentAttributesFactory->create($dehydratedOriginalProps->getPropValue(self::ATTRIBUTES_KEY, [])); + $attributes = new ComponentAttributes($dehydratedOriginalProps->getPropValue(self::ATTRIBUTES_KEY, []), $this->twig->getRuntime(EscaperRuntime::class)); $dehydratedOriginalProps->removePropValue(self::ATTRIBUTES_KEY); $needProcessOnUpdatedHooks = []; diff --git a/src/LiveComponent/src/Util/ChildComponentPartialRenderer.php b/src/LiveComponent/src/Util/ChildComponentPartialRenderer.php index e3651ad2e84..d6fc45c9f06 100644 --- a/src/LiveComponent/src/Util/ChildComponentPartialRenderer.php +++ b/src/LiveComponent/src/Util/ChildComponentPartialRenderer.php @@ -15,8 +15,10 @@ use Symfony\Contracts\Service\ServiceSubscriberInterface; use Symfony\UX\LiveComponent\LiveComponentHydrator; use Symfony\UX\LiveComponent\Metadata\LiveComponentMetadataFactory; -use Symfony\UX\TwigComponent\ComponentAttributesFactory; +use Symfony\UX\TwigComponent\ComponentAttributes; use Symfony\UX\TwigComponent\ComponentFactory; +use Twig\Environment; +use Twig\Runtime\EscaperRuntime; /** * @author Ryan Weaver @@ -28,7 +30,7 @@ class ChildComponentPartialRenderer implements ServiceSubscriberInterface public function __construct( private FingerprintCalculator $fingerprintCalculator, private TwigAttributeHelperFactory $attributeHelperFactory, - private ComponentAttributesFactory $componentAttributesFactory, + private Environment $twig, private ContainerInterface $container, ) { } @@ -85,7 +87,7 @@ public function renderChildComponent(string $deterministicId, string $currentPro private function createHtml(array $attributes, string $childTag): string { $attributes['data-live-preserve'] = true; - $attributes = $this->componentAttributesFactory->create($attributes); + $attributes = new ComponentAttributes($attributes, $this->twig->getRuntime(EscaperRuntime::class)); return \sprintf('<%s%s>', $childTag, $attributes, $childTag); } diff --git a/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php b/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php index be0d53d413c..9df33c46c22 100644 --- a/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php +++ b/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php @@ -43,8 +43,9 @@ use Symfony\UX\LiveComponent\Tests\Fixtures\Enum\ZeroIntEnum; use Symfony\UX\LiveComponent\Tests\LiveComponentTestHelper; use Symfony\UX\TwigComponent\ComponentAttributes; -use Symfony\UX\TwigComponent\ComponentAttributesFactory; use Symfony\UX\TwigComponent\ComponentMetadata; +use Twig\Environment; +use Twig\Runtime\EscaperRuntime; use Zenstruck\Foundry\Test\Factories; use Zenstruck\Foundry\Test\ResetDatabase; @@ -76,9 +77,9 @@ private function executeHydrationTestCase(callable $testFactory, ?int $minPhpVer $metadataFactory = self::getContainer()->get('ux.live_component.metadata_factory'); \assert($metadataFactory instanceof LiveComponentMetadataFactory); $testCase = $testBuilder->getTest($metadataFactory); - - $componentAttributesFactory = self::getContainer()->get('ux.twig_component.component_attributes_factory'); - \assert($componentAttributesFactory instanceof ComponentAttributesFactory); + + $twig = self::getContainer()->get('twig'); + \assert($twig instanceof Environment); // keep a copy of the original, empty component object for hydration later $originalComponentWithData = clone $testCase->component; @@ -94,7 +95,7 @@ private function executeHydrationTestCase(callable $testFactory, ?int $minPhpVer $dehydratedProps = $this->hydrator()->dehydrate( $originalComponentWithData, - $componentAttributesFactory->create([]), // not worried about testing these here + new ComponentAttributes([], $twig->getRuntime(EscaperRuntime::class)), // not worried about testing these here $liveMetadata, ); @@ -136,7 +137,7 @@ private function executeHydrationTestCase(callable $testFactory, ?int $minPhpVer $dehydratedProps2 = $this->hydrator()->dehydrate( $componentAfterHydration, - $componentAttributesFactory->create(), + new ComponentAttributes([], $twig->getRuntime(EscaperRuntime::class)), $liveMetadata, ); $this->hydrator()->hydrate( @@ -1824,14 +1825,14 @@ public static function falseyValueProvider(): iterable yield ['nullableBool', '', null]; yield 'fooey-o-booey-todo' => ['nullableBool', ' ', null]; } - + private function createComponentAttributes(array $attributes = []): ComponentAttributes { - $factory = self::getContainer()->get('ux.twig_component.component_attributes_factory'); - \assert($factory instanceof ComponentAttributesFactory); - - return $factory->create($attributes); - } + $twig = self::getContainer()->get('twig'); + \assert($twig instanceof Environment); + + return new ComponentAttributes($attributes, $twig->getRuntime(EscaperRuntime::class)); + } private function createLiveMetadata(object $component): LiveComponentMetadata { diff --git a/src/LiveComponent/tests/Unit/LiveComponentHydratorTest.php b/src/LiveComponent/tests/Unit/LiveComponentHydratorTest.php index 5c053dfb2a3..2ddf1666dc0 100644 --- a/src/LiveComponent/tests/Unit/LiveComponentHydratorTest.php +++ b/src/LiveComponent/tests/Unit/LiveComponentHydratorTest.php @@ -20,7 +20,6 @@ use Symfony\UX\LiveComponent\LiveComponentHydrator; use Symfony\UX\LiveComponent\Metadata\LiveComponentMetadataFactory; use Symfony\UX\LiveComponent\Metadata\LivePropMetadata; -use Symfony\UX\TwigComponent\ComponentAttributesFactory; use Twig\Environment; final class LiveComponentHydratorTest extends TestCase @@ -36,7 +35,7 @@ public function testConstructWithEmptySecret(): void $this->createMock(LiveComponentMetadataFactory::class), $this->createMock(NormalizerInterface::class), '', - new ComponentAttributesFactory($this->createMock(Environment::class)), + $this->createMock(Environment::class), ); } @@ -48,7 +47,7 @@ public function testItCanHydrateWithNullValues() $this->createMock(LiveComponentMetadataFactory::class), new Serializer(normalizers: [new ObjectNormalizer()]), 'foo', - new ComponentAttributesFactory($this->createMock(Environment::class)), + $this->createMock(Environment::class), ); $hydratedValue = $hydrator->hydrateValue( diff --git a/src/TwigComponent/CHANGELOG.md b/src/TwigComponent/CHANGELOG.md index c97353192c7..770a5e15aab 100644 --- a/src/TwigComponent/CHANGELOG.md +++ b/src/TwigComponent/CHANGELOG.md @@ -1,11 +1,17 @@ # CHANGELOG +## 2.25.1 + +- [SECURITY] `ComponentAttributes` now requires a `Twig\Runtime\EscaperRuntime` + instance as second argument +- Remove `HtmlAttributeEscaperInterface`, `TwigHtmlAttributeEscaper` and `ComponentAttributesFactory` + ## 2.25.0 - [SECURITY] Make `ComponentAttributes` responsible for attribute escaping ensuring - consistent and secure HTML output across all rendering contexts. + consistent and secure HTML output across all rendering contexts - Deprecate not passing an `HtmlAttributeEscaperInterface` to the `ComponentAttributes` - constructor. + constructor ## 2.20.0 diff --git a/src/TwigComponent/src/ComponentAttributes.php b/src/TwigComponent/src/ComponentAttributes.php index c905547055a..140e39f8d74 100644 --- a/src/TwigComponent/src/ComponentAttributes.php +++ b/src/TwigComponent/src/ComponentAttributes.php @@ -12,8 +12,8 @@ namespace Symfony\UX\TwigComponent; use Symfony\UX\StimulusBundle\Dto\StimulusAttributes; -use Symfony\UX\TwigComponent\Escaper\HtmlAttributeEscaperInterface; use Symfony\WebpackEncoreBundle\Dto\AbstractStimulusDto; +use Twig\Runtime\EscaperRuntime; /** * @author Kevin Bond @@ -29,19 +29,13 @@ final class ComponentAttributes implements \Stringable, \IteratorAggregate, \Cou /** @var array */ private array $rendered = []; - private readonly ?HtmlAttributeEscaperInterface $escaper; - /** * @param array $attributes */ public function __construct( private array $attributes, - ?HtmlAttributeEscaperInterface $escaper = null, + private readonly EscaperRuntime $escaper, ) { - // Third argument used as internal flag to prevent multiple deprecations - if ((null === $this->escaper = $escaper) && 3 > func_num_args()) { - trigger_deprecation('symfony/ux-twig-component', '2.24', 'Not passing an "%s" to "%s" is deprecated and will throw in 3.0.', HtmlAttributeEscaperInterface::class, self::class); - } } public function __toString(): string @@ -87,13 +81,17 @@ public function __toString(): string // - special syntax names (Vue.js, Svelte, Alpine.js, ...) // v-*, x-*, @*, :* if (!ctype_alpha(str_replace(['-', '_', ':', '@', '.'], '', $key))) { - $key = $this->escaper?->escapeName($key) ?? $key; + $key = (string) $this->escaper->escape($key, 'html_attr'); } if (true === $value) { $attributes .= ' '.$key; } else { - $attributes .= ' '.\sprintf('%s="%s"', $key, $this->escaper?->escapeValue($value) ?? $value); + if (!ctype_alnum(str_replace(['-', '_'], '', $value))) { + $value = $this->escaper->escape($value, 'html'); + } + + $attributes .= ' '.\sprintf('%s="%s"', $key, $value); } } @@ -167,7 +165,7 @@ public function defaults(iterable $attributes): self unset($attributes[$attribute]); } - return new self($attributes, $this->escaper, true); + return new self($attributes, $this->escaper); } /** @@ -183,7 +181,7 @@ public function only(string ...$keys): self } } - return new self($attributes, $this->escaper, true); + return new self($attributes, $this->escaper); } /** @@ -221,7 +219,7 @@ public function add($stimulusDto): self ))); unset($controllersAttributes['data-controller']); - $clone = new self($attributes, $this->escaper, true); + $clone = new self($attributes, $this->escaper); // add the remaining attributes for values/classes return $clone->defaults($controllersAttributes); @@ -233,7 +231,7 @@ public function remove($key): self unset($attributes[$key]); - return new self($attributes, $this->escaper, true); + return new self($attributes, $this->escaper); } public function nested(string $namespace): self @@ -249,7 +247,7 @@ public function nested(string $namespace): self } } - return new self($attributes, $this->escaper, true); + return new self($attributes, $this->escaper); } public function getIterator(): \Traversable diff --git a/src/TwigComponent/src/ComponentAttributesFactory.php b/src/TwigComponent/src/ComponentAttributesFactory.php deleted file mode 100644 index 5238bf4fc7e..00000000000 --- a/src/TwigComponent/src/ComponentAttributesFactory.php +++ /dev/null @@ -1,42 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\UX\TwigComponent; - -use Symfony\UX\TwigComponent\Escaper\HtmlAttributeEscaperInterface; -use Symfony\UX\TwigComponent\Escaper\TwigHtmlAttributeEscaper; -use Twig\Environment; -use Twig\Runtime\EscaperRuntime; - -/** - * @author Simon André - * - * @internal - */ -final class ComponentAttributesFactory -{ - private readonly HtmlAttributeEscaperInterface $escaper; - - public function __construct( - private readonly Environment $twig, - ) { - } - - public function create(array $attributes = []): ComponentAttributes - { - return new ComponentAttributes($attributes, $this->getEscaper()); - } - - private function getEscaper(): HtmlAttributeEscaperInterface - { - return $this->escaper ??= new TwigHtmlAttributeEscaper($this->twig->getRuntime(EscaperRuntime::class)); - } -} diff --git a/src/TwigComponent/src/ComponentFactory.php b/src/TwigComponent/src/ComponentFactory.php index acfac114dc0..22619fb7842 100644 --- a/src/TwigComponent/src/ComponentFactory.php +++ b/src/TwigComponent/src/ComponentFactory.php @@ -17,6 +17,8 @@ use Symfony\Contracts\Service\ResetInterface; use Symfony\UX\TwigComponent\Event\PostMountEvent; use Symfony\UX\TwigComponent\Event\PreMountEvent; +use Twig\Environment; +use Twig\Runtime\EscaperRuntime; /** * @author Kevin Bond @@ -38,7 +40,7 @@ public function __construct( private EventDispatcherInterface $eventDispatcher, private array $config, private readonly array $classMap, - private ComponentAttributesFactory $componentAttributesFactory, + private readonly Environment $twig, ) { } @@ -120,7 +122,7 @@ public function mountFromObject(object $component, array $data, ComponentMetadat return new MountedComponent( $componentMetadata->getName(), $component, - $this->componentAttributesFactory->create([...$attributes, ...$data]), + new ComponentAttributes([...$attributes, ...$data], $this->twig->getRuntime(EscaperRuntime::class)), $originalData, $postMount->getExtraMetadata(), ); diff --git a/src/TwigComponent/src/DependencyInjection/TwigComponentExtension.php b/src/TwigComponent/src/DependencyInjection/TwigComponentExtension.php index 0eeeb6b4746..dc71c610931 100644 --- a/src/TwigComponent/src/DependencyInjection/TwigComponentExtension.php +++ b/src/TwigComponent/src/DependencyInjection/TwigComponentExtension.php @@ -30,7 +30,6 @@ use Symfony\UX\TwigComponent\Attribute\AsTwigComponent; use Symfony\UX\TwigComponent\CacheWarmer\TwigComponentCacheWarmer; use Symfony\UX\TwigComponent\Command\TwigComponentDebugCommand; -use Symfony\UX\TwigComponent\ComponentAttributesFactory; use Symfony\UX\TwigComponent\ComponentFactory; use Symfony\UX\TwigComponent\ComponentProperties; use Symfony\UX\TwigComponent\ComponentRenderer; @@ -93,7 +92,7 @@ static function (ChildDefinition $definition, AsTwigComponent $attribute) { new Reference('event_dispatcher'), new AbstractArgument(\sprintf('Added in %s.', TwigComponentPass::class)), new AbstractArgument(\sprintf('Added in %s.', TwigComponentPass::class)), - new Reference('ux.twig_component.component_attributes_factory'), + new Reference('twig'), ]) ->addTag('kernel.reset', ['method' => 'reset']) ; @@ -108,12 +107,6 @@ static function (ChildDefinition $definition, AsTwigComponent $attribute) { ]) ; - $container->register('ux.twig_component.component_attributes_factory', ComponentAttributesFactory::class) - ->setArguments([ - new Reference('twig'), - ]) - ; - $container->register('ux.twig_component.component_renderer', ComponentRenderer::class) ->setArguments([ new Reference('twig'), diff --git a/src/TwigComponent/src/Escaper/HtmlAttributeEscaperInterface.php b/src/TwigComponent/src/Escaper/HtmlAttributeEscaperInterface.php deleted file mode 100644 index 24525bb5bc6..00000000000 --- a/src/TwigComponent/src/Escaper/HtmlAttributeEscaperInterface.php +++ /dev/null @@ -1,33 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\UX\TwigComponent\Escaper; - -/** - * Escapes HTML attribute names and values. - * - * Implementations should escape attribute names and values according - * to the HTML specification. They can add additional escaping rules. - * - * @see https://html.spec.whatwg.org/multipage/syntax.html#attributes-2 - */ -interface HtmlAttributeEscaperInterface -{ - /** - * Escapes an HTML attribute name. - */ - public function escapeName(string $name): string; - - /** - * Escapes an HTML attribute value. - */ - public function escapeValue(string $value): string; -} diff --git a/src/TwigComponent/src/Escaper/TwigHtmlAttributeEscaper.php b/src/TwigComponent/src/Escaper/TwigHtmlAttributeEscaper.php deleted file mode 100644 index c8a2b0a383c..00000000000 --- a/src/TwigComponent/src/Escaper/TwigHtmlAttributeEscaper.php +++ /dev/null @@ -1,54 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\UX\TwigComponent\Escaper; - -use Symfony\UX\TwigComponent\Exception\RuntimeException; -use Twig\Runtime\EscaperRuntime; - -/** - * HTML attribute escaper using Twig EscaperRuntime. - * - * @internal - */ -final class TwigHtmlAttributeEscaper implements HtmlAttributeEscaperInterface -{ - public function __construct( - private readonly EscaperRuntime $escaper, - ) { - } - - public function escapeName(string $name): string - { - if (ctype_alpha(\str_replace(['-', '_'], '', $name))) { - return $name; - } - - try { - return $this->escaper->escape($name, 'html_attr'); - } catch (\Throwable $e) { - throw new RuntimeException(\sprintf('An error occurred while escaping the attribute name "%s".', $name), 0, $e); - } - } - - public function escapeValue(string $value): string - { - if (ctype_alnum(\str_replace(['-', '_',], '', $value))) { - return $value; - } - - try { - return $this->escaper->escape($value, 'html'); - } catch (\Throwable $e) { - throw new RuntimeException(\sprintf('An error occurred while escaping the attribute value "%s".', $value), 0, $e); - } - } -} diff --git a/src/TwigComponent/tests/Unit/ComponentAttributesTest.php b/src/TwigComponent/tests/Unit/ComponentAttributesTest.php index 54bb560a9d8..4e48ab850e8 100644 --- a/src/TwigComponent/tests/Unit/ComponentAttributesTest.php +++ b/src/TwigComponent/tests/Unit/ComponentAttributesTest.php @@ -15,10 +15,10 @@ use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\UX\StimulusBundle\Dto\StimulusAttributes; use Symfony\UX\TwigComponent\ComponentAttributes; -use Symfony\UX\TwigComponent\Escaper\HtmlAttributeEscaperInterface; use Symfony\WebpackEncoreBundle\Dto\AbstractStimulusDto; use Twig\Environment; use Twig\Loader\ArrayLoader; +use Twig\Runtime\EscaperRuntime; /** * @author Kevin Bond @@ -39,14 +39,14 @@ public function __toString(): string }, 'value' => '', 'autofocus' => true, - ], $this->createEscaper()); + ], new EscaperRuntime()); $this->assertSame(' class="foo" style="color:black;" value="" autofocus', (string) $attributes); } public function testCanSetDefaults(): void { - $attributes = new ComponentAttributes(['class' => 'foo', 'style' => 'color:black;'], $this->createEscaper()); + $attributes = new ComponentAttributes(['class' => 'foo', 'style' => 'color:black;'], new EscaperRuntime()); $this->assertSame( ['class' => 'bar foo', 'style' => 'color:black;'], @@ -57,19 +57,19 @@ public function testCanSetDefaults(): void (string) $attributes->defaults(['class' => 'bar', 'style' => 'font-size: 10;']) ); - $this->assertSame(['class' => 'foo'], (new ComponentAttributes([], $this->createEscaper()))->defaults(['class' => 'foo'])->all()); + $this->assertSame(['class' => 'foo'], (new ComponentAttributes([], new EscaperRuntime()))->defaults(['class' => 'foo'])->all()); } public function testCanGetOnly(): void { - $attributes = new ComponentAttributes(['class' => 'foo', 'style' => 'color:black;'], $this->createEscaper()); + $attributes = new ComponentAttributes(['class' => 'foo', 'style' => 'color:black;'], new EscaperRuntime()); $this->assertSame(['class' => 'foo'], $attributes->only('class')->all()); } public function testCanGetWithout(): void { - $attributes = new ComponentAttributes(['class' => 'foo', 'style' => 'color:black;'], $this->createEscaper()); + $attributes = new ComponentAttributes(['class' => 'foo', 'style' => 'color:black;'], new EscaperRuntime()); $this->assertSame(['class' => 'foo'], $attributes->without('style')->all()); } @@ -83,7 +83,7 @@ public function testCanAddStimulusController(): void 'class' => 'foo', 'data-controller' => 'live', 'data-live-data-value' => '{}', - ], $this->createEscaper()); + ], new EscaperRuntime()); $controllerDto = $this->createMock(AbstractStimulusDto::class); $controllerDto->expects(self::once()) @@ -110,7 +110,7 @@ public function testCanAddStimulusControllerIfNoneAlreadyPresent(): void { $attributes = new ComponentAttributes([ 'class' => 'foo', - ], $this->createEscaper()); + ], new EscaperRuntime()); $controllerDto = $this->createMock(AbstractStimulusDto::class); $controllerDto->expects(self::once()) @@ -140,7 +140,7 @@ public function testCanAddStimulusControllerViaStimulusAttributes(): void 'class' => 'foo', 'data-controller' => 'live', 'data-live-data-value' => '{}', - ], $this->createEscaper()); + ], new EscaperRuntime()); $stimulusAttributes = new StimulusAttributes(new Environment(new ArrayLoader())); $stimulusAttributes->addController('foo', ['name' => 'ryan', 'some_array' => ['a', 'b']]); @@ -165,7 +165,7 @@ public function testCanAddStimulusActionViaStimulusAttributes(): void $attributes = new ComponentAttributes([ 'class' => 'foo', 'data-action' => 'live#foo', - ], $this->createEscaper()); + ], new EscaperRuntime()); $stimulusAttributes = new StimulusAttributes(new Environment(new ArrayLoader())); $stimulusAttributes->addAction('foo', 'barMethod'); @@ -179,12 +179,12 @@ public function testCanAddStimulusActionViaStimulusAttributes(): void public function testBooleanBehaviour(): void { - $attributes = new ComponentAttributes(['disabled' => true], $this->createEscaper()); + $attributes = new ComponentAttributes(['disabled' => true], new EscaperRuntime()); $this->assertSame(['disabled' => true], $attributes->all()); $this->assertSame(' disabled', (string) $attributes); - $attributes = new ComponentAttributes(['disabled' => false], $this->createEscaper()); + $attributes = new ComponentAttributes(['disabled' => false], new EscaperRuntime()); $this->assertSame(['disabled' => false], $attributes->all()); $this->assertSame('', (string) $attributes); @@ -195,7 +195,7 @@ public function testBooleanBehaviour(): void */ public function testNullBehaviour(): void { - $attributes = new ComponentAttributes(['disabled' => null], $this->createEscaper()); + $attributes = new ComponentAttributes(['disabled' => null], new EscaperRuntime()); $this->assertSame(['disabled' => null], $attributes->all()); $this->assertSame(' disabled', (string) $attributes); @@ -203,7 +203,7 @@ public function testNullBehaviour(): void public function testIsTraversableAndCountable(): void { - $attributes = new ComponentAttributes(['foo' => 'bar'], $this->createEscaper()); + $attributes = new ComponentAttributes(['foo' => 'bar'], new EscaperRuntime()); $this->assertSame($attributes->all(), iterator_to_array($attributes)); $this->assertCount(1, $attributes); @@ -211,7 +211,7 @@ public function testIsTraversableAndCountable(): void public function testRenderSingleAttribute(): void { - $attributes = new ComponentAttributes(['attr1' => 'value1', 'attr2' => 'value2'], $this->createEscaper()); + $attributes = new ComponentAttributes(['attr1' => 'value1', 'attr2' => 'value2'], new EscaperRuntime()); $this->assertSame('value1', $attributes->render('attr1')); $this->assertNull($attributes->render('attr3')); @@ -227,7 +227,7 @@ public function __toString(): string } }, 'attr2' => 'value2', - ], $this->createEscaper()); + ], new EscaperRuntime()); $this->assertSame('value1', $attributes->render('attr1')); $this->assertSame(' attr2="value2"', (string) $attributes); @@ -235,7 +235,7 @@ public function __toString(): string public function testCannotRenderNonStringAttribute(): void { - $attributes = new ComponentAttributes(['attr1' => false], $this->createEscaper()); + $attributes = new ComponentAttributes(['attr1' => false], new EscaperRuntime()); $this->expectException(\LogicException::class); @@ -244,7 +244,7 @@ public function testCannotRenderNonStringAttribute(): void public function testCanCheckIfAttributeExists(): void { - $attributes = new ComponentAttributes(['foo' => 'bar'], $this->createEscaper()); + $attributes = new ComponentAttributes(['foo' => 'bar'], new EscaperRuntime()); $this->assertTrue($attributes->has('foo')); } @@ -255,7 +255,7 @@ public function testNestedAttributes(): void 'class' => 'foo', 'title:class' => 'bar', 'title:span:class' => 'baz', - ], $this->createEscaper()); + ], new EscaperRuntime()); $this->assertSame(' class="foo"', (string) $attributes); $this->assertSame(' class="bar"', (string) $attributes->nested('title')); @@ -268,7 +268,7 @@ public function testPrefixedAttributes(): void $attributes = new ComponentAttributes([ 'x-click' => 'x+', 'title:x-click' => 'title:x+', - ], $this->createEscaper()); + ], new EscaperRuntime()); $this->assertSame(' x-click="x+"', (string) $attributes); $this->assertSame(' x-click="title:x+"', (string) $attributes->nested('title')); @@ -285,7 +285,7 @@ public function testConvertTrueAriaAttributeValue(): void 'aria-false' => 'false', 'aria-foobar' => 'foobar', 'aria-number' => '1', - ], $this->createEscaper()); + ], new EscaperRuntime()); $this->assertStringNotContainsString('aria-bar', (string) $attributes); $this->assertStringContainsString('aria-foo="true"', (string) $attributes); @@ -309,7 +309,7 @@ public function testConvertTrueAriaAttributeValue(): void */ public function testAllowsSpecialSyntaxAttributeNames(string $name): void { - $attributes = new ComponentAttributes([$name => 'value'], $this->createEscaper()); + $attributes = new ComponentAttributes([$name => 'value'], new EscaperRuntime()); $this->assertSame(' '.$name.'="value"', (string) $attributes); } @@ -323,48 +323,68 @@ public static function provideSpecialSyntaxAttributeNames(): iterable yield ['x-on:click']; } - public function testTransmitsEscaper(): void + public function testThrowsTypeErrorWithoutEscaperRuntime(): void { - $escaper = new class implements HtmlAttributeEscaperInterface { - public function escapeName(string $name, string $charset = 'UTF-8'): string - { - return 'N('.$name.')'; - } - - public function escapeValue(string $value, string $charset = 'UTF-8'): string - { - return 'V('.$value.')'; - } - }; - $attributes = new ComponentAttributes(['f"oo' => 'b"ar', 'ke"y' => true], $escaper); - - $this->assertSame('N(f"oo)="V(b"ar)" N(ke"y)', trim($attributes)); - $this->assertSame('N(f"oo)="V(b"ar)" N(ke"y)', trim($attributes->defaults([]))); - $this->assertSame('N(ke"y)', trim($attributes->without('f"oo'))); - $this->assertSame('N(f"oo)="V(b"ar)"', trim($attributes->only('f"oo'))); + $this->expectException(\TypeError::class); + new ComponentAttributes([]); } /** - * @group legacy + * @dataProvider nameProvider */ - public function testTriggersDeprecationWithoutEscaper(): void + public function testEscapeName(string $input, string $expected): void { - $this->expectDeprecation('Since symfony/ux-twig-component 2.24: Not passing an "Symfony\UX\TwigComponent\Escaper\HtmlAttributeEscaperInterface" to "Symfony\UX\TwigComponent\ComponentAttributes" is deprecated and will throw in 3.0.'); - new ComponentAttributes([]); + $runtime = new EscaperRuntime(); + $attributes = new ComponentAttributes([$input => 'foo'], $runtime); + + $this->assertSame(' '.$expected.'="foo"', (string) $attributes); + } + + /** + * @dataProvider valueProvider + */ + public function testEscapeValue(string $input, string $expected): void + { + $runtime = new EscaperRuntime(); + $attributes = new ComponentAttributes(['foo' => $input], $runtime); + + $this->assertSame(' foo="'.$expected.'"', (string) $attributes); + } + + public static function nameProvider(): iterable + { + // Should not escape + yield 'basic' => ['class', 'class']; + yield 'data-' => ['data-user', 'data-user']; + yield 'aria' => ['aria-label', 'aria-label']; + yield 'alnum' => ['attr123', 'attr123']; + // Should escape + yield 'double quote' => ['"', '"']; + yield 'ampersand' => ['&', '&']; + yield 'less than' => ['<', '<']; + yield 'greater than' => ['>', '>']; + // Twig strict escaping + yield 'scripts' => ['>', '<script>alert(1)</script>']; + yield 'inline xss' => ['', '<img src=x onerror=alert(1)>']; + yield 'malicious attr' => ['foo="bar"', 'foo="bar"']; + yield 'sql injection' => ["' OR 1=1 --", '' OR 1=1 --']; + yield 'url encoded xss' => ['%3Cscript%3Ealert(1)%3C/script%3E', '%3Cscript%3Ealert(1)%3C/script%3E']; } } diff --git a/src/TwigComponent/tests/Unit/ComponentFactoryTest.php b/src/TwigComponent/tests/Unit/ComponentFactoryTest.php index b848dfef878..b842b7e9ae0 100644 --- a/src/TwigComponent/tests/Unit/ComponentFactoryTest.php +++ b/src/TwigComponent/tests/Unit/ComponentFactoryTest.php @@ -15,7 +15,6 @@ use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; -use Symfony\UX\TwigComponent\ComponentAttributesFactory; use Symfony\UX\TwigComponent\ComponentFactory; use Symfony\UX\TwigComponent\ComponentTemplateFinderInterface; use Twig\Environment; @@ -34,7 +33,7 @@ public function testMetadataForConfig(): void $this->createMock(EventDispatcherInterface::class), ['foo' => ['key' => 'foo', 'template' => 'bar.html.twig']], [], - new ComponentAttributesFactory($this->createMock(Environment::class)), + $this->createMock(Environment::class), ); $metadata = $factory->metadataFor('foo'); @@ -55,7 +54,7 @@ public function testMetadataForResolveAlias(): void 'foo' => ['key' => 'foo', 'template' => 'foo.html.twig'], ], ['Foo\\Bar' => 'bar'], - new ComponentAttributesFactory($this->createMock(Environment::class)), + $this->createMock(Environment::class), ); $metadata = $factory->metadataFor('Foo\\Bar'); @@ -79,7 +78,7 @@ public function testMetadataForReuseAnonymousConfig(): void $this->createMock(EventDispatcherInterface::class), [], [], - new ComponentAttributesFactory($this->createMock(Environment::class)), + $this->createMock(Environment::class), ); $metadata = $factory->metadataFor('foo'); diff --git a/src/TwigComponent/tests/Unit/ComponentStackTest.php b/src/TwigComponent/tests/Unit/ComponentStackTest.php index 5b7bb5b94ba..4ef48e40ea5 100644 --- a/src/TwigComponent/tests/Unit/ComponentStackTest.php +++ b/src/TwigComponent/tests/Unit/ComponentStackTest.php @@ -14,16 +14,16 @@ use PHPUnit\Framework\TestCase; use Symfony\UX\TwigComponent\ComponentAttributes; use Symfony\UX\TwigComponent\ComponentStack; -use Symfony\UX\TwigComponent\Escaper\HtmlAttributeEscaperInterface; use Symfony\UX\TwigComponent\MountedComponent; +use Twig\Runtime\EscaperRuntime; final class ComponentStackTest extends TestCase { public function testPushAndPopAndFetchingComponents(): void { $stack = new ComponentStack(); - $component1 = new MountedComponent('component1', new \stdClass(), new ComponentAttributes([], $this->createMock(HtmlAttributeEscaperInterface::class))); - $component2 = new MountedComponent('component2', new \stdClass(), new ComponentAttributes([], $this->createMock(HtmlAttributeEscaperInterface::class))); + $component1 = new MountedComponent('component1', new \stdClass(), new ComponentAttributes([], new EscaperRuntime())); + $component2 = new MountedComponent('component2', new \stdClass(), new ComponentAttributes([], new EscaperRuntime())); $this->assertNull($stack->pop()); $this->assertNull($stack->getCurrentComponent()); diff --git a/src/TwigComponent/tests/Unit/Escaper/TwigHtmlAttributeEscaperTest.php b/src/TwigComponent/tests/Unit/Escaper/TwigHtmlAttributeEscaperTest.php deleted file mode 100644 index c350fd708da..00000000000 --- a/src/TwigComponent/tests/Unit/Escaper/TwigHtmlAttributeEscaperTest.php +++ /dev/null @@ -1,94 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\UX\TwigComponent\Tests\Unit\Escaper; - -use PHPUnit\Framework\TestCase; -use Symfony\UX\TwigComponent\Escaper\TwigHtmlAttributeEscaper; -use Twig\Runtime\EscaperRuntime; - -class TwigHtmlAttributeEscaperTest extends TestCase -{ - /** - * @dataProvider nameProvider - */ - public function testEscapeName(string $input, string $expected): void - { - $escaper = new TwigHtmlAttributeEscaper(new EscaperRuntime()); - $this->assertSame($expected, $escaper->escapeName($input)); - } - - /** - * @dataProvider nameProvider - */ - public function testEscapeNameReturnSameAsTwig(string $input, string $expected): void - { - $runtime = new EscaperRuntime(); - $escaper = new TwigHtmlAttributeEscaper($runtime); - $this->assertSame($runtime->escape($input, 'html_attr'), $escaper->escapeName($input)); - } - - /** - * @dataProvider valueProvider - */ - public function testEscapeValue(string $input, string $expected): void - { - $escaper = new TwigHtmlAttributeEscaper(new EscaperRuntime()); - $this->assertSame($expected, $escaper->escapeValue($input)); - } - - /** - * @dataProvider valueProvider - */ - public function testEscapeValueReturnSameAsTwig(string $input): void - { - $runtime = new EscaperRuntime(); - $escaper = new TwigHtmlAttributeEscaper($runtime); - $this->assertSame($runtime->escape($input, 'html'), $escaper->escapeValue($input)); - } - - public static function nameProvider(): iterable - { - // Should not escape - yield 'basic' => ['class', 'class']; - yield 'data-' => ['data-user', 'data-user']; - yield 'aria' => ['aria-label', 'aria-label']; - yield 'alnum' => ['attr123', 'attr123']; - // Should escape - yield 'double quote' => ['"', '"']; - yield 'ampersand' => ['&', '&']; - yield 'less than' => ['<', '<']; - yield 'greater than' => ['>', '>']; - // Twig strict escaping - yield 'scripts' => ['>', '<script>alert(1)</script>']; - yield 'inline xss' => ['', '<img src=x onerror=alert(1)>']; - yield 'malicious attr' => ['foo="bar"', 'foo="bar"']; - yield 'sql injection' => ["' OR 1=1 --", '' OR 1=1 --']; - yield 'url encoded xss' => ['%3Cscript%3Ealert(1)%3C/script%3E', '%3Cscript%3Ealert(1)%3C/script%3E']; - } -} diff --git a/src/TwigComponent/tests/Unit/EventListener/TwigComponentLoggerListenerTest.php b/src/TwigComponent/tests/Unit/EventListener/TwigComponentLoggerListenerTest.php index 7b683327e55..3a02709b21f 100644 --- a/src/TwigComponent/tests/Unit/EventListener/TwigComponentLoggerListenerTest.php +++ b/src/TwigComponent/tests/Unit/EventListener/TwigComponentLoggerListenerTest.php @@ -14,11 +14,11 @@ use PHPUnit\Framework\TestCase; use Symfony\UX\TwigComponent\ComponentAttributes; use Symfony\UX\TwigComponent\ComponentMetadata; -use Symfony\UX\TwigComponent\Escaper\HtmlAttributeEscaperInterface; use Symfony\UX\TwigComponent\Event\PostRenderEvent; use Symfony\UX\TwigComponent\Event\PreRenderEvent; use Symfony\UX\TwigComponent\EventListener\TwigComponentLoggerListener; use Symfony\UX\TwigComponent\MountedComponent; +use Twig\Runtime\EscaperRuntime; /** * @author Simon André @@ -29,9 +29,8 @@ public function testLoggerStoreEvents(): void { $logger = new TwigComponentLoggerListener(); $this->assertSame([], $logger->getEvents()); - $escaper = $this->createHtmlAttributeEscaper(); - $mounted = new MountedComponent('foo', new \stdClass(), new ComponentAttributes([], $escaper)); + $mounted = new MountedComponent('foo', new \stdClass(), new ComponentAttributes([], new EscaperRuntime())); $eventA = new PreRenderEvent($mounted, new ComponentMetadata(['template' => 'bar']), []); $logger->onPreRender($eventA); $eventB = new PostRenderEvent($mounted); @@ -43,7 +42,7 @@ public function testLoggerStoreEvents(): void public function testLoggerReset(): void { $logger = new TwigComponentLoggerListener(); - $escaper = $this->createHtmlAttributeEscaper(); + $escaper = new EscaperRuntime(); $logger->onPreRender(new PreRenderEvent(new MountedComponent('foo', new \stdClass(), new ComponentAttributes([], $escaper)), new ComponentMetadata(['template' => 'bar']), [])); $this->assertNotSame([], $logger->getEvents()); @@ -51,19 +50,4 @@ public function testLoggerReset(): void $logger->reset(); $this->assertSame([], $logger->getEvents()); } - - private function createHtmlAttributeEscaper(): HtmlAttributeEscaperInterface - { - return new class implements HtmlAttributeEscaperInterface { - public function escapeName(string $name, string $charset = 'UTF-8'): string - { - return $name; - } - - public function escapeValue(string $value, string $charset = 'UTF-8'): string - { - return $value; - } - }; - } }