From e0a9339b7e6abcc6142b0e6a0992db88260030dc Mon Sep 17 00:00:00 2001 From: Titouan Galopin Date: Sun, 26 Dec 2021 17:11:01 +0100 Subject: [PATCH 1/2] Integrate the HtmlSanitizer component to the FrameworkBundle --- .../DependencyInjection/Configuration.php | 121 ++++++++++++++++++ .../FrameworkExtension.php | 86 +++++++++++++ .../Resources/config/html_sanitizer.php | 25 ++++ .../Resources/config/schema/symfony-1.0.xsd | 60 +++++++++ .../DependencyInjection/Fixtures/php/full.php | 48 +++++++ .../Fixtures/php/html_sanitizer_default.php | 7 + .../DependencyInjection/Fixtures/xml/full.xml | 9 ++ .../FrameworkExtensionTest.php | 108 ++++++++++++++++ .../Bundle/FrameworkBundle/composer.json | 1 + 9 files changed, 465 insertions(+) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Resources/config/html_sanitizer.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer_default.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 5511672345465..ba8137b3ccdf8 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -25,6 +25,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Exception\LogicException; use Symfony\Component\Form\Form; +use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface; use Symfony\Component\HttpClient\HttpClient; use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\Lock\Lock; @@ -167,6 +168,7 @@ public function getConfigTreeBuilder(): TreeBuilder $this->addNotifierSection($rootNode, $enableIfStandalone); $this->addRateLimiterSection($rootNode, $enableIfStandalone); $this->addUidSection($rootNode, $enableIfStandalone); + $this->addHtmlSanitizerSection($rootNode, $enableIfStandalone); return $treeBuilder; } @@ -2106,4 +2108,123 @@ private function addUidSection(ArrayNodeDefinition $rootNode, callable $enableIf ->end() ; } + + private function addHtmlSanitizerSection(ArrayNodeDefinition $rootNode, callable $enableIfStandalone) + { + $rootNode + ->children() + ->arrayNode('html_sanitizer') + ->info('HtmlSanitizer configuration') + ->{$enableIfStandalone('symfony/html-sanitizer', HtmlSanitizerInterface::class)}() + ->fixXmlConfig('sanitizer') + ->children() + ->scalarNode('default') + ->defaultNull() + ->info('Default sanitizer to use when injecting without named binding.') + ->end() + ->arrayNode('sanitizers') + ->normalizeKeys(false) + ->useAttributeAsKey('name') + ->arrayPrototype() + ->children() + ->booleanNode('allow_safe_elements') + ->info('Allows "safe" elements and attributes.') + ->defaultFalse() + ->end() + ->booleanNode('allow_all_static_elements') + ->info('Allows all static elements and attributes from the W3C Sanitizer API standard.') + ->defaultFalse() + ->end() + ->arrayNode('allow_elements') + ->info('Configures elements as allowed. Allowed elements are elements the sanitizer should retain from the input.') + ->fixXmlConfig('allow-element') + ->normalizeKeys(false) + ->useAttributeAsKey('name') + ->variablePrototype()->end() + ->end() + ->arrayNode('block_elements') + ->info('Configures elements as blocked. Blocked elements are elements the sanitizer should remove from the input, but retain their children.') + ->fixXmlConfig('block-element') + ->scalarPrototype()->end() + ->end() + ->arrayNode('drop_elements') + ->info('Configures elements as dropped. Dropped elements are elements the sanitizer should remove from the input, including their children.') + ->fixXmlConfig('drop-element') + ->scalarPrototype()->end() + ->end() + ->arrayNode('allow_attributes') + ->info('Configures attributes as allowed. Allowed attributes are attributes the sanitizer should retain from the input.') + ->fixXmlConfig('allow-attribute') + ->normalizeKeys(false) + ->useAttributeAsKey('name') + ->variablePrototype()->end() + ->end() + ->arrayNode('drop_attributes') + ->info('Configures attributes as dropped. Dropped attributes are attributes the sanitizer should remove from the input.') + ->fixXmlConfig('drop-attribute') + ->normalizeKeys(false) + ->useAttributeAsKey('name') + ->variablePrototype()->end() + ->end() + ->arrayNode('force_attributes') + ->info('Forcefully set the values of certain attributes on certain elements.') + ->fixXmlConfig('force-attribute') + ->normalizeKeys(false) + ->useAttributeAsKey('name') + ->arrayPrototype() + ->normalizeKeys(false) + ->useAttributeAsKey('name') + ->scalarPrototype()->end() + ->end() + ->end() + ->booleanNode('force_https_urls') + ->info('Transforms URLs using the HTTP scheme to use the HTTPS scheme instead.') + ->defaultFalse() + ->end() + ->arrayNode('allowed_link_schemes') + ->info('Allows only a given list of schemes to be used in links href attributes.') + ->fixXmlConfig('allow-link-scheme') + ->scalarPrototype()->end() + ->end() + ->arrayNode('allowed_link_hosts') + ->info('Allows only a given list of hosts to be used in links href attributes.') + ->fixXmlConfig('allow-link-host') + ->scalarPrototype()->end() + ->end() + ->booleanNode('allow_relative_links') + ->info('Allows relative URLs to be used in links href attributes.') + ->defaultFalse() + ->end() + ->arrayNode('allowed_media_schemes') + ->info('Allows only a given list of schemes to be used in media source attributes (img, audio, video, ...).') + ->fixXmlConfig('allow-media-scheme') + ->scalarPrototype()->end() + ->end() + ->arrayNode('allowed_media_hosts') + ->info('Allows only a given list of hosts to be used in media source attributes (img, audio, video, ...).') + ->fixXmlConfig('allow-media-host') + ->scalarPrototype()->end() + ->end() + ->booleanNode('allow_relative_medias') + ->info('Allows relative URLs to be used in media source attributes (img, audio, video, ...).') + ->defaultFalse() + ->end() + ->arrayNode('with_attribute_sanitizers') + ->info('Registers custom attribute sanitizers.') + ->fixXmlConfig('with-attribute-sanitizer') + ->scalarPrototype()->end() + ->end() + ->arrayNode('without_attribute_sanitizers') + ->info('Unregisters custom attribute sanitizers.') + ->fixXmlConfig('without-attribute-sanitizer') + ->scalarPrototype()->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ->end() + ; + } } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index aeed63946b25a..af64c9bc807db 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -73,6 +73,9 @@ use Symfony\Component\Form\FormTypeExtensionInterface; use Symfony\Component\Form\FormTypeGuesserInterface; use Symfony\Component\Form\FormTypeInterface; +use Symfony\Component\HtmlSanitizer\HtmlSanitizer; +use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig; +use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\Retry\GenericRetryStrategy; use Symfony\Component\HttpClient\RetryableHttpClient; @@ -531,6 +534,14 @@ public function load(array $configs, ContainerBuilder $container) // profiler depends on form, validation, translation, messenger, mailer, http-client, notifier, serializer being registered $this->registerProfilerConfiguration($config['profiler'], $container, $loader); + if ($this->isConfigEnabled($container, $config['html_sanitizer'])) { + if (!class_exists(HtmlSanitizerConfig::class)) { + throw new LogicException('HtmlSanitizer support cannot be enabled as the HtmlSanitizer component is not installed. Try running "composer require symfony/html-sanitizer".'); + } + + $this->registerHtmlSanitizerConfiguration($config['html_sanitizer'], $container, $loader); + } + $this->addAnnotatedClassesToCompile([ '**\\Controller\\', '**\\Entity\\', @@ -2659,6 +2670,81 @@ private function registerUidConfiguration(array $config, ContainerBuilder $conta } } + private function registerHtmlSanitizerConfiguration(array $config, ContainerBuilder $container, PhpFileLoader $loader) + { + $loader->load('html_sanitizer.php'); + + foreach ($config['sanitizers'] as $sanitizerName => $sanitizerConfig) { + $configId = 'html_sanitizer.config.'.$sanitizerName; + $def = $container->register($configId, HtmlSanitizerConfig::class); + + // Base + if ($sanitizerConfig['allow_safe_elements']) { + $def->addMethodCall('allowSafeElements', [], true); + } + + if ($sanitizerConfig['allow_all_static_elements']) { + $def->addMethodCall('allowAllStaticElements', [], true); + } + + // Configures elements + foreach ($sanitizerConfig['allow_elements'] as $element => $attributes) { + $def->addMethodCall('allowElement', [$element, $attributes], true); + } + + foreach ($sanitizerConfig['block_elements'] as $element) { + $def->addMethodCall('blockElement', [$element], true); + } + + foreach ($sanitizerConfig['drop_elements'] as $element) { + $def->addMethodCall('dropElement', [$element], true); + } + + // Configures attributes + foreach ($sanitizerConfig['allow_attributes'] as $attribute => $elements) { + $def->addMethodCall('allowAttribute', [$attribute, $elements], true); + } + + foreach ($sanitizerConfig['drop_attributes'] as $attribute => $elements) { + $def->addMethodCall('dropAttribute', [$attribute, $elements], true); + } + + // Force attributes + foreach ($sanitizerConfig['force_attributes'] as $element => $attributes) { + foreach ($attributes as $attrName => $attrValue) { + $def->addMethodCall('forceAttribute', [$element, $attrName, $attrValue], true); + } + } + + // Settings + $def->addMethodCall('forceHttpsUrls', [$sanitizerConfig['force_https_urls']], true); + $def->addMethodCall('allowLinkSchemes', [$sanitizerConfig['allowed_link_schemes']], true); + $def->addMethodCall('allowLinkHosts', [$sanitizerConfig['allowed_link_hosts']], true); + $def->addMethodCall('allowRelativeLinks', [$sanitizerConfig['allow_relative_links']], true); + $def->addMethodCall('allowMediaSchemes', [$sanitizerConfig['allowed_media_schemes']], true); + $def->addMethodCall('allowMediaHosts', [$sanitizerConfig['allowed_media_hosts']], true); + $def->addMethodCall('allowRelativeMedias', [$sanitizerConfig['allow_relative_medias']], true); + + // Custom attribute sanitizers + foreach ($sanitizerConfig['with_attribute_sanitizers'] as $serviceName) { + $def->addMethodCall('withAttributeSanitizer', [new Reference($serviceName)], true); + } + + foreach ($sanitizerConfig['without_attribute_sanitizers'] as $serviceName) { + $def->addMethodCall('withoutAttributeSanitizer', [new Reference($serviceName)], true); + } + + // Create the sanitizer and link its config + $sanitizerId = 'html_sanitizer.sanitizer.'.$sanitizerName; + $container->register($sanitizerId, HtmlSanitizer::class)->addArgument(new Reference($configId)); + + $container->registerAliasForArgument($sanitizerId, HtmlSanitizerInterface::class, $sanitizerName); + } + + $default = $config['default'] ? 'html_sanitizer.sanitizer.'.$config['default'] : 'html_sanitizer'; + $container->setAlias(HtmlSanitizerInterface::class, new Reference($default)); + } + private function resolveTrustedHeaders(array $headers): int { $trustedHeaders = 0; diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/html_sanitizer.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/html_sanitizer.php new file mode 100644 index 0000000000000..558188d18915f --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/html_sanitizer.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Loader\Configurator; + +use Symfony\Component\HtmlSanitizer\HtmlSanitizer; +use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig; + +return static function (ContainerConfigurator $container) { + $container->services() + ->set('html_sanitizer.config', HtmlSanitizerConfig::class) + ->call('allowSafeElements') + + ->set('html_sanitizer', HtmlSanitizer::class) + ->args([service('html_sanitizer.config')]) + ; +}; diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index f932e596b9b7e..49765a8f56474 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -39,6 +39,7 @@ + @@ -819,4 +820,63 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php index 113324790d7ad..1aff4528aadcf 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php @@ -80,4 +80,52 @@ 'pdf' => 'application/pdf', ], ], + 'html_sanitizer' => [ + 'default' => 'my.sanitizer', + 'sanitizers' => [ + 'my.sanitizer' => [ + 'allow_safe_elements' => true, + 'allow_all_static_elements' => true, + 'allow_elements' => [ + 'custom-tag-1' => ['data-attr-1'], + 'custom-tag-2' => [], + 'custom-tag-3' => '*', + ], + 'block_elements' => [ + 'custom-tag-4', + ], + 'drop_elements' => [ + 'custom-tag-5', + ], + 'allow_attributes' => [ + 'data-attr-2' => ['custom-tag-6'], + 'data-attr-3' => [], + 'data-attr-4' => '*', + ], + 'drop_attributes' => [ + 'data-attr-5' => ['custom-tag-6'], + 'data-attr-6' => [], + 'data-attr-7' => '*', + ], + 'force_attributes' => [ + 'custom-tag-7' => [ + 'data-attr-8' => 'value', + ], + ], + 'force_https_urls' => true, + 'allowed_link_schemes' => ['http', 'https', 'mailto'], + 'allowed_link_hosts' => ['symfony.com'], + 'allow_relative_links' => true, + 'allowed_media_schemes' => ['http', 'https', 'data'], + 'allowed_media_hosts' => ['symfony.com'], + 'allow_relative_medias' => true, + 'with_attribute_sanitizers' => [ + 'App\\Sanitizer\\CustomAttributeSanitizer', + ], + 'without_attribute_sanitizers' => [ + 'App\\Sanitizer\\OtherCustomAttributeSanitizer', + ], + ], + ], + ], ]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer_default.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer_default.php new file mode 100644 index 0000000000000..8be1095e0868d --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer_default.php @@ -0,0 +1,7 @@ +loadFromExtension('framework', [ + 'html_sanitizer' => [ + 'enabled' => true, + ], +]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml index 00da2a6c0f963..bf08c25c1d644 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml @@ -39,5 +39,14 @@ + + + text/csv + text/plain + + + application/pdf + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index ba19f50419ede..69296d461e734 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -47,6 +47,9 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\Finder\Finder; use Symfony\Component\Form\Form; +use Symfony\Component\HtmlSanitizer\HtmlSanitizer; +use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig; +use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\RetryableHttpClient; use Symfony\Component\HttpClient\ScopingHttpClient; @@ -2027,6 +2030,111 @@ public function testLocaleSwitcherServiceRegistered() $this->assertEquals(new Reference('router.request_context', ContainerBuilder::IGNORE_ON_INVALID_REFERENCE), $switcherDef->getArgument(2)); } + public function testHtmlSanitizerDefault() + { + $container = $this->createContainerFromFile('html_sanitizer_default'); + + // html_sanitizer service + $this->assertTrue($container->hasDefinition('html_sanitizer'), '->registerHtmlSanitizerConfiguration() loads html_sanitizer.php'); + $this->assertSame(HtmlSanitizer::class, $container->getDefinition('html_sanitizer')->getClass()); + $this->assertCount(1, $args = $container->getDefinition('html_sanitizer')->getArguments()); + $this->assertSame('html_sanitizer.config', (string) $args[0]); + + // html_sanitizer.config service + $this->assertTrue($container->hasDefinition('html_sanitizer.config'), '->registerHtmlSanitizerConfiguration() loads html_sanitizer.php'); + $this->assertSame(HtmlSanitizerConfig::class, $container->getDefinition('html_sanitizer.config')->getClass()); + $this->assertCount(1, $calls = $container->getDefinition('html_sanitizer.config')->getMethodCalls()); + $this->assertSame(['allowSafeElements', []], $calls[0]); + + // Default alias + $this->assertSame( + 'html_sanitizer', + (string) $container->getAlias(HtmlSanitizerInterface::class), + '->registerHtmlSanitizerConfiguration() creates appropriate default alias' + ); + } + + public function testHtmlSanitizerFull() + { + $container = $this->createContainerFromFile('full'); + + // html_sanitizer service + $this->assertTrue($container->hasDefinition('html_sanitizer'), '->registerHtmlSanitizerConfiguration() loads html_sanitizer.php'); + $this->assertSame(HtmlSanitizer::class, $container->getDefinition('html_sanitizer')->getClass()); + $this->assertCount(1, $args = $container->getDefinition('html_sanitizer')->getArguments()); + $this->assertSame('html_sanitizer.config', (string) $args[0]); + + // html_sanitizer.config service + $this->assertTrue($container->hasDefinition('html_sanitizer.config'), '->registerHtmlSanitizerConfiguration() loads html_sanitizer.php'); + $this->assertSame(HtmlSanitizerConfig::class, $container->getDefinition('html_sanitizer.config')->getClass()); + $this->assertCount(1, $calls = $container->getDefinition('html_sanitizer.config')->getMethodCalls()); + $this->assertSame(['allowSafeElements', []], $calls[0]); + + // my.sanitizer + $this->assertTrue($container->hasDefinition('html_sanitizer.sanitizer.my.sanitizer'), '->registerHtmlSanitizerConfiguration() loads custom sanitizer'); + $this->assertSame(HtmlSanitizer::class, $container->getDefinition('html_sanitizer.sanitizer.my.sanitizer')->getClass()); + $this->assertCount(1, $args = $container->getDefinition('html_sanitizer.sanitizer.my.sanitizer')->getArguments()); + $this->assertSame('html_sanitizer.config.my.sanitizer', (string) $args[0]); + + // my.sanitizer config + $this->assertTrue($container->hasDefinition('html_sanitizer.config.my.sanitizer'), '->registerHtmlSanitizerConfiguration() loads custom sanitizer'); + $this->assertSame(HtmlSanitizerConfig::class, $container->getDefinition('html_sanitizer.config.my.sanitizer')->getClass()); + $this->assertCount(23, $calls = $container->getDefinition('html_sanitizer.config.my.sanitizer')->getMethodCalls()); + $this->assertSame( + [ + ['allowSafeElements', [], true], + ['allowAllStaticElements', [], true], + ['allowElement', ['custom-tag-1', ['data-attr-1']], true], + ['allowElement', ['custom-tag-2', []], true], + ['allowElement', ['custom-tag-3', '*'], true], + ['blockElement', ['custom-tag-4'], true], + ['dropElement', ['custom-tag-5'], true], + ['allowAttribute', ['data-attr-2', ['custom-tag-6']], true], + ['allowAttribute', ['data-attr-3', []], true], + ['allowAttribute', ['data-attr-4', '*'], true], + ['dropAttribute', ['data-attr-5', ['custom-tag-6']], true], + ['dropAttribute', ['data-attr-6', []], true], + ['dropAttribute', ['data-attr-7', '*'], true], + ['forceAttribute', ['custom-tag-7', 'data-attr-8', 'value'], true], + ['forceHttpsUrls', [true], true], + ['allowLinkSchemes', [['http', 'https', 'mailto']], true], + ['allowLinkHosts', [['symfony.com']], true], + ['allowRelativeLinks', [true], true], + ['allowMediaSchemes', [['http', 'https', 'data']], true], + ['allowMediaHosts', [['symfony.com']], true], + ['allowRelativeMedias', [true], true], + ['withAttributeSanitizer', ['@App\\Sanitizer\\CustomAttributeSanitizer'], true], + ['withoutAttributeSanitizer', ['@App\\Sanitizer\\OtherCustomAttributeSanitizer'], true], + ], + + // Convert references to their names for easier assertion + array_map( + static function ($call) { + foreach ($call[1] as $k => $arg) { + $call[1][$k] = $arg instanceof Reference ? '@'.$arg : $arg; + } + + return $call; + }, + $calls + ) + ); + + // Named alias + $this->assertSame( + 'html_sanitizer.sanitizer.my.sanitizer', + (string) $container->getAlias(HtmlSanitizerInterface::class.' $mySanitizer'), + '->registerHtmlSanitizerConfiguration() creates appropriate named alias' + ); + + // Default alias + $this->assertSame( + 'html_sanitizer.sanitizer.my.sanitizer', + (string) $container->getAlias(HtmlSanitizerInterface::class), + '->registerHtmlSanitizerConfiguration() creates appropriate default alias' + ); + } + protected function createContainer(array $data = []) { return new ContainerBuilder(new EnvPlaceholderParameterBag(array_merge([ diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index c515fc557ce71..8a44eff41ac86 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -44,6 +44,7 @@ "symfony/polyfill-intl-icu": "~1.0", "symfony/form": "^5.4|^6.0", "symfony/expression-language": "^5.4|^6.0", + "symfony/html-sanitizer": "^6.1", "symfony/http-client": "^5.4|^6.0", "symfony/lock": "^5.4|^6.0", "symfony/mailer": "^5.4|^6.0", From 4dd3fd6d93b9748d8da1ad37dccdffdee1d9386a Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Thu, 14 Apr 2022 21:25:34 +0200 Subject: [PATCH 2/2] Finished XML config implementation --- .../DependencyInjection/Configuration.php | 58 ++++++++++++----- .../Resources/config/schema/symfony-1.0.xsd | 55 ++++++++-------- .../DependencyInjection/ConfigurationTest.php | 6 ++ .../DependencyInjection/Fixtures/php/full.php | 47 +------------- .../Fixtures/php/html_sanitizer.php | 47 ++++++++++++++ .../Fixtures/php/html_sanitizer_default.php | 7 -- .../DependencyInjection/Fixtures/xml/full.xml | 9 --- .../Fixtures/xml/html_sanitizer.xml | 63 ++++++++++++++++++ .../Fixtures/yml/html_sanitizer.yml | 40 ++++++++++++ .../FrameworkExtensionTest.php | 65 +++++-------------- 10 files changed, 243 insertions(+), 154 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer.php delete mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer_default.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/html_sanitizer.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/html_sanitizer.yml diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index ba8137b3ccdf8..3d4408153b969 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -2123,9 +2123,20 @@ private function addHtmlSanitizerSection(ArrayNodeDefinition $rootNode, callable ->info('Default sanitizer to use when injecting without named binding.') ->end() ->arrayNode('sanitizers') - ->normalizeKeys(false) ->useAttributeAsKey('name') ->arrayPrototype() + ->fixXmlConfig('allow_element') + ->fixXmlConfig('block_element') + ->fixXmlConfig('drop_element') + ->fixXmlConfig('allow_attribute') + ->fixXmlConfig('drop_attribute') + ->fixXmlConfig('force_attribute') + ->fixXmlConfig('allowed_link_scheme') + ->fixXmlConfig('allowed_link_host') + ->fixXmlConfig('allowed_media_scheme') + ->fixXmlConfig('allowed_media_host') + ->fixXmlConfig('with_attribute_sanitizer') + ->fixXmlConfig('without_attribute_sanitizer') ->children() ->booleanNode('allow_safe_elements') ->info('Allows "safe" elements and attributes.') @@ -2136,39 +2147,58 @@ private function addHtmlSanitizerSection(ArrayNodeDefinition $rootNode, callable ->defaultFalse() ->end() ->arrayNode('allow_elements') - ->info('Configures elements as allowed. Allowed elements are elements the sanitizer should retain from the input.') - ->fixXmlConfig('allow-element') + ->info('Configures the elements that the sanitizer should retain from the input. The element name is the key, the value is either a list of allowed attributes for this element or "*" to allow the default set of attributes (https://wicg.github.io/sanitizer-api/#default-configuration).') + ->example(['i' => '*', 'a' => ['title'], 'span' => 'class']) ->normalizeKeys(false) ->useAttributeAsKey('name') - ->variablePrototype()->end() + ->variablePrototype() + ->beforeNormalization() + ->ifArray()->then(fn ($n) => $n['attribute'] ?? $n) + ->end() + ->validate() + ->ifTrue(fn ($n): bool => !\is_string($n) && !\is_array($n)) + ->thenInvalid('The value must be either a string or an array of strings.') + ->end() + ->end() ->end() ->arrayNode('block_elements') ->info('Configures elements as blocked. Blocked elements are elements the sanitizer should remove from the input, but retain their children.') - ->fixXmlConfig('block-element') + ->beforeNormalization() + ->ifString() + ->then(fn (string $n): array => (array) $n) + ->end() ->scalarPrototype()->end() ->end() ->arrayNode('drop_elements') ->info('Configures elements as dropped. Dropped elements are elements the sanitizer should remove from the input, including their children.') - ->fixXmlConfig('drop-element') + ->beforeNormalization() + ->ifString() + ->then(fn (string $n): array => (array) $n) + ->end() ->scalarPrototype()->end() ->end() ->arrayNode('allow_attributes') ->info('Configures attributes as allowed. Allowed attributes are attributes the sanitizer should retain from the input.') - ->fixXmlConfig('allow-attribute') ->normalizeKeys(false) ->useAttributeAsKey('name') - ->variablePrototype()->end() + ->variablePrototype() + ->beforeNormalization() + ->ifArray()->then(fn ($n) => $n['element'] ?? $n) + ->end() + ->end() ->end() ->arrayNode('drop_attributes') ->info('Configures attributes as dropped. Dropped attributes are attributes the sanitizer should remove from the input.') - ->fixXmlConfig('drop-attribute') ->normalizeKeys(false) ->useAttributeAsKey('name') - ->variablePrototype()->end() + ->variablePrototype() + ->beforeNormalization() + ->ifArray()->then(fn ($n) => $n['element'] ?? $n) + ->end() + ->end() ->end() ->arrayNode('force_attributes') ->info('Forcefully set the values of certain attributes on certain elements.') - ->fixXmlConfig('force-attribute') ->normalizeKeys(false) ->useAttributeAsKey('name') ->arrayPrototype() @@ -2183,12 +2213,10 @@ private function addHtmlSanitizerSection(ArrayNodeDefinition $rootNode, callable ->end() ->arrayNode('allowed_link_schemes') ->info('Allows only a given list of schemes to be used in links href attributes.') - ->fixXmlConfig('allow-link-scheme') ->scalarPrototype()->end() ->end() ->arrayNode('allowed_link_hosts') ->info('Allows only a given list of hosts to be used in links href attributes.') - ->fixXmlConfig('allow-link-host') ->scalarPrototype()->end() ->end() ->booleanNode('allow_relative_links') @@ -2197,12 +2225,10 @@ private function addHtmlSanitizerSection(ArrayNodeDefinition $rootNode, callable ->end() ->arrayNode('allowed_media_schemes') ->info('Allows only a given list of schemes to be used in media source attributes (img, audio, video, ...).') - ->fixXmlConfig('allow-media-scheme') ->scalarPrototype()->end() ->end() ->arrayNode('allowed_media_hosts') ->info('Allows only a given list of hosts to be used in media source attributes (img, audio, video, ...).') - ->fixXmlConfig('allow-media-host') ->scalarPrototype()->end() ->end() ->booleanNode('allow_relative_medias') @@ -2211,12 +2237,10 @@ private function addHtmlSanitizerSection(ArrayNodeDefinition $rootNode, callable ->end() ->arrayNode('with_attribute_sanitizers') ->info('Registers custom attribute sanitizers.') - ->fixXmlConfig('with-attribute-sanitizer') ->scalarPrototype()->end() ->end() ->arrayNode('without_attribute_sanitizers') ->info('Unregisters custom attribute sanitizers.') - ->fixXmlConfig('without-attribute-sanitizer') ->scalarPrototype()->end() ->end() ->end() diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index 49765a8f56474..3ffa5b571503b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -831,41 +831,20 @@ - - - + - - - - - - - - - - + + - - - - - - - - - - - - + @@ -873,10 +852,32 @@ - + - + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index cf313d9a5f9ec..07612df737190 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -19,6 +19,7 @@ use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\Definition\Processor; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\HtmlSanitizer\HtmlSanitizer; use Symfony\Component\HttpClient\HttpClient; use Symfony\Component\Lock\Store\SemaphoreStore; use Symfony\Component\Mailer\Mailer; @@ -639,6 +640,11 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor 'name_based_uuid_version' => 5, 'time_based_uuid_version' => 6, ], + 'html_sanitizer' => [ + 'enabled' => !class_exists(FullStack::class) && class_exists(HtmlSanitizer::class), + 'default' => null, + 'sanitizers' => [], + ], 'exceptions' => [], ]; } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php index 1aff4528aadcf..fd04b996da496 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php @@ -81,51 +81,6 @@ ], ], 'html_sanitizer' => [ - 'default' => 'my.sanitizer', - 'sanitizers' => [ - 'my.sanitizer' => [ - 'allow_safe_elements' => true, - 'allow_all_static_elements' => true, - 'allow_elements' => [ - 'custom-tag-1' => ['data-attr-1'], - 'custom-tag-2' => [], - 'custom-tag-3' => '*', - ], - 'block_elements' => [ - 'custom-tag-4', - ], - 'drop_elements' => [ - 'custom-tag-5', - ], - 'allow_attributes' => [ - 'data-attr-2' => ['custom-tag-6'], - 'data-attr-3' => [], - 'data-attr-4' => '*', - ], - 'drop_attributes' => [ - 'data-attr-5' => ['custom-tag-6'], - 'data-attr-6' => [], - 'data-attr-7' => '*', - ], - 'force_attributes' => [ - 'custom-tag-7' => [ - 'data-attr-8' => 'value', - ], - ], - 'force_https_urls' => true, - 'allowed_link_schemes' => ['http', 'https', 'mailto'], - 'allowed_link_hosts' => ['symfony.com'], - 'allow_relative_links' => true, - 'allowed_media_schemes' => ['http', 'https', 'data'], - 'allowed_media_hosts' => ['symfony.com'], - 'allow_relative_medias' => true, - 'with_attribute_sanitizers' => [ - 'App\\Sanitizer\\CustomAttributeSanitizer', - ], - 'without_attribute_sanitizers' => [ - 'App\\Sanitizer\\OtherCustomAttributeSanitizer', - ], - ], - ], + 'enabled' => true, ], ]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer.php new file mode 100644 index 0000000000000..e7b1bd41fc360 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer.php @@ -0,0 +1,47 @@ +loadFromExtension('framework', [ + 'html_sanitizer' => [ + 'default' => 'my.sanitizer', + 'sanitizers' => [ + 'my.sanitizer' => [ + 'allow_safe_elements' => true, + 'allow_all_static_elements' => true, + 'allow_elements' => [ + 'iframe' => 'src', + 'custom-tag' => ['data-attr', 'data-attr-1'], + 'custom-tag-2' => '*', + ], + 'block_elements' => ['section'], + 'drop_elements' => ['video'], + 'allow_attributes' => [ + 'src' => ['iframe'], + 'data-attr' => '*', + ], + 'drop_attributes' => [ + 'data-attr' => ['custom-tag'], + 'data-attr-1' => [], + 'data-attr-2' => '*', + ], + 'force_attributes' => [ + 'a' => ['rel' => 'noopener noreferrer'], + 'h1' => ['class' => 'bp4-heading'], + ], + 'force_https_urls' => true, + 'allowed_link_schemes' => ['http', 'https', 'mailto'], + 'allowed_link_hosts' => ['symfony.com'], + 'allow_relative_links' => true, + 'allowed_media_schemes' => ['http', 'https', 'data'], + 'allowed_media_hosts' => ['symfony.com'], + 'allow_relative_medias' => true, + 'with_attribute_sanitizers' => [ + 'App\\Sanitizer\\CustomAttributeSanitizer', + ], + 'without_attribute_sanitizers' => [ + 'App\\Sanitizer\\OtherCustomAttributeSanitizer', + ], + ], + 'all.sanitizer' => null, + ], + ], +]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer_default.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer_default.php deleted file mode 100644 index 8be1095e0868d..0000000000000 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/html_sanitizer_default.php +++ /dev/null @@ -1,7 +0,0 @@ -loadFromExtension('framework', [ - 'html_sanitizer' => [ - 'enabled' => true, - ], -]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml index bf08c25c1d644..00da2a6c0f963 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml @@ -39,14 +39,5 @@ - - - text/csv - text/plain - - - application/pdf - - diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/html_sanitizer.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/html_sanitizer.xml new file mode 100644 index 0000000000000..05cf704dd2c6c --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/html_sanitizer.xml @@ -0,0 +1,63 @@ + + + + + + + + + src + + + data-attr + data-attr-1 + + + * + + section + video + + iframe + + + * + + + custom-tag + + + + * + + + noopener noreferrer + + + bp4-heading + + http + https + mailto + symfony.com + http + https + data + symfony.com + App\Sanitizer\CustomAttributeSanitizer + App\Sanitizer\OtherCustomAttributeSanitizer + + + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/html_sanitizer.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/html_sanitizer.yml new file mode 100644 index 0000000000000..1c4f5dfcd5a4b --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/html_sanitizer.yml @@ -0,0 +1,40 @@ +framework: + html_sanitizer: + default: my.sanitizer + sanitizers: + my.sanitizer: + allow_safe_elements: true + allow_all_static_elements: true + allow_elements: + iframe: 'src' + custom-tag: ['data-attr', 'data-attr-1'] + custom-tag-2: '*' + block_elements: + - section + drop_elements: + - video + allow_attributes: + src: ['iframe'] + data-attr: '*' + drop_attributes: + data-attr: [custom-tag] + data-attr-1: [] + data-attr-2: '*' + force_attributes: + a: + rel: noopener noreferrer + h1: + class: bp4-heading + force_https_urls: true + allowed_link_schemes: ['http', 'https', 'mailto'] + allowed_link_hosts: ['symfony.com'] + allow_relative_links: true + allowed_media_schemes: ['http', 'https', 'data'] + allowed_media_hosts: ['symfony.com'] + allow_relative_medias: true + with_attribute_sanitizers: + - App\Sanitizer\CustomAttributeSanitizer + without_attribute_sanitizers: + - App\Sanitizer\OtherCustomAttributeSanitizer + + all.sanitizer: null diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 69296d461e734..e99bd83ba2028 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -2030,33 +2030,9 @@ public function testLocaleSwitcherServiceRegistered() $this->assertEquals(new Reference('router.request_context', ContainerBuilder::IGNORE_ON_INVALID_REFERENCE), $switcherDef->getArgument(2)); } - public function testHtmlSanitizerDefault() + public function testHtmlSanitizer() { - $container = $this->createContainerFromFile('html_sanitizer_default'); - - // html_sanitizer service - $this->assertTrue($container->hasDefinition('html_sanitizer'), '->registerHtmlSanitizerConfiguration() loads html_sanitizer.php'); - $this->assertSame(HtmlSanitizer::class, $container->getDefinition('html_sanitizer')->getClass()); - $this->assertCount(1, $args = $container->getDefinition('html_sanitizer')->getArguments()); - $this->assertSame('html_sanitizer.config', (string) $args[0]); - - // html_sanitizer.config service - $this->assertTrue($container->hasDefinition('html_sanitizer.config'), '->registerHtmlSanitizerConfiguration() loads html_sanitizer.php'); - $this->assertSame(HtmlSanitizerConfig::class, $container->getDefinition('html_sanitizer.config')->getClass()); - $this->assertCount(1, $calls = $container->getDefinition('html_sanitizer.config')->getMethodCalls()); - $this->assertSame(['allowSafeElements', []], $calls[0]); - - // Default alias - $this->assertSame( - 'html_sanitizer', - (string) $container->getAlias(HtmlSanitizerInterface::class), - '->registerHtmlSanitizerConfiguration() creates appropriate default alias' - ); - } - - public function testHtmlSanitizerFull() - { - $container = $this->createContainerFromFile('full'); + $container = $this->createContainerFromFile('html_sanitizer'); // html_sanitizer service $this->assertTrue($container->hasDefinition('html_sanitizer'), '->registerHtmlSanitizerConfiguration() loads html_sanitizer.php'); @@ -2084,18 +2060,18 @@ public function testHtmlSanitizerFull() [ ['allowSafeElements', [], true], ['allowAllStaticElements', [], true], - ['allowElement', ['custom-tag-1', ['data-attr-1']], true], - ['allowElement', ['custom-tag-2', []], true], - ['allowElement', ['custom-tag-3', '*'], true], - ['blockElement', ['custom-tag-4'], true], - ['dropElement', ['custom-tag-5'], true], - ['allowAttribute', ['data-attr-2', ['custom-tag-6']], true], - ['allowAttribute', ['data-attr-3', []], true], - ['allowAttribute', ['data-attr-4', '*'], true], - ['dropAttribute', ['data-attr-5', ['custom-tag-6']], true], - ['dropAttribute', ['data-attr-6', []], true], - ['dropAttribute', ['data-attr-7', '*'], true], - ['forceAttribute', ['custom-tag-7', 'data-attr-8', 'value'], true], + ['allowElement', ['iframe', 'src'], true], + ['allowElement', ['custom-tag', ['data-attr', 'data-attr-1']], true], + ['allowElement', ['custom-tag-2', '*'], true], + ['blockElement', ['section'], true], + ['dropElement', ['video'], true], + ['allowAttribute', ['src', $this instanceof XmlFrameworkExtensionTest ? 'iframe' : ['iframe']], true], + ['allowAttribute', ['data-attr', '*'], true], + ['dropAttribute', ['data-attr', $this instanceof XmlFrameworkExtensionTest ? 'custom-tag' : ['custom-tag']], true], + ['dropAttribute', ['data-attr-1', []], true], + ['dropAttribute', ['data-attr-2', '*'], true], + ['forceAttribute', ['a', 'rel', 'noopener noreferrer'], true], + ['forceAttribute', ['h1', 'class', 'bp4-heading'], true], ['forceHttpsUrls', [true], true], ['allowLinkSchemes', [['http', 'https', 'mailto']], true], ['allowLinkHosts', [['symfony.com']], true], @@ -2121,18 +2097,11 @@ static function ($call) { ); // Named alias - $this->assertSame( - 'html_sanitizer.sanitizer.my.sanitizer', - (string) $container->getAlias(HtmlSanitizerInterface::class.' $mySanitizer'), - '->registerHtmlSanitizerConfiguration() creates appropriate named alias' - ); + $this->assertSame('html_sanitizer.sanitizer.my.sanitizer', (string) $container->getAlias(HtmlSanitizerInterface::class.' $mySanitizer'), '->registerHtmlSanitizerConfiguration() creates appropriate named alias'); + $this->assertSame('html_sanitizer.sanitizer.all.sanitizer', (string) $container->getAlias(HtmlSanitizerInterface::class.' $allSanitizer'), '->registerHtmlSanitizerConfiguration() creates appropriate named alias'); // Default alias - $this->assertSame( - 'html_sanitizer.sanitizer.my.sanitizer', - (string) $container->getAlias(HtmlSanitizerInterface::class), - '->registerHtmlSanitizerConfiguration() creates appropriate default alias' - ); + $this->assertSame('html_sanitizer.sanitizer.my.sanitizer', (string) $container->getAlias(HtmlSanitizerInterface::class), '->registerHtmlSanitizerConfiguration() creates appropriate default alias'); } protected function createContainer(array $data = [])