[FrameworkBundle] Allow default action in configuration #57653
Conversation
|
@xabbuh : done. However I don't know how to update the test in FrameworkExtensionTest to try with sanitizer <7.2 & >=7.2. |
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
|
I don't know where and how to fix those errors (related to xml structure). If anyone can point me in the right direction 🙏 |
|
@Neirda24 In the |
| @@ -2382,6 +2383,10 @@ private function addHtmlSanitizerSection(ArrayNodeDefinition $rootNode, callable | |||
| ->fixXmlConfig('with_attribute_sanitizer') | |||
| ->fixXmlConfig('without_attribute_sanitizer') | |||
| ->children() | |||
| ->enumNode('default_action') | |||
| ->info('Defines how the sanitizer must behave by default.') | |||
| ->values(array_map(static fn (HtmlSanitizerAction $action): string => $action->value, HtmlSanitizerAction::cases())) | |||
There was a problem hiding this comment.
| ->values(array_map(static fn (HtmlSanitizerAction $action): string => $action->value, HtmlSanitizerAction::cases())) | |
| ->values(array_column(HtmlSanitizerAction::cases(), 'value')) |
There was a problem hiding this comment.
but this won't work if the component is not installed, isn't it?
There was a problem hiding this comment.
Yes good point. Just came here to say #57686 might allow this to be cleaner, but indeed if the enum class is missing what do we do?
There was a problem hiding this comment.
Ok so now that #57686 is merged, I would say this could probably be changed to:
->enumNode('default_action')->enumFqcn(HtmlSanitizerAction::class)->end()
The question of what to do when !class_exists(HtmlSanitizerAction::class).. I would say make the enum node conditional, and only declare it if the class exists, so that if html-sanitizer is outdated and you try to configure a default_action the config will error with an unknown option default_action. For better DX we could also if the enum class is not there define it as a dummy null value but throw with a message about html-sanitizer bieng outdated if any value is passed in.
| @@ -3006,6 +3007,17 @@ private function registerHtmlSanitizerConfiguration(array $config, ContainerBuil | |||
| $def = $container->register($configId, HtmlSanitizerConfig::class); | |||
|
|
|||
| // Base | |||
| if ($sanitizerConfig['default_action'] ?? false) { | |||
| if (!class_exists(HtmlSanitizerAction::class)) { | |||
There was a problem hiding this comment.
this is dead code because of the way the config is done at the moment: if there is a value, it must be one of the allowed cases, which are derived from the enum
| @@ -918,6 +918,7 @@ | |||
| <xsd:attribute name="allow-relative-links" type="xsd:boolean" /> | |||
| <xsd:attribute name="allow-relative-medias" type="xsd:boolean" /> | |||
| <xsd:attribute name="max-input-length" type="xsd:positiveInteger" /> | |||
| <xsd:attribute name="default-action" type="xsd:string" /> | |||
There was a problem hiding this comment.
can't we restrict the possible values to the ones allowed by the enum?
|
What's the status of this PR? |
See symfony/symfony-docs#20019 for documentation.