diff --git a/.travis.yml b/.travis.yml index edd425ba..0324b0a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,11 +17,10 @@ cache: jobs: include: # Lowest - - php: 5.5 - dist: trusty - env: NO_FLEX=1 COMPOSER_FLAGS="--prefer-lowest" SYMFONY_DEPRECATIONS_HELPER=weak - php: 7.1 - env: SYMFONY_REQUIRE="4.3.*" COMPOSER_FLAGS="--prefer-lowest" + env: COMPOSER_FLAGS="--prefer-lowest" SYMFONY_DEPRECATIONS_HELPER=weak + - php: 7.1 + env: SYMFONY_REQUIRE="4.3.*" COMPOSER_FLAGS="--prefer-lowest" SYMFONY_DEPRECATIONS_HELPER=weak # Stable - php: 7.2 @@ -54,7 +53,7 @@ jobs: before_install: - mv ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/xdebug.ini{,.disabled} || echo "xdebug not available" - composer self-update - - if [[ -z $NO_FLEX ]]; then composer global require --no-progress --no-scripts --no-plugins symfony/flex; fi; + - composer global require --no-progress --no-scripts --no-plugins symfony/flex install: - composer update --prefer-dist --no-interaction $COMPOSER_FLAGS diff --git a/CHANGELOG b/CHANGELOG index 74331c35..62e68f86 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,30 @@ +## Version 3.0 (12/2019) + + * [BC break] Dropped support for PHP 5.x. PHP 7.1 minimum required. + * [BC break] Added type hints for scalar and return type hints where possible. + * [BC Break] The bundle configuration has changed: + ```yaml + # Before + exercise_html_purifier: + default: + Cache.SerializerPath: '%kernel.cache_dir%/htmlpurifier' + # ... + custom: + Core.Encoding: 'ISO-8859-1' + + # After + exercise_html_purifier: + default_cache_serializer_path: '%kernel.cache_dir%/htmlpurifier' + html_profiles: + default: + # ... + custom: + config: + Core.Encoding: 'ISO-8859-1' + ``` + * Added an `HTMLPurifierConfigFactory` to handle cache and custom definitions. + * Refactored `SerializerCacheWarmer` to preload each profile configuration + ## Version 2.0 (08/2018) * Added compatibility for Symfony 5 and Twig 3 diff --git a/README.md b/README.md index 1a1694f5..40e40a9f 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ [![Total Downloads](https://poser.pugx.org/exercise/htmlpurifier-bundle/downloads)](https://packagist.org/packages/exercise/htmlpurifier-bundle) [![Latest Stable Version](https://poser.pugx.org/exercise/htmlpurifier-bundle/v/stable)](https://packagist.org/packages/exercise/htmlpurifier-bundle) [![License](https://poser.pugx.org/exercise/htmlpurifier-bundle/license)](https://packagist.org/packages/exercise/htmlpurifier-bundle) -[![Build Status](https://travis-ci.org/Exercise/HTMLPurifierBundle.svg?branch=2.0)](https://travis-ci.org/Exercise/HTMLPurifierBundle) +[![Build Status](https://travis-ci.org/Exercise/HTMLPurifierBundle.svg?branch=master)](https://travis-ci.org/Exercise/HTMLPurifierBundle) # ExerciseHTMLPurifierBundle @@ -36,14 +36,14 @@ Register the bundle in Symfony 3: public function registerBundles() { - return array( - new Exercise\HTMLPurifierBundle\ExerciseHTMLPurifierBundle(), + return [ // ... - ); + new Exercise\HTMLPurifierBundle\ExerciseHTMLPurifierBundle(), + ]; } ``` -## Configuration in Symfony 3 without Symfony Flex +## Configuration in Symfony 3 If you do not explicitly configure this bundle, an HTMLPurifier service will be defined as `exercise_html_purifier.default`. This behavior is the same as if you @@ -53,8 +53,7 @@ had specified the following configuration: # app/config.yml exercise_html_purifier: - default: - Cache.SerializerPath: '%kernel.cache_dir%/htmlpurifier' + default_cache_serializer_path: '%kernel.cache_dir%/htmlpurifier' ``` The `default` profile is special in that it is used as the configuration for the @@ -65,10 +64,11 @@ other profiles you might define. # app/config.yml exercise_html_purifier: - default: - Cache.SerializerPath: '%kernel.cache_dir%/htmlpurifier' - custom: - Core.Encoding: 'ISO-8859-1' + default_cache_serializer_path: '%kernel.cache_dir%/htmlpurifier' + html_profiles: + custom: + config: + Core.Encoding: 'ISO-8859-1' ``` In this example, a `exercise_html_purifier.custom` service will also be defined, @@ -81,7 +81,7 @@ option to suppress the default path. [configuration documentation]: http://htmlpurifier.org/live/configdoc/plain.html -## Configuration using Symfony Flex +## Configuration in Symfony 4 and up If you do not explicitly configure this bundle, an HTMLPurifier service will be defined as `exercise_html_purifier.default`. This behavior is the same as if you @@ -91,8 +91,7 @@ had specified the following configuration: # config/packages/exercise_html_purifier.yaml exercise_html_purifier: - default: - Cache.SerializerPath: '%kernel.cache_dir%/htmlpurifier' + default_cache_serializer_path: '%kernel.cache_dir%/htmlpurifier' ``` The `default` profile is special, it is *always* defined and its configuration @@ -104,27 +103,33 @@ configuration. # config/packages/exercise_html_purifier.yaml exercise_html_purifier: - default: - Cache.SerializerPath: '%kernel.cache_dir%/htmlpurifier' - custom: - Core.Encoding: 'ISO-8859-1' + default_cache_serializer_path: 'tmp/htmlpurifier' + html_profiles: + default: + config: + Cache.SerializerPermissions: 777 + custom: + config: + Core.Encoding: 'ISO-8859-1' ``` - + ## Autowiring By default type hinting `\HtmlPurifier` in your services will autowire the `exercise_html_purifier.default` service. To override it and use your own config as default autowired services just add -this in you `app/config/services.yml` or `config/services.yaml`: +this in you `app/config/services.yml` in you use symfony 3 or `config/services.yaml` +if you use symfony 4: ```yaml +# config/services.yaml services: - # ... - + #... + exercise_html_purifier.default: '@exercise_html_purifier.custom' ``` -## Using a custom purifier class as default +### Using a custom purifier class as default If you want to use your own class as default purifier, define a new alias: @@ -139,6 +144,29 @@ services: In such case, the custom purifier will use its own defined configuration, ignoring the bundle configuration. +### Argument binding + +The bundle also leverages the alias argument binding for each profile. So the +following config: + +```yaml + html_profiles: + blog: + # ... + gallery: + # ... +``` + +will register the following binding: + +```php + // default config is bound whichever argument name is used +public function __construct(\HTMLPurifier $purifier) {} +public function __construct(\HTMLPurifier $htmlPurifier) {} +public function __construct(\HTMLPurifier $blogPurifier) {} // blog config +public function __construct(\HTMLPurifier $galleryPurifier) {} // gallery config +``` + ## Form Type Extension This bundles provides a form type extension for filtering form fields with @@ -229,6 +257,128 @@ $builder {{ html_string|purify('custom') }} ``` +## How to Customize a Config Definition + +# Custom Attributes + +In some case, you might want to set some rules for a specific tag. +This is what the following config is about: + +```yaml +# config/packages/exercise_html_purifier.yaml +exercise_html_purifier: + html_profiles: + default: + config: + HTML.Allowed: < + *[id|class|name], + a[href|title|rel|target], + img[src|alt|height|width], + br,div,embed,object,u,em,ul,ol,li,strong,span + attributes: + img: + # attribute name, type (Integer, Color, ...) + data-id: ID + data-image-size: Text + span: + data-link: URI +``` + +See [HTMLPurifier_AttrTypes][] for more options. + + [HTMLPurifier_AttrTypes]: https://github.com/ezyang/htmlpurifier/blob/master/library/HTMLPurifier/AttrTypes.php + +# Custom Elements + +In some case, you might want to set some rules for a specific tag. +This is what the following config is about: + +```yaml +# config/packages/exercise_html_purifier.yaml +exercise_html_purifier: + html_profiles: + default: + # ... + elements: + video: + - Block + - 'Optional: (source, Flow) | (Flow, source) | Flow' + - Common # allows a set of common attributes + # The 4th and 5th arguments are optional + - src: URI # list of type rules by attributes + type: Text + width: Length + height: Length + poster: URI + preload: 'Enum#auto,metadata,none' + controls: Bool + source: + - Block + - Flow + - Common + - { src: URI, type: Text } + - [style] # list of forbidden attributes +``` + +Would be equivalent to: + +```php +$def = $config->getHTMLDefintion(true); +$def->addElement('video', 'Block', 'Optional: (source, Flow) | (Flow, source) | Flow', 'Common', [ + 'src' => 'URI', + 'type' => 'Text', + 'width' => 'Length', + 'height' => 'Length', + 'poster' => 'URI', + 'preload' => 'Enum#auto,metadata,none', + 'controls' => 'Bool', +]); +$source = $def->addElement('source', 'Block', 'Flow', 'Common', [ + 'src' => 'URI', + 'type' => 'Text', +]); +$source->excludes = ['style' => true]; +``` + +See [HTMLPurifier documentation][] for more details. + + [HTMLPurifier documentation]: http://htmlpurifier.org/docs/enduser-customize.html + +# Blank Elements + +It might happen that you need a tag clean from any attributes. +Then just add it to the list: + +```yaml +# config/packages/exercise_html_purifier.yaml +exercise_html_purifier: + html_profiles: + default: + # ... + blank_elements: [legend, figcaption] +``` + +## How to Reuse Profiles + +What can really convenient is to reuse some profile definition +to build other custom definitions. + +```yaml +# config/packages/exercise_html_purifier.yaml +exercise_html_purifier: + html_profiles: + base: + # ... + video: + # ... + all: + parents: [base, video] +``` + +In this example the profile named "all" will inherit the "default" profile, +then the two custom ones. The order is important as each profile overrides the +previous, and "all" could define its own rules too. + ## Contributing PRs are welcomed :). Please target the `2.0` branch for bug fixes and `master` diff --git a/Tests/HTMLPurifierConfigFactoryTest.php b/Tests/HTMLPurifierConfigFactoryTest.php new file mode 100644 index 00000000..e0a129d6 --- /dev/null +++ b/Tests/HTMLPurifierConfigFactoryTest.php @@ -0,0 +1,67 @@ +mkdir(self::$cacheDir); + } + + public static function tearDownAfterClass(): void + { + (new Filesystem())->remove(self::$cacheDir); + } + + public function testCreateUseDoesNotBuildDefinitionByDefault() + { + TestHTMLPurifierConfigFactory::create('default', []); + + $this->assertSame(0, TestHTMLPurifierConfigFactory::$calledBuild); + } + + public function testCreateUseSerializedCache() + { + $configArgs = [ + 'test', /* profile */ + [/* config array */ + 'Cache.SerializerPath' => self::$cacheDir, + 'HTML.Nofollow' => true, + ], + null, /* default config */ + [], /* parents */ + ['a' => ['href' => 'URI']], /* attributes */ + ]; + + (new \HTMLPurifier( + TestHTMLPurifierConfigFactory::create(...$configArgs) + ))->purify('
test
'); + + TestHTMLPurifierConfigFactory::create(...$configArgs); + + $this->assertSame(1, TestHTMLPurifierConfigFactory::$calledBuild); + } +} + +class TestHTMLPurifierConfigFactory extends HTMLPurifierConfigFactory +{ + public static $calledBuild = 0; + + public static function buildHTMLDefinition( + \HTMLPurifier_Definition $def, + array $attributes, + array $elements, + array $blankElements + ): void { + ++self::$calledBuild; + parent::buildHTMLDefinition($def, $attributes, $elements, $blankElements); + } +} diff --git a/composer.json b/composer.json index 6c0bd3e5..c9aee97b 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,7 @@ } ], "require": { - "php": "^5.5.9|>=7.0.8", + "php": "^7.1.3", "ezyang/htmlpurifier": "~4.0", "symfony/config": "~3.4 || ~4.0 || ^5.0", "symfony/dependency-injection": "~3.4.1 || ^4.0.1 || ^5.0", @@ -35,7 +35,7 @@ }, "extra": { "branch-alias": { - "dev-master": "2.0.x-dev" + "dev-master": "3.0.x-dev" } } } diff --git a/phpunit.xml.dist b/phpunit.xml.dist index ad6b264b..60a44c95 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,13 +1,6 @@ - ./tests diff --git a/src/CacheWarmer/SerializerCacheWarmer.php b/src/CacheWarmer/SerializerCacheWarmer.php index bee1fd53..a0f9c762 100644 --- a/src/CacheWarmer/SerializerCacheWarmer.php +++ b/src/CacheWarmer/SerializerCacheWarmer.php @@ -2,30 +2,40 @@ namespace Exercise\HTMLPurifierBundle\CacheWarmer; +use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistryInterface; +use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface; /** * Cache warmer for creating HTMLPurifier's cache directory and contents. * - * Run purify() with various contents to have the caches built here, and not - * on first use, as the owning user may be different then, causing problems - * with file ownership when deleting the cached files later. + * Create all purifiers to generate their caches here, and not on first use, as + * the owning user may be different then, causing problems with file ownership + * when deleting the cached files later. + * + * See https://github.com/Exercise/HTMLPurifierBundle/issues/22 * * @author Henrik Bjornskov + * @author Jules Pietri */ class SerializerCacheWarmer implements CacheWarmerInterface { private $paths; - private $htmlPurifier; + private $profiles; + private $registry; + private $filesystem; /** - * @param string[] $paths - * @param \HTMLPurifier $htmlPurifier Used to build cache within bundle runtime + * @param string[] $paths + * @param string[] $profiles + * @param HTMLPurifiersRegistryInterface $registry Used to build cache within bundle runtime */ - public function __construct(array $paths, \HTMLPurifier $htmlPurifier) + public function __construct(array $paths, array $profiles, HTMLPurifiersRegistryInterface $registry, Filesystem $filesystem) { $this->paths = $paths; - $this->htmlPurifier = $htmlPurifier; + $this->profiles = $profiles; + $this->registry = $registry; + $this->filesystem = $filesystem; } /** @@ -34,19 +44,14 @@ public function __construct(array $paths, \HTMLPurifier $htmlPurifier) public function warmUp($cacheDir) { foreach ($this->paths as $path) { - if (!is_dir($path)) { - if (false === @mkdir($path, 0777, true)) { - throw new \RuntimeException(sprintf('Unable to create the HTMLPurifier Serializer cache directory "%s".', $path)); - } - } elseif (!is_writable($path)) { - throw new \RuntimeException(sprintf('The HTMLPurifier Serializer cache directory "%s" is not writeable for the current system user.', $path)); - } + $this->filesystem->remove($path); // clean previous cache + $this->filesystem->mkdir($path); } - // build htmlPurifier cache for HTML/CSS & URIs with the other Symfony cache warmups. - // see https://github.com/Exercise/HTMLPurifierBundle/issues/22 - $this->htmlPurifier->purify('
-2
'); - $this->htmlPurifier->purify('
'); + foreach ($this->profiles as $profile) { + // Will build the configuration + $this->registry->get($profile)->purify("
"); + } } /** diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index f30b6a76..8ecee7ad 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -4,6 +4,7 @@ use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; +use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; class Configuration implements ConfigurationInterface { @@ -22,10 +23,77 @@ public function getConfigTreeBuilder() } $rootNode - ->useAttributeAsKey('name') - ->prototype('array') - ->useAttributeAsKey('name') - ->prototype('variable') + ->children() + ->scalarNode('default_cache_serializer_path') + ->defaultValue('%kernel.cache_dir%/htmlpurifier') + ->end() + ->arrayNode('html_profiles') + ->useAttributeAsKey('name') + ->normalizeKeys(false) + ->validate() + ->always(function ($profiles) { + foreach ($profiles as $profile => $definition) { + foreach ($definition['parents'] as $parent) { + if (!isset($profiles[$parent])) { + throw new InvalidConfigurationException(sprintf('Invalid parent "%s" is not defined for profile "%s".', $parent, $profile)); + } + } + } + + return $profiles; + }) + ->end() + ->arrayPrototype() + ->children() + ->arrayNode('config') + ->defaultValue([]) + ->info('An array of parameters.') + ->useAttributeAsKey('parameter') + ->normalizeKeys(false) + ->variablePrototype()->end() + ->end() + ->arrayNode('attributes') + ->defaultValue([]) + ->info('Every key is a tag name, with arrays for rules') + ->normalizeKeys(false) + ->useAttributeAsKey('tag_name') + ->arrayPrototype() + ->info('Every key is an attribute name for a rule like "Text"') + ->useAttributeAsKey('attribute_name') + ->normalizeKeys(false) + ->scalarPrototype()->end() + ->end() + ->end() + ->arrayNode('elements') + ->defaultValue([]) + ->info('Every key is a tag name, with an array of four values as definition. The fourth is an optional array of attributes rules.') + ->normalizeKeys(false) + ->useAttributeAsKey('tag_name') + ->info('An array represents a definition, with three required elements: a type ("Inline", "Block", ...), a content type ("Empty", "Optional: #PCDATA", ...), an attributes set ("Core", "Common", ...), a fourth optional may define attributes rules as array, and fifth for forbidden attributes.') + ->arrayPrototype() + ->validate() + ->ifTrue(function ($array) { + $count = count($array); + + return 3 > $count || $count > 5; + }) + ->thenInvalid('An element definition must define three to five elements: a type ("Inline", "Block", ...), a content type ("Empty", "Optional: #PCDATA", ...), an attributes set ("Core", "Common", ...), and a fourth optional may define attributes rules as array, and fifth for forbidden attributes.') + ->end() + ->variablePrototype()->end() + ->end() + ->end() + ->arrayNode('blank_elements') + ->defaultValue([]) + ->info('An array of tag names that should purify everything.') + ->scalarPrototype()->end() + ->end() + ->arrayNode('parents') + ->defaultValue([]) + ->info('An array of config names that should be inherited.') + ->scalarPrototype()->end() + ->end() + ->end() + ->end() ->end() ->end() ; diff --git a/src/DependencyInjection/ExerciseHTMLPurifierExtension.php b/src/DependencyInjection/ExerciseHTMLPurifierExtension.php index c8aae1b6..76965aaf 100644 --- a/src/DependencyInjection/ExerciseHTMLPurifierExtension.php +++ b/src/DependencyInjection/ExerciseHTMLPurifierExtension.php @@ -3,6 +3,7 @@ namespace Exercise\HTMLPurifierBundle\DependencyInjection; use Exercise\HTMLPurifierBundle\DependencyInjection\Compiler\HTMLPurifierPass; +use Exercise\HTMLPurifierBundle\HTMLPurifierConfigFactory; use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistry; use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistryInterface; use Symfony\Component\Config\FileLocator; @@ -13,59 +14,61 @@ class ExerciseHTMLPurifierExtension extends Extension { - /** - * {@inheritdoc} - */ public function load(array $configs, ContainerBuilder $container) { $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); $loader->load('html_purifier.xml'); - /* Prepend the default configuration. This cannot be defined within the - * Configuration class, since the root node's children are array - * prototypes. - * - * This cache path may be suppressed by either unsetting the "default" - * configuration (relying on canBeUnset() on the prototype node) or - * setting the "Cache.SerializerPath" option to null. - */ - array_unshift($configs, [ - 'default' => [ - 'Cache.SerializerPath' => '%kernel.cache_dir%/htmlpurifier', - ], - ]); - $configs = $this->processConfiguration(new Configuration(), $configs); + // Set default serializer cache path, while ensuring a default profile is defined + $configs['html_profiles']['default']['config']['Cache.SerializerPath'] = $configs['default_cache_serializer_path']; + $serializerPaths = []; + // Drop when require Symfony > 3.4 + $registerAlias = method_exists($container, 'registerAliasForArgument'); - foreach ($configs as $name => $config) { + foreach ($configs['html_profiles'] as $name => $definition) { $configId = "exercise_html_purifier.config.$name"; - $configDefinition = $container->register($configId, \HTMLPurifier_Config::class) - ->setPublic(false) - ; + $default = null; + $parents = []; // stores inherited configs - if ('default' === $name) { - $configDefinition - ->setFactory([\HTMLPurifier_Config::class, 'create']) - ->addArgument($config) - ; - } else { - $configDefinition - ->setFactory([\HTMLPurifier_Config::class, 'inherit']) - ->addArgument(new Reference('exercise_html_purifier.config.default')) - ->addMethodCall('loadArray', [$config]) - ; + if ('default' !== $name) { + $default = new Reference('exercise_html_purifier.config.default'); + $parentNames = $definition['parents']; + + unset($parentNames['default']); // default is always inherited + foreach ($parentNames as $parentName) { + self::resolveProfileInheritance($parentName, $configs['html_profiles'], $parents); + } } - $container->register("exercise_html_purifier.$name", \HTMLPurifier::class) - ->addArgument(new Reference($configId)) + $container->register($configId, \HTMLPurifier_Config::class) + ->setFactory([HTMLPurifierConfigFactory::class, 'create']) + ->setArguments([ + $name, + $definition['config'], + $default, + self::getResolvedConfig('config', $parents), + self::getResolvedConfig('attributes', $parents, $definition), + self::getResolvedConfig('elements', $parents, $definition), + self::getResolvedConfig('blank_elements', $parents, $definition), + ]) + ; + + $id = "exercise_html_purifier.$name"; + $container->register($id, \HTMLPurifier::class) + ->setArguments([new Reference($configId)]) ->addTag(HTMLPurifierPass::PURIFIER_TAG, ['profile' => $name]) ; - if (isset($config['Cache.SerializerPath'])) { - $serializerPaths[] = $config['Cache.SerializerPath']; + if (isset($definition['config']['Cache.SerializerPath'])) { + $serializerPaths[] = $definition['config']['Cache.SerializerPath']; + } + + if ($registerAlias && $default) { + $container->registerAliasForArgument($id, \HTMLPurifier::class, "$name.purifier"); } } @@ -78,14 +81,43 @@ public function load(array $configs, ContainerBuilder $container) $container->setAlias(\HTMLPurifier::class, 'exercise_html_purifier.default') ->setPublic(false) ; - $container->setParameter('exercise_html_purifier.cache_warmer.serializer.paths', array_unique($serializerPaths)); + $container->getDefinition('exercise_html_purifier.cache_warmer.serializer') + ->setArgument(0, array_unique($serializerPaths)) + ->setArgument(1, array_keys($configs['html_profiles'])) + ; } - /** - * {@inheritdoc} - */ public function getAlias() { return 'exercise_html_purifier'; } + + private static function resolveProfileInheritance(string $parent, array $configs, array &$resolved): void + { + if (isset($resolved[$parent])) { + // Another profile already inherited this config, skip + return; + } + + foreach ($configs[$parent]['parents'] as $grandParent) { + self::resolveProfileInheritance($grandParent, $configs, $resolved); + } + + $resolved[$parent]['config'] = $configs[$parent]['config']; + $resolved[$parent]['attributes'] = $configs[$parent]['attributes']; + $resolved[$parent]['elements'] = $configs[$parent]['elements']; + $resolved[$parent]['blank_elements'] = $configs[$parent]['blank_elements']; + } + + private static function getResolvedConfig(string $parameter, array $parents, array $definition = null): array + { + if (null !== $definition) { + return array_filter(array_merge( + array_column($parents, $parameter), + isset($definition[$parameter]) ? $definition[$parameter] : [] + )); + } + + return array_filter(array_column($parents, $parameter)); + } } diff --git a/src/Form/Listener/HTMLPurifierListener.php b/src/Form/Listener/HTMLPurifierListener.php index 846151fe..98d86dd1 100644 --- a/src/Form/Listener/HTMLPurifierListener.php +++ b/src/Form/Listener/HTMLPurifierListener.php @@ -12,16 +12,13 @@ class HTMLPurifierListener implements EventSubscriberInterface private $registry; private $profile; - /** - * @param string $profile - */ - public function __construct(HTMLPurifiersRegistryInterface $registry, $profile) + public function __construct(HTMLPurifiersRegistryInterface $registry, string $profile) { $this->registry = $registry; $this->profile = $profile; } - public function purifySubmittedData(FormEvent $event) + public function purifySubmittedData(FormEvent $event): void { if (!is_scalar($data = $event->getData())) { // Hope there is a view transformer, otherwise an error might happen @@ -49,10 +46,7 @@ public static function getSubscribedEvents() ]; } - /** - * @return \HTMLPurifier - */ - private function getPurifier() + private function getPurifier(): \HTMLPurifier { return $this->registry->get($this->profile); } diff --git a/src/Form/TypeExtension/ForwardCompatTypeExtensionTrait.php b/src/Form/TypeExtension/ForwardCompatTypeExtensionTrait.php deleted file mode 100644 index 4f1f1eeb..00000000 --- a/src/Form/TypeExtension/ForwardCompatTypeExtensionTrait.php +++ /dev/null @@ -1,47 +0,0 @@ - ['src' => 'URI', 'data-type' => Text']] ] + * @param array $elements An array of arrays by element to add or override, arrays must + * hold a type ("Inline, "Block", ...), a content type ("Empty", + * "Optional: #PCDATA", ...), an attributes set ("Core", "Common", + * ...), a fourth optional may define attributes rules as array, and + * a fifth to list forbidden attributes + * @param array $blankElements An array of tag names that should not have any attributes + */ + public static function create( + string $profile, + array $configArray, + \HTMLPurifier_Config $defaultConfig = null, + array $parents = [], + array $attributes = [], + array $elements = [], + array $blankElements = [] + ): \HTMLPurifier_Config { + if ($defaultConfig) { + $config = \HTMLPurifier_Config::inherit($defaultConfig); + } else { + $config = \HTMLPurifier_Config::createDefault(); + } + + foreach ($parents as $parent) { + $config->loadArray($parent); + } + + $config->loadArray($configArray); + + // Make the config unique + $config->set('HTML.DefinitionID', $profile); + $config->set('HTML.DefinitionRev', 1); + + $def = $config->maybeGetRawHTMLDefinition(); + + // If the definition is not cached, build it + if ($def && ($attributes || $elements || $blankElements)) { + static::buildHTMLDefinition($def, $attributes, $elements, $blankElements); + } + + return $config; + } + + /** + * Builds a config definition from the given parameters. + * + * This build should never happen on runtime, since purifiers cache should + * be generated during warm up. + */ + public static function buildHTMLDefinition(\HTMLPurifier_Definition $def, array $attributes, array $elements, array $blankElements): void + { + foreach ($attributes as $elementName => $rule) { + foreach ($rule as $attributeName => $definition) { + /* @see \HTMLPurifier_AttrTypes */ + $def->addAttribute($elementName, $attributeName, $definition); + } + } + + foreach ($elements as $elementName => $config) { + /* @see \HTMLPurifier_HTMLModule::addElement() */ + $el = $def->addElement($elementName, $config[0], $config[1], $config[2], isset($config[3]) ? $config[3] : []); + + if (isset($config[4])) { + $el->excludes = array_fill_keys($config[4], true); + } + } + + foreach ($blankElements as $blankElement) { + /* @see \HTMLPurifier_HTMLModule::addBlankElement() */ + $def->addBlankElement($blankElement); + } + } +} diff --git a/src/HTMLPurifiersRegistry.php b/src/HTMLPurifiersRegistry.php index a96917ce..ec8f39c8 100644 --- a/src/HTMLPurifiersRegistry.php +++ b/src/HTMLPurifiersRegistry.php @@ -16,7 +16,7 @@ public function __construct(ContainerInterface $purifiersLocator) /** * {@inheritdoc} */ - public function has($profile) + public function has(string $profile): bool { return $this->purifiersLocator->has($profile); } @@ -24,7 +24,7 @@ public function has($profile) /** * {@inheritdoc} */ - public function get($profile) + public function get(string $profile): \HTMLPurifier { return $this->purifiersLocator->get($profile); } diff --git a/src/HTMLPurifiersRegistryInterface.php b/src/HTMLPurifiersRegistryInterface.php index 06136d49..0f484a72 100644 --- a/src/HTMLPurifiersRegistryInterface.php +++ b/src/HTMLPurifiersRegistryInterface.php @@ -4,17 +4,7 @@ interface HTMLPurifiersRegistryInterface { - /** - * @param string $profile - * - * @return bool - */ - public function has($profile); + public function has(string $profile): bool; - /** - * @param string $profile - * - * @return \HTMLPurifier - */ - public function get($profile); + public function get(string $profile): \HTMLPurifier; } diff --git a/src/Resources/config/html_purifier.xml b/src/Resources/config/html_purifier.xml index f3e2bc63..d3bfac09 100644 --- a/src/Resources/config/html_purifier.xml +++ b/src/Resources/config/html_purifier.xml @@ -4,8 +4,10 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - %exercise_html_purifier.cache_warmer.serializer.paths% - + + + + diff --git a/src/Twig/HTMLPurifierRuntime.php b/src/Twig/HTMLPurifierRuntime.php index 025cf0e7..8c152528 100644 --- a/src/Twig/HTMLPurifierRuntime.php +++ b/src/Twig/HTMLPurifierRuntime.php @@ -22,7 +22,7 @@ public function __construct(HTMLPurifiersRegistryInterface $registry) * * @return string The purified html string */ - public function purify($string, $profile = 'default') + public function purify(string $string, string $profile = 'default'): string { return $this->getHTMLPurifierForProfile($profile)->purify($string); } @@ -30,13 +30,9 @@ public function purify($string, $profile = 'default') /** * Gets the HTMLPurifier service corresponding to the given profile. * - * @param string $profile - * - * @return \HTMLPurifier - * * @throws \InvalidArgumentException If the profile does not exist */ - private function getHTMLPurifierForProfile($profile) + private function getHTMLPurifierForProfile(string $profile): \HTMLPurifier { return $this->purifiersRegistry->get($profile); } diff --git a/tests/CacheWarmer/SerializerCacheWarmerTest.php b/tests/CacheWarmer/SerializerCacheWarmerTest.php index fa20dd62..42856753 100644 --- a/tests/CacheWarmer/SerializerCacheWarmerTest.php +++ b/tests/CacheWarmer/SerializerCacheWarmerTest.php @@ -3,46 +3,60 @@ namespace Exercise\HTMLPurifierBundle\Tests\CacheWarmer; use Exercise\HTMLPurifierBundle\CacheWarmer\SerializerCacheWarmer; +use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistryInterface; use PHPUnit\Framework\TestCase; +use Symfony\Component\Filesystem\Filesystem; class SerializerCacheWarmerTest extends TestCase { public function testShouldBeRequired() { - $cacheWarmer = new SerializerCacheWarmer([], new \HTMLPurifier()); + $cacheWarmer = new SerializerCacheWarmer([], [], $this->createMock(HTMLPurifiersRegistryInterface::class), new Filesystem()); + $this->assertFalse($cacheWarmer->isOptional()); } - public function testFailsWhenNotWriteable() + public function testWarmUpShouldCreatePaths() { - $path = sys_get_temp_dir().'/'.uniqid('htmlpurifierbundle_fails'); + $fs = new Filesystem(); + $path = sys_get_temp_dir().DIRECTORY_SEPARATOR.'html_purifier'; - if (false === @mkdir($path, 0000)) { - $this->markTestSkipped('Tmp dir is not writeable.'); + if ($fs->exists($path)) { + $fs->remove($path); } - $this->expectException('RuntimeException'); + $this->assertFalse($fs->exists($path)); - $cacheWarmer = new SerializerCacheWarmer([$path], new \HTMLPurifier()); + $cacheWarmer = new SerializerCacheWarmer([$path], [], $this->createMock(HTMLPurifiersRegistryInterface::class), $fs); $cacheWarmer->warmUp(null); - @rmdir($path); + $this->assertTrue($fs->exists($path)); + + $fs->remove($path); } - public function testShouldCreatePaths() + public function testWarmUpShouldCallPurifyForEachProfile() { - if (!is_writable(sys_get_temp_dir())) { - $this->markTestSkipped(sprintf('The system temp directory "%s" is not writeable for the current system user.', sys_get_temp_dir())); - } - - $path = sys_get_temp_dir().'/'.uniqid('htmlpurifierbundle'); - - $cacheWarmer = new SerializerCacheWarmer([$path], new \HTMLPurifier()); + $purifier = $this->createMock(\HTMLPurifier::class); + $purifier->expects($this->exactly(2)) + ->method('purify') + ; + + $registry = $this->createMock(HTMLPurifiersRegistryInterface::class); + $registry->expects($this->exactly(2)) + ->method('get') + ->willReturn($purifier) + ; + $registry->expects($this->at(0)) + ->method('get') + ->with('first') + ; + $registry->expects($this->at(1)) + ->method('get') + ->with('second') + ; + + $cacheWarmer = new SerializerCacheWarmer([], ['first', 'second'], $registry, new Filesystem()); $cacheWarmer->warmUp(null); - - $this->assertTrue(is_dir($path)); - $this->assertTrue(is_writeable($path)); - - rmdir($path); } } diff --git a/tests/DependencyInjection/Compiler/HTMLPurifierPassTest.php b/tests/DependencyInjection/Compiler/HTMLPurifierPassTest.php index b7c88e4a..7c484f2f 100644 --- a/tests/DependencyInjection/Compiler/HTMLPurifierPassTest.php +++ b/tests/DependencyInjection/Compiler/HTMLPurifierPassTest.php @@ -5,9 +5,9 @@ use Exercise\HTMLPurifierBundle\DependencyInjection\Compiler\HTMLPurifierPass; use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistry; use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistryInterface; -use Exercise\HTMLPurifierBundle\Tests\ForwardCompatTestTrait; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; @@ -16,12 +16,10 @@ class HTMLPurifierPassTest extends TestCase { - use ForwardCompatTestTrait; - /** @var ContainerBuilder|MockObject */ private $container; - private function doSetUp() + protected function setUp(): void { $this->container = $this->createPartialMock(ContainerBuilder::class, [ 'hasAlias', @@ -31,7 +29,7 @@ private function doSetUp() ]); } - private function doTearDown() + protected function tearDown(): void { $this->container = null; } @@ -95,6 +93,22 @@ public function testProcessDoNothingIfRegistryIsNotDefined() $pass = new HTMLPurifierPass(); $pass->process($this->container); } + + public function testProcessFailsIfTaggedServiceMissesProfileName() + { + $container = new ContainerBuilder(); + $container->register(DummyPurifier::class) + ->addTag('exercise.html_purifier') + ; + $container->register('exercise_html_purifier.purifiers_registry', HTMLPurifiersRegistry::class); + $container->setAlias(HTMLPurifiersRegistryInterface::class, 'exercise_html_purifier.purifiers_registry'); + + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Tag "exercise.html_purifier" must define a "profile" attribute.'); + + $pass = new HTMLPurifierPass(); + $pass->process($container); + } } class DummyPurifier extends \HTMLPurifier diff --git a/tests/DependencyInjection/ExerciseHTMLPurifierExtensionTest.php b/tests/DependencyInjection/ExerciseHTMLPurifierExtensionTest.php index 5e445548..274e5431 100644 --- a/tests/DependencyInjection/ExerciseHTMLPurifierExtensionTest.php +++ b/tests/DependencyInjection/ExerciseHTMLPurifierExtensionTest.php @@ -4,16 +4,17 @@ use Exercise\HTMLPurifierBundle\DependencyInjection\Compiler\HTMLPurifierPass; use Exercise\HTMLPurifierBundle\DependencyInjection\ExerciseHTMLPurifierExtension; -use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistry; +use Exercise\HTMLPurifierBundle\HTMLPurifierConfigFactory; use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistryInterface; -use Exercise\HTMLPurifierBundle\Tests\ForwardCompatTestTrait; use PHPUnit\Framework\TestCase; +use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Reference; class ExerciseHTMLPurifierExtensionTest extends TestCase { - use ForwardCompatTestTrait; + private const DEFAULT_CACHE_PATH = '%kernel.cache_dir%/htmlpurifier'; /** * @var ContainerBuilder @@ -30,16 +31,17 @@ class ExerciseHTMLPurifierExtensionTest extends TestCase */ private $defaultConfig; - private function doSetUp() + public function setUp(): void { $this->container = new ContainerBuilder(); + $this->container->setParameter('kernel.cache_dir', '/tmp'); $this->extension = new ExerciseHTMLPurifierExtension(); $this->defaultConfig = [ - 'Cache.SerializerPath' => '%kernel.cache_dir%/htmlpurifier', + 'Cache.SerializerPath' => self::DEFAULT_CACHE_PATH, ]; } - private function doTearDown() + public function tearDown(): void { $this->defaultConfig = null; $this->extension = null; @@ -51,77 +53,321 @@ public function testShouldLoadDefaultConfiguration() $this->extension->load([], $this->container); $this->assertDefaultConfigDefinition($this->defaultConfig); - $this->assertCacheWarmerSerializerPaths(['%kernel.cache_dir%/htmlpurifier']); + $this->assertCacheWarmerSerializerArgs([self::DEFAULT_CACHE_PATH], ['default']); $this->assertRegistryHasProfiles(['default']); } + public function testInvalidParent() + { + $config = [ + 'html_profiles' => [ + 'custom' => [ + 'config' => ['AutoFormat.AutoParagraph' => true], + ], + 'custom_2' => [ + 'config' => ['AutoFormat.Linkify' => true], + 'parents' => ['custom', 'unknown'], + ], + ], + ]; + + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid parent "unknown" is not defined for profile "custom_2".'); + + $this->extension->load([$config], $this->container); + } + + /** + * @dataProvider provideInvalidElementDefinitions + */ + public function testInvalidElements(array $elementDefinition) + { + $config = [ + 'html_profiles' => [ + 'default' => [ + 'elements' => ['a' => []], + ], + ], + ]; + + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid configuration for path "exercise_html_purifier.html_profiles.default.elements.a": An element definition must define three to five elements: a type ("Inline", "Block", ...), a content type ("Empty", "Optional: #PCDATA", ...), an attributes set ("Core", "Common", ...), and a fourth optional may define attributes rules as array, and fifth for forbidden attributes.'); + + $this->extension->load([$config], $this->container); + } + + public function provideInvalidElementDefinitions(): iterable + { + yield 'empty array' => [[]]; + yield 'only one argument' => [['']]; + yield 'only two arguments' => [['', '']]; + yield 'too many arguments' => [['', '', '', [], [], 'extra argument']]; + } + public function testShouldAllowOverridingDefaultConfigurationCacheSerializerPath() { $config = [ - 'default' => [ - 'AutoFormat.AutoParagraph' => true, - 'Cache.SerializerPath' => null, + 'default_cache_serializer_path' => null, + 'html_profiles' => [ + 'default' => [ + 'config' => [ + 'AutoFormat.AutoParagraph' => true, + ], + ], ], ]; $this->extension->load([$config], $this->container); - $this->assertDefaultConfigDefinition($config['default']); - $this->assertCacheWarmerSerializerPaths([]); + $this->assertDefaultConfigDefinition(array_merge($config['html_profiles']['default']['config'], [ + 'Cache.SerializerPath' => null, + ])); + $this->assertCacheWarmerSerializerArgs([], ['default']); $this->assertRegistryHasProfiles(['default']); } public function testShouldNotDeepMergeOptions() { $configs = [ - ['default' => [ - 'Core.HiddenElements' => ['script' => true], - 'Cache.SerializerPath' => null, + ['html_profiles' => [ + 'default' => [ + 'config' => [ + 'Core.HiddenElements' => ['script' => true], + ], + ], ]], - ['default' => [ - 'Core.HiddenElements' => ['style' => true], + ['html_profiles' => [ + 'default' => [ + 'config' => [ + 'Core.HiddenElements' => ['style' => true], + ], + ], ]], ]; $this->extension->load($configs, $this->container); - $this->assertDefaultConfigDefinition([ + $this->assertDefaultConfigDefinition(array_merge([ 'Core.HiddenElements' => ['style' => true], - 'Cache.SerializerPath' => null, - ]); - $this->assertCacheWarmerSerializerPaths([]); + ], $this->defaultConfig)); + $this->assertCacheWarmerSerializerArgs([self::DEFAULT_CACHE_PATH], ['default']); $this->assertRegistryHasProfiles(['default']); } public function testShouldLoadCustomConfiguration() { $config = [ - 'default' => [ - 'AutoFormat.AutoParagraph' => true, + 'html_profiles' => [ + 'default' => [ + 'config' => [ + 'AutoFormat.AutoParagraph' => true, + ], + ], + 'simple' => [ + 'config' => [ + 'Cache.DefinitionImpl' => null, + 'Cache.SerializerPath' => '%kernel.cache_dir%/htmlpurifier-simple', + 'AutoFormat.Linkify' => true, + 'AutoFormat.RemoveEmpty' => true, + 'AutoFormat.RemoveEmpty.RemoveNbsp' => true, + 'HTML.Allowed' => 'a[href],strong,em,p,li,ul,ol', + ], + ], + 'advanced' => [ + 'config' => [ + 'Cache.DefinitionImpl' => null, + ], + ], + ], + ]; + + $this->extension->load([$config], $this->container); + + $profiles = ['default', 'simple', 'advanced']; + + $this->assertDefaultConfigDefinition(array_merge($config['html_profiles']['default']['config'], $this->defaultConfig)); + $this->assertConfigDefinition('simple', $config['html_profiles']['simple']['config']); + $this->assertConfigDefinition('advanced', $config['html_profiles']['advanced']['config']); + $this->assertCacheWarmerSerializerArgs([ + self::DEFAULT_CACHE_PATH, + self::DEFAULT_CACHE_PATH.'-simple', + ], $profiles); + $this->assertRegistryHasProfiles($profiles); + } + + public function testShouldLoadComplexCustomConfiguration() + { + $defaultConfig = [ + 'AutoFormat.AutoParagraph' => true, + ]; + $defaultAttributes = [ + 'a' => ['href' => 'URI'], + 'span' => ['data-link' => 'URI'], + ]; + $defaultBlankElements = [ + 'figcaption', + 'legend', + ]; + $simpleConfig = [ + 'AutoFormat.Linkify' => true, + 'AutoFormat.RemoveEmpty' => true, + 'AutoFormat.RemoveEmpty.RemoveNbsp' => true, + 'HTML.Allowed' => 'a[href],strong,em,p,li,ul,ol', + ]; + $videoElements = [ + 'video' => [ + 'Block', + 'Optional: (source, Flow) | (Flow, source) | Flow', + 'Common', + [ + 'src' => 'URI', + 'type' => 'Text', + 'width' => 'Length', + 'height' => 'Length', + 'poster' => 'URI', + 'preload' => 'Enum#auto,metadata,none', + 'controls' => 'Bool', + ], ], - 'simple' => [ - 'Cache.DefinitionImpl' => null, - 'Cache.SerializerPath' => '%kernel.cache_dir%/htmlpurifier-simple', - 'AutoFormat.Linkify' => true, - 'AutoFormat.RemoveEmpty' => true, - 'AutoFormat.RemoveEmpty.RemoveNbsp' => true, - 'HTML.Allowed' => 'a[href],strong,em,p,li,ul,ol', + ]; + $advancedConfig = [ + 'Core.HiddenElements' => ['script' => true], + ]; + $allParents = ['simple', 'video', 'advanced']; + + $config = [ + 'html_profiles' => [ + 'default' => [ + 'config' => $defaultConfig, + 'attributes' => $defaultAttributes, + 'blank_elements' => $defaultBlankElements, + ], + 'simple' => [ + 'config' => $simpleConfig, + ], + 'video' => [ + 'elements' => $videoElements, + ], + 'advanced' => [ + 'config' => $advancedConfig, + ], + 'all' => [ + 'parents' => $allParents, + ], ], - 'advanced' => [ - 'Cache.DefinitionImpl' => null, + ]; + + $this->extension->load([$config], $this->container); + + $profiles = ['default', 'simple', 'video', 'advanced', 'all']; + + $this->assertDefaultConfigDefinition( + array_merge($defaultConfig, $this->defaultConfig), + $defaultAttributes, + [], + $defaultBlankElements + ); + $this->assertConfigDefinition('simple', $simpleConfig); + $this->assertConfigDefinition( + 'video', + [/* config */], + [/* parents */], + [/* attributes */], + $videoElements + ); + $this->assertConfigDefinition('advanced', $advancedConfig); + $this->assertConfigDefinition( + 'all', + [/* config */], + [$simpleConfig, /* video config is filtered */ 2 => $advancedConfig], + [/* attributes */], + [/* simple elements are filtered */ 1 => $videoElements], + [/* blank elements */] + ); + $this->assertCacheWarmerSerializerArgs([self::DEFAULT_CACHE_PATH], $profiles); + $this->assertRegistryHasProfiles($profiles); + } + + public function testShouldRegisterAliases() + { + if (!method_exists($this->container, 'registerAliasForArgument')) { + $this->markTestSkipped('Alias arguments binding is not available.'); + } + + $config = [ + 'html_profiles' => [ + 'default' => [ + 'config' => [ + 'AutoFormat.AutoParagraph' => true, + ], + ], + 'simple' => [ + 'config' => [ + 'HTML.Allowed' => 'a[href],strong,em,p,li,ul,ol', + ], + ], + 'advanced' => [ + 'config' => [ + 'Core.HiddenElements' => ['script' => true], + ], + ], ], ]; $this->extension->load([$config], $this->container); - $this->assertDefaultConfigDefinition(array_replace($this->defaultConfig, $config['default'])); - $this->assertConfigDefinition('simple', $config['simple']); - $this->assertConfigDefinition('advanced', $config['advanced']); - $this->assertCacheWarmerSerializerPaths([ - '%kernel.cache_dir%/htmlpurifier', - '%kernel.cache_dir%/htmlpurifier-simple', - ]); - $this->assertRegistryHasProfiles(['default', 'simple', 'advanced']); + $this->container->register(ServiceWithDefaultConfig::class) + ->setAutowired(true) + ->setPublic(true) + ; + $this->container->register(ServiceWithDefaultConfig2::class) + ->setAutowired(true) + ->setPublic(true) + ; + $this->container->register(ServiceWithSimpleConfig::class) + ->setAutowired(true) + ->setPublic(true) + ; + $this->container->register(ServiceWithAdvancedConfig::class) + ->setAutowired(true) + ->setPublic(true) + ; + + $this->container->compile(); + + $defaultConfigArgument1 = $this->container->findDefinition(ServiceWithDefaultConfig::class) + ->getArgument(0) + ; + + $this->assertInstanceOf(Reference::class, $defaultConfigArgument1); + $this->assertSame('exercise_html_purifier.default', (string) $defaultConfigArgument1); + + $defaultConfigArgument2 = $this->container->findDefinition(ServiceWithDefaultConfig2::class) + ->getArgument(0) + ; + + $this->assertInstanceOf(Reference::class, $defaultConfigArgument2); + $this->assertSame('exercise_html_purifier.default', (string) $defaultConfigArgument2); + + $simpleConfigArgument = $this->container->findDefinition(ServiceWithSimpleConfig::class) + ->getArgument(0) + ; + + $this->assertInstanceOf(Definition::class, $simpleConfigArgument); + $this->assertSame( + 'simple', + $simpleConfigArgument->getTag(HTMLPurifierPass::PURIFIER_TAG)[0]['profile'] ?? '' + ); + + $advancedConfigArgument = $this->container->findDefinition(ServiceWithAdvancedConfig::class) + ->getArgument(0) + ; + + $this->assertInstanceOf(Definition::class, $advancedConfigArgument); + $this->assertSame( + 'advanced', + $advancedConfigArgument->getTag(HTMLPurifierPass::PURIFIER_TAG)[0]['profile'] ?? '' + ); } /** @@ -130,30 +376,39 @@ public function testShouldLoadCustomConfiguration() * * @param string $name */ - private function assertConfigDefinition($name, array $config) + private function assertConfigDefinition($name, array $config, array $parents = [], array $attributes = [], array $elements = [], array $blankElements = []) { $this->assertTrue($this->container->hasDefinition('exercise_html_purifier.config.'.$name)); $definition = $this->container->getDefinition('exercise_html_purifier.config.'.$name); - $this->assertSame([\HTMLPurifier_Config::class, 'inherit'], $definition->getFactory()); + $this->assertEquals([new Reference(HTMLPurifierConfigFactory::class), 'create'], $definition->getFactory()); $args = $definition->getArguments(); - - $this->assertCount(1, $args); - $this->assertEquals([$config], $definition->getMethodCalls()[0][1]); + $defaultConfig = $definition->getArgument(2); + + $this->assertCount(7, $args); + $this->assertSame($name, $definition->getArgument(0)); + $this->assertSame($config, $definition->getArgument(1)); + $this->assertInstanceOf(Reference::class, $defaultConfig); + $this->assertSame('exercise_html_purifier.config.default', (string) $defaultConfig); + $this->assertSame($parents, $definition->getArgument(3)); + $this->assertSame($attributes, $definition->getArgument(4)); + $this->assertSame($elements, $definition->getArgument(5)); + $this->assertSame($blankElements, $definition->getArgument(6)); } /** * Asserts that the default config definition loads the given options. */ - private function assertDefaultConfigDefinition(array $config) + private function assertDefaultConfigDefinition(array $config, array $attributes = [], array $elements = [], array $blankElements = []): void { $this->assertTrue($this->container->hasDefinition('exercise_html_purifier.config.default')); $definition = $this->container->getDefinition('exercise_html_purifier.config.default'); - $this->assertEquals([\HTMLPurifier_Config::class, 'create'], $definition->getFactory()); - $this->assertEquals([$config], $definition->getArguments()); + + $this->assertEquals([new Reference(HTMLPurifierConfigFactory::class), 'create'], $definition->getFactory()); + $this->assertSame(['default', $config, null, [], $attributes, $elements, $blankElements], $definition->getArguments(), 'Default config is invalid.'); } /** @@ -161,35 +416,52 @@ private function assertDefaultConfigDefinition(array $config) * * @param string[] $profiles */ - private function assertRegistryHasProfiles(array $profiles) + private function assertRegistryHasProfiles(array $profiles): void { - $this->assertTrue($this->container->hasAlias(HTMLPurifiersRegistryInterface::class), 'The registry interface alias must exist.'); - - try { - $registry = $this->container->findDefinition(HTMLPurifiersRegistryInterface::class); - } catch (ServiceNotFoundException $e) { - $this->fail(sprintf('Alias %s does not target a valid id: %s.', HTMLPurifiersRegistryInterface::class, $e->getMessage())); + foreach ($profiles as $profile) { + $this->assertTrue($this->container->hasDefinition("exercise_html_purifier.$profile")); + $this->assertTrue($this->container->hasDefinition("exercise_html_purifier.config.$profile")); } + } - $this->assertSame(HTMLPurifiersRegistry::class, $registry->getClass()); + /** + * Assert that the cache warmer serializer paths equal the given array. + */ + private function assertCacheWarmerSerializerArgs(array $paths, array $profiles): void + { + $serializer = $this->container->getDefinition('exercise_html_purifier.cache_warmer.serializer'); - foreach ($profiles as $profile) { - $purifierId = "exercise_html_purifier.$profile"; + $this->assertSame($serializer->getArgument(0), $paths); + $this->assertSame($serializer->getArgument(1), $profiles); + $this->assertSame((string) $serializer->getArgument(2), HTMLPurifiersRegistryInterface::class); + $this->assertSame((string) $serializer->getArgument(3), 'filesystem'); + } +} - $this->assertTrue($this->container->has($purifierId), "The service $purifierId should be registered."); +class ServiceWithDefaultConfig +{ + public function __construct(\HTMLPurifier $purifier) + { + } +} - $tag = ['profile' => $profile]; - $purifier = $this->container->findDefinition($purifierId); +class ServiceWithDefaultConfig2 +{ + public function __construct(\HTMLPurifier $htmlPurifier) + { + } +} - $this->assertSame([HTMLPurifierPass::PURIFIER_TAG => [$tag]], $purifier->getTags()); - } +class ServiceWithSimpleConfig +{ + public function __construct(\HTMLPurifier $simplePurifier) + { } +} - /** - * Assert that the cache warmer serializer paths equal the given array. - */ - private function assertCacheWarmerSerializerPaths(array $paths) +class ServiceWithAdvancedConfig +{ + public function __construct(\HTMLPurifier $advancedPurifier) { - $this->assertEquals($paths, $this->container->getParameter('exercise_html_purifier.cache_warmer.serializer.paths')); } } diff --git a/tests/Form/Listener/HTMLPurifierListenerTest.php b/tests/Form/Listener/HTMLPurifierListenerTest.php index a712df69..14447a60 100644 --- a/tests/Form/Listener/HTMLPurifierListenerTest.php +++ b/tests/Form/Listener/HTMLPurifierListenerTest.php @@ -94,14 +94,14 @@ public function testPurifyDoNothingForEmptyOrNonScalarData($input) $listener->purifySubmittedData($event); } - public function provideInvalidInput() + public function provideInvalidInput(): iterable { yield ['']; yield [[]]; yield [new \stdClass()]; } - private function getFormEvent($data) + private function getFormEvent($data): FormEvent { return new FormEvent($this->createMock(FormInterface::class), $data); } diff --git a/tests/Form/TypeExtension/HTMLPurifierTextTypeExtensionTest.php b/tests/Form/TypeExtension/HTMLPurifierTextTypeExtensionTest.php index d8a71df2..1bddef62 100644 --- a/tests/Form/TypeExtension/HTMLPurifierTextTypeExtensionTest.php +++ b/tests/Form/TypeExtension/HTMLPurifierTextTypeExtensionTest.php @@ -5,26 +5,24 @@ use Exercise\HTMLPurifierBundle\Form\Listener\HTMLPurifierListener; use Exercise\HTMLPurifierBundle\Form\TypeExtension\HTMLPurifierTextTypeExtension; use Exercise\HTMLPurifierBundle\HTMLPurifiersRegistryInterface; -use Exercise\HTMLPurifierBundle\Tests\ForwardCompatTestTrait; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\Test\FormIntegrationTestCase; +use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; class HTMLPurifierTextTypeExtensionTest extends FormIntegrationTestCase { - use ForwardCompatTestTrait; - private $registry; - private function doSetUp() + protected function setUp(): void { $this->registry = $this->createMock(HTMLPurifiersRegistryInterface::class); parent::setUp(); } - private function doTearDown() + protected function tearDown(): void { parent::tearDown(); @@ -71,7 +69,7 @@ public function testPurifyOptionsNeedDefaultProfile() ->method('get') ; - $this->expectException('Symfony\Component\OptionsResolver\Exception\InvalidOptionsException'); + $this->expectException(InvalidOptionsException::class); $this->expectExceptionMessage('The profile "default" is not registered.'); $this->factory->create(TextType::class, null, ['purify_html' => true]); @@ -108,7 +106,7 @@ public function testInvalidProfile() ->method('get') ; - $this->expectException('Symfony\Component\OptionsResolver\Exception\InvalidOptionsException'); + $this->expectException(InvalidOptionsException::class); $this->expectExceptionMessage('The profile "test" is not registered.'); $this->factory->create(TextType::class, null, [ @@ -117,10 +115,7 @@ public function testInvalidProfile() ]); } - /** - * @return bool - */ - private function hasPurifierListener(FormInterface $form) + private function hasPurifierListener(FormInterface $form): bool { foreach ($form->getConfig()->getEventDispatcher()->getListeners(FormEvents::PRE_SUBMIT) as $listener) { if ($listener[0] instanceof HTMLPurifierListener) { diff --git a/tests/ForwardCompatTestTrait.php b/tests/ForwardCompatTestTrait.php deleted file mode 100644 index 3870ed3f..00000000 --- a/tests/ForwardCompatTestTrait.php +++ /dev/null @@ -1,72 +0,0 @@ -hasReturnType()) { - eval(' - namespace Exercise\HTMLPurifierBundle\Tests; - - /** - * @internal - */ - trait ForwardCompatTestTrait - { - private function doSetUp(): void - { - } - - private function doTearDown(): void - { - } - - protected function setUp(): void - { - $this->doSetUp(); - } - protected function tearDown(): void - { - $this->doTearDown(); - } - } -'); -} else { - /** - * @internal - */ - trait ForwardCompatTestTrait - { - /** - * @return void - */ - private function doSetUp() - { - } - - /** - * @return void - */ - private function doTearDown() - { - } - - /** - * @return void - */ - protected function setUp() - { - $this->doSetUp(); - } - - /** - * @return void - */ - protected function tearDown() - { - $this->doTearDown(); - } - } -} diff --git a/tests/HTMLPurifiersRegistryTest.php b/tests/HTMLPurifiersRegistryTest.php index 378e73d8..d58d6df0 100644 --- a/tests/HTMLPurifiersRegistryTest.php +++ b/tests/HTMLPurifiersRegistryTest.php @@ -8,24 +8,22 @@ class HTMLPurifiersRegistryTest extends TestCase { - use ForwardCompatTestTrait; - private $locator; private $registry; - private function doSetUp() + protected function setUp(): void { $this->locator = $this->createMock(ContainerInterface::class); $this->registry = new HTMLPurifiersRegistry($this->locator); } - private function doTearDown() + protected function tearDown(): void { $this->registry = null; $this->locator = null; } - public function provideProfiles() + public function provideProfiles(): iterable { yield ['default']; yield ['test']; diff --git a/tests/Twig/HTMLPurifierRuntimeTest.php b/tests/Twig/HTMLPurifierRuntimeTest.php index 7910e491..e6b03349 100644 --- a/tests/Twig/HTMLPurifierRuntimeTest.php +++ b/tests/Twig/HTMLPurifierRuntimeTest.php @@ -38,7 +38,7 @@ public function testPurifyFilter($profile) $this->assertEquals($purifiedInput, $extension->purify($input, $profile)); } - public function providePurifierProfiles() + public function providePurifierProfiles(): iterable { yield ['default']; yield ['custom'];