Skip to content

Commit

Permalink
feature #46335 [Form][FrameworkBundle][TwigBundle] Add Twig filter, f…
Browse files Browse the repository at this point in the history
…orm-type extension and improve service definitions for HtmlSanitizer (nicolas-grekas)

This PR was merged into the 6.1 branch.

Discussion
----------

[Form][FrameworkBundle][TwigBundle] Add Twig filter, form-type extension and improve service definitions for HtmlSanitizer

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This PR borrows the twig filter and the form-type extension from https://github.com/tgalopin/html-sanitizer-bundle/

Given the form builder (the input), it allows doing:
```php
$builder
    ->add('content', TextareaType::class, ['sanitize_html' => true])
;
```

And on the template side (the output), it allows doing:
```jinja
<div>
    {{ html|sanitize_html }}
</div>
```

In order to be able to wire the corresponding services, I had to change the way html-sanitizer is wired by framework-bundle:
What we need here is a default name that we can rely on. By making the default name configurable, the current way to configure html_sanitizer makes it hard for other bundles (twig-bundle here) to know what the default name is. Eg here I would have to make the bundle read the config settings of framework-bundle. I solved this issue by making the default name non-configurable - aka I'm creating a convention: the default name is "default", and this "default" sanitizer is the one aliased to the html_sanitizer service, or to the non-named autowiring alias.

I'm submitting this PR to 6.1 because html-sanitizer is new. If we prefer merging this in 6.2, the updated service wiring must be merged into 6.1 at least.

Commits
-------

0ea89e5 [FrameworkBundle][TwigBundle][Form] Add Twig filter, form-type extension and improve service definitions for HtmlSanitizer
  • Loading branch information
fabpot committed May 14, 2022
2 parents 41c8f5c + 0ea89e5 commit baa367e
Show file tree
Hide file tree
Showing 21 changed files with 312 additions and 43 deletions.
40 changes: 40 additions & 0 deletions src/Symfony/Bridge/Twig/Extension/HtmlSanitizerExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Twig\Extension;

use Psr\Container\ContainerInterface;
use Twig\Extension\AbstractExtension;
use Twig\TwigFilter;

/**
* @author Titouan Galopin <galopintitouan@gmail.com>
*/
final class HtmlSanitizerExtension extends AbstractExtension
{
public function __construct(
private ContainerInterface $sanitizers,
private string $defaultSanitizer = 'default',
) {
}

public function getFilters(): array
{
return [
new TwigFilter('sanitize_html', $this->sanitize(...), ['is_safe' => ['html']]),
];
}

public function sanitize(string $html, string $sanitizer = null): string
{
return $this->sanitizers->get($sanitizer ?? $this->defaultSanitizer)->sanitize($html);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Twig\Tests\Extension;

use PHPUnit\Framework\TestCase;
use Symfony\Bridge\Twig\Extension\HtmlSanitizerExtension;
use Symfony\Component\DependencyInjection\ServiceLocator;
use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface;
use Twig\Environment;
use Twig\Loader\ArrayLoader;

class HtmlSanitizerExtensionTest extends TestCase
{
public function testSanitizeHtml()
{
$loader = new ArrayLoader([
'foo' => '{{ "foobar"|sanitize_html }}',
'bar' => '{{ "foobar"|sanitize_html("bar") }}',
]);

$twig = new Environment($loader, ['debug' => true, 'cache' => false, 'autoescape' => 'html', 'optimizations' => 0]);

$fooSanitizer = $this->createMock(HtmlSanitizerInterface::class);
$fooSanitizer->expects($this->once())
->method('sanitize')
->with('foobar')
->willReturn('foo');

$barSanitizer = $this->createMock(HtmlSanitizerInterface::class);
$barSanitizer->expects($this->once())
->method('sanitize')
->with('foobar')
->willReturn('bar');

$twig->addExtension(new HtmlSanitizerExtension(new ServiceLocator([
'foo' => fn () => $fooSanitizer,
'bar' => fn () => $barSanitizer,
]), 'foo'));

$this->assertSame('foo', $twig->render('foo'));
$this->assertSame('bar', $twig->render('bar'));
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Bridge/Twig/UndefinedCallableHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class UndefinedCallableHandler
private const FILTER_COMPONENTS = [
'humanize' => 'form',
'trans' => 'translation',
'sanitize_html' => 'html-sanitizer',
'yaml_encode' => 'yaml',
'yaml_dump' => 'yaml',
];
Expand Down Expand Up @@ -61,6 +62,7 @@ class UndefinedCallableHandler
];

private const FULL_STACK_ENABLE = [
'html-sanitizer' => 'enable "framework.html_sanitizer"',
'form' => 'enable "framework.form"',
'security-core' => 'add the "SecurityBundle"',
'security-http' => 'add the "SecurityBundle"',
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Bridge/Twig/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"symfony/dependency-injection": "^5.4|^6.0",
"symfony/finder": "^5.4|^6.0",
"symfony/form": "^6.1",
"symfony/html-sanitizer": "^6.1",
"symfony/http-foundation": "^5.4|^6.0",
"symfony/http-kernel": "^5.4|^6.0",
"symfony/intl": "^5.4|^6.0",
Expand Down Expand Up @@ -65,6 +66,7 @@
"symfony/finder": "",
"symfony/asset": "For using the AssetExtension",
"symfony/form": "For using the FormExtension",
"symfony/html-sanitizer": "For using the HtmlSanitizerExtension",
"symfony/http-kernel": "For using the HttpKernelExtension",
"symfony/routing": "For using the RoutingExtension",
"symfony/translation": "For using the TranslationExtension",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class UnusedTagsPass implements CompilerPassInterface
'form.type',
'form.type_extension',
'form.type_guesser',
'html_sanitizer',
'http_client.client',
'kernel.cache_clearer',
'kernel.cache_warmer',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2129,10 +2129,6 @@ private function addHtmlSanitizerSection(ArrayNodeDefinition $rootNode, callable
->{$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')
->useAttributeAsKey('name')
->arrayPrototype()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
use Symfony\Component\Finder\Finder;
use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator;
use Symfony\Component\Form\Extension\HtmlSanitizer\Type\TextTypeHtmlSanitizerExtension;
use Symfony\Component\Form\Form;
use Symfony\Component\Form\FormTypeExtensionInterface;
use Symfony\Component\Form\FormTypeGuesserInterface;
Expand Down Expand Up @@ -485,6 +486,9 @@ public function load(array $configs, ContainerBuilder $container)
$container->removeDefinition('form.type_extension.form.validator');
$container->removeDefinition('form.type_guesser.validator');
}
if (!$this->isConfigEnabled($container, $config['html_sanitizer']) || !class_exists(TextTypeHtmlSanitizerExtension::class)) {
$container->removeDefinition('form.type_extension.form.html_sanitizer');
}
} else {
$container->removeDefinition('console.command.form_debug');
}
Expand Down Expand Up @@ -2740,13 +2744,14 @@ private function registerHtmlSanitizerConfiguration(array $config, ContainerBuil

// Create the sanitizer and link its config
$sanitizerId = 'html_sanitizer.sanitizer.'.$sanitizerName;
$container->register($sanitizerId, HtmlSanitizer::class)->addArgument(new Reference($configId));
$container->register($sanitizerId, HtmlSanitizer::class)
->addTag('html_sanitizer', ['sanitizer' => $sanitizerName])
->addArgument(new Reference($configId));

$container->registerAliasForArgument($sanitizerId, HtmlSanitizerInterface::class, $sanitizerName);
if ('default' !== $sanitizerName) {
$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
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/form.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
use Symfony\Component\Form\Extension\Core\Type\FileType;
use Symfony\Component\Form\Extension\Core\Type\FormType;
use Symfony\Component\Form\Extension\Core\Type\SubmitType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\Extension\Core\Type\TransformationFailureExtension;
use Symfony\Component\Form\Extension\DependencyInjection\DependencyInjectionExtension;
use Symfony\Component\Form\Extension\HtmlSanitizer\Type\TextTypeHtmlSanitizerExtension;
use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationRequestHandler;
use Symfony\Component\Form\Extension\HttpFoundation\Type\FormTypeHttpFoundationExtension;
use Symfony\Component\Form\Extension\Validator\Type\FormTypeValidatorExtension;
Expand Down Expand Up @@ -113,6 +115,10 @@
->args([service('translator')->ignoreOnInvalid()])
->tag('form.type_extension', ['extended-type' => FormType::class])

->set('form.type_extension.form.html_sanitizer', TextTypeHtmlSanitizerExtension::class)
->args([tagged_locator('html_sanitizer', 'sanitizer')])
->tag('form.type_extension', ['extended-type' => TextType::class])

->set('form.type_extension.form.http_foundation', FormTypeHttpFoundationExtension::class)
->args([service('form.type_extension.form.request_handler')])
->tag('form.type_extension')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,18 @@

use Symfony\Component\HtmlSanitizer\HtmlSanitizer;
use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig;
use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface;

return static function (ContainerConfigurator $container) {
$container->services()
->set('html_sanitizer.config', HtmlSanitizerConfig::class)
->set('html_sanitizer.config.default', HtmlSanitizerConfig::class)
->call('allowSafeElements')

->set('html_sanitizer', HtmlSanitizer::class)
->set('html_sanitizer.sanitizer.default', HtmlSanitizer::class)
->args([service('html_sanitizer.config')])
->tag('html_sanitizer', ['name' => 'default'])

->alias('html_sanitizer', 'html_sanitizer.sanitizer.default')
->alias(HtmlSanitizerInterface::class, 'html_sanitizer')
;
};
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,6 @@
<xsd:element name="sanitizer" type="sanitizer" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
<xsd:attribute name="enabled" type="xsd:boolean" />
<xsd:attribute name="default" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="sanitizer">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,6 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor
],
'html_sanitizer' => [
'enabled' => !class_exists(FullStack::class) && class_exists(HtmlSanitizer::class),
'default' => null,
'sanitizers' => [],
],
'exceptions' => [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
$container->loadFromExtension('framework', [
'http_method_override' => false,
'html_sanitizer' => [
'default' => 'my.sanitizer',
'sanitizers' => [
'my.sanitizer' => [
'default' => [
'allow_safe_elements' => true,
'allow_all_static_elements' => true,
'allow_elements' => [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

<config xmlns="http://symfony.com/schema/dic/symfony" http-method-override="false">
<html-sanitizer default="my.sanitizer">
<sanitizer name="my.sanitizer"
<html-sanitizer>
<sanitizer name="default"
allow-safe-elements="true"
allow-all-static-elements="true"
force-https-urls="true"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
framework:
http_method_override: false
html_sanitizer:
default: my.sanitizer
sanitizers:
my.sanitizer:
default:
allow_safe_elements: true
allow_all_static_elements: true
allow_elements:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2030,27 +2030,16 @@ public function testHtmlSanitizer()
$container = $this->createContainerFromFile('html_sanitizer');

// 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->assertTrue($container->hasAlias('html_sanitizer'), '->registerHtmlSanitizerConfiguration() loads html_sanitizer.php');
$this->assertSame('html_sanitizer.sanitizer.default', (string) $container->getAlias('html_sanitizer'));
$this->assertSame(HtmlSanitizer::class, $container->getDefinition('html_sanitizer.sanitizer.default')->getClass());
$this->assertCount(1, $args = $container->getDefinition('html_sanitizer.sanitizer.default')->getArguments());
$this->assertSame('html_sanitizer.config.default', (string) $args[0]);

// config
$this->assertTrue($container->hasDefinition('html_sanitizer.config.default'), '->registerHtmlSanitizerConfiguration() loads custom sanitizer');
$this->assertSame(HtmlSanitizerConfig::class, $container->getDefinition('html_sanitizer.config.default')->getClass());
$this->assertCount(23, $calls = $container->getDefinition('html_sanitizer.config.default')->getMethodCalls());
$this->assertSame(
[
['allowSafeElements', [], true],
Expand Down Expand Up @@ -2092,11 +2081,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.all.sanitizer', (string) $container->getAlias(HtmlSanitizerInterface::class.' $allSanitizer'), '->registerHtmlSanitizerConfiguration() creates appropriate named alias');
$this->assertSame('html_sanitizer.sanitizer.all.sanitizer', (string) $container->getAlias(HtmlSanitizerInterface::class.' $allSanitizer'));
$this->assertFalse($container->hasAlias(HtmlSanitizerInterface::class.' $default'));

// Default alias
$this->assertSame('html_sanitizer.sanitizer.my.sanitizer', (string) $container->getAlias(HtmlSanitizerInterface::class), '->registerHtmlSanitizerConfiguration() creates appropriate default alias');
$this->assertSame('html_sanitizer', (string) $container->getAlias(HtmlSanitizerInterface::class));
}

protected function createContainer(array $data = [])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Form\AbstractRendererEngine;
use Symfony\Component\Form\Form;
use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\Mailer\Mailer;
use Symfony\Component\Translation\Translator;
Expand Down Expand Up @@ -54,6 +55,10 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('console.php');
}

if (!$container::willBeAvailable('symfony/html-sanitizer', HtmlSanitizerInterface::class, ['symfony/twig-bundle'])) {
$container->removeDefinition('twig.extension.htmlsanitizer');
}

if ($container::willBeAvailable('symfony/mailer', Mailer::class, ['symfony/twig-bundle'])) {
$loader->load('mailer.php');
}
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Bundle/TwigBundle/Resources/config/twig.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Symfony\Bridge\Twig\Extension\AssetExtension;
use Symfony\Bridge\Twig\Extension\CodeExtension;
use Symfony\Bridge\Twig\Extension\ExpressionExtension;
use Symfony\Bridge\Twig\Extension\HtmlSanitizerExtension;
use Symfony\Bridge\Twig\Extension\HttpFoundationExtension;
use Symfony\Bridge\Twig\Extension\HttpKernelExtension;
use Symfony\Bridge\Twig\Extension\HttpKernelRuntime;
Expand Down Expand Up @@ -118,6 +119,9 @@

->set('twig.extension.expression', ExpressionExtension::class)

->set('twig.extension.htmlsanitizer', HtmlSanitizerExtension::class)
->args([tagged_locator('html_sanitizer', 'sanitizer')])

->set('twig.extension.httpkernel', HttpKernelExtension::class)

->set('twig.runtime.httpkernel', HttpKernelRuntime::class)
Expand Down
Loading

0 comments on commit baa367e

Please sign in to comment.