-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Serializer] Add SanitizeDenormalizer for automatic sanitization #62117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 8.1
Are you sure you want to change the base?
Changes from all commits
3f33854
8b014cd
f41a2e5
288ac9e
9887ad6
b25d115
7edab34
61f49fa
32e14c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,7 @@ | |
| use Symfony\Component\Serializer\Normalizer\ObjectNormalizer; | ||
| use Symfony\Component\Serializer\Normalizer\ProblemNormalizer; | ||
| use Symfony\Component\Serializer\Normalizer\PropertyNormalizer; | ||
| use Symfony\Component\Serializer\Normalizer\SanitizeDenormalizer; | ||
| use Symfony\Component\Serializer\Normalizer\TranslatableNormalizer; | ||
| use Symfony\Component\Serializer\Normalizer\UidNormalizer; | ||
| use Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer; | ||
|
|
@@ -233,5 +234,9 @@ | |
|
|
||
| ->set('serializer.normalizer.number', NumberNormalizer::class) | ||
| ->tag('serializer.normalizer', ['built_in' => true, 'priority' => -915]) | ||
|
|
||
| ->set('serializer.denormalizer.sanitize', SanitizeDenormalizer::class) | ||
| ->args([service('service_container')]) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. injecting the main DI container is a no-go. It should inject a service locator with the html sanitizer services instead (the main DI container won't be able to get those services anyway as they are not public) |
||
| ->tag('serializer.normalizer', ['built_in' => true, 'priority' => -800]) | ||
| ; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| <?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\Component\Serializer\Normalizer; | ||
|
|
||
| use Psr\Container\ContainerInterface; | ||
| use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface; | ||
| use Symfony\Component\Serializer\Attribute\Context; | ||
| use Symfony\Component\Serializer\Exception\BadMethodCallException; | ||
| use Symfony\Component\Serializer\Exception\LogicException; | ||
|
|
||
| /** | ||
| * Denormalizer that sanitizes string properties based on the Context attribute. | ||
| * | ||
| * @author Mohamed Senoussi <lesfootix@gmail.com> | ||
| */ | ||
| final class SanitizeDenormalizer implements DenormalizerInterface, DenormalizerAwareInterface | ||
| { | ||
| use DenormalizerAwareTrait; | ||
momito69 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| public const SANITIZER = 'sanitizer'; | ||
| public const SANITIZE_HTML = 'sanitize_html'; | ||
| public const DEFAULT_SANITIZER = 'default'; | ||
|
|
||
| public function __construct( | ||
| private ContainerInterface $sanitizers, | ||
| ) { | ||
| } | ||
|
|
||
| public function getSupportedTypes(?string $format): array | ||
| { | ||
| return ['native-string' => false, 'native-array' => false, '*' => false]; | ||
| } | ||
|
|
||
| public function denormalize(mixed $data, string $type, ?string $format = null, array $context = []): mixed | ||
| { | ||
| if (!isset($this->denormalizer)) { | ||
| throw new BadMethodCallException('Please set a denormalizer before calling denormalize()!'); | ||
| } | ||
|
|
||
| if (!\is_array($data)) { | ||
| return $this->denormalizer->denormalize($data, $type, $format, $context); | ||
| } | ||
|
|
||
| $reflection = new \ReflectionClass($type); | ||
| $sanitizedData = $data; | ||
|
|
||
| foreach ($reflection->getProperties() as $property) { | ||
| $name = $property->getName(); | ||
| if (!isset($data[$name])) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is broken as it forgets about the support of many advanced features of the ObjectNormalizer (name converters, serialized names, etc...). |
||
| continue; | ||
| } | ||
|
|
||
| foreach ($property->getAttributes(Context::class) as $attribute) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not make any sense. You should read |
||
| $attributeContext = $attribute->newInstance(); | ||
| if (($attributeContext->denormalizationContext[self::SANITIZE_HTML] ?? false) !== true) { | ||
| continue; | ||
| } | ||
| $sanitizer = $this->getSanitizer($attributeContext->denormalizationContext[self::SANITIZER] ?? self::DEFAULT_SANITIZER); | ||
| $sanitizedData[$name] = $this->sanitizeValue($data[$name], $sanitizer, $name); | ||
| $context['sanitized'] = true; | ||
| } | ||
| } | ||
|
|
||
| return $this->denormalizer->denormalize($sanitizedData, $type, $format, $context); | ||
| } | ||
|
|
||
| public function supportsDenormalization(mixed $data, string $type, ?string $format = null, array $context = []): bool | ||
| { | ||
| if (!isset($this->denormalizer)) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!class_exists($type)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my previous review, I asked for this serializer to be a serializer applied on the string, not applied at the object level. |
||
| return false; | ||
| } | ||
|
|
||
| if (isset($context['sanitized'])) { | ||
| return false; | ||
| } | ||
|
|
||
| $reflection = new \ReflectionClass($type); | ||
| foreach ($reflection->getProperties() as $property) { | ||
| foreach ($property->getAttributes(Context::class) as $attribute) { | ||
| $attributeContext = $attribute->newInstance(); | ||
| if (($attributeContext->denormalizationContext[self::SANITIZE_HTML] ?? false) === true) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private function sanitizeValue(mixed $value, HtmlSanitizerInterface $sanitizer, string $propertyName): mixed | ||
| { | ||
| return match (true) { | ||
| \is_string($value) => $sanitizer->sanitize($value), | ||
| \is_array($value) => array_map(static function ($v) use ($sanitizer, $propertyName) { | ||
| if (!\is_string($v)) { | ||
| throw new LogicException(\sprintf('Cannot sanitize property "%s". Only string items are supported.', $propertyName)); | ||
| } | ||
|
|
||
| return $sanitizer->sanitize($v); | ||
| }, $value), | ||
| default => throw new LogicException(\sprintf('Cannot sanitize property "%s". Only string or array of strings are supported.', $propertyName)), | ||
| }; | ||
| } | ||
|
|
||
| private function getSanitizer(string $name): HtmlSanitizerInterface | ||
| { | ||
| if (!$this->sanitizers->has($name)) { | ||
| throw new LogicException(\sprintf('Sanitizer "%s" is not defined in the container.', $name)); | ||
| } | ||
|
|
||
| $sanitizer = $this->sanitizers->get($name); | ||
| if (!$sanitizer instanceof HtmlSanitizerInterface) { | ||
| throw new LogicException(\sprintf('Sanitizer "%s" must implement HtmlSanitizerInterface.', $name)); | ||
| } | ||
|
|
||
| return $sanitizer; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <?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\Component\Serializer\Tests\Fixtures\Attributes; | ||
|
|
||
| use Symfony\Component\Serializer\Attribute\Context; | ||
| use Symfony\Component\Serializer\Normalizer\SanitizeDenormalizer; | ||
|
|
||
| /** | ||
| * @author Mohamed Senoussi <lesfootix@gmail.com> | ||
| */ | ||
| class SanitizeDummy | ||
| { | ||
| public function __construct( | ||
| public string $id, | ||
| public string $firstName, | ||
| public string $lastName, | ||
| #[Context( | ||
| denormalizationContext: [ SanitizeDenormalizer::SANITIZE_HTML => true ], | ||
| )] | ||
| public string $bio | ||
| ) {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <?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\Component\Serializer\Tests\Fixtures\Attributes; | ||
|
|
||
| /** | ||
| * @author Mohamed Senoussi <lesfootix@gmail.com> | ||
| */ | ||
| class SanitizeDummyWithArrayOfObject | ||
| { | ||
| public function __construct( | ||
| public string $id, | ||
| /** @var SanitizeDummy[] */ | ||
| public array $objects, | ||
| ) {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| <?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\Component\Serializer\Tests\Fixtures\Attributes; | ||
|
|
||
| use Symfony\Component\Serializer\Attribute\Context; | ||
| use Symfony\Component\Serializer\Normalizer\SanitizeDenormalizer; | ||
|
|
||
| /** | ||
| * @author Mohamed Senoussi <lesfootix@gmail.com> | ||
| */ | ||
| class SanitizeDummyWithArrayOfString | ||
| { | ||
| public function __construct( | ||
| public string $id, | ||
| /** @var string[] */ | ||
| #[Context(denormalizationContext: [ SanitizeDenormalizer::SANITIZE_HTML => true ])] | ||
| public array $strings, | ||
| ) {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| <?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\Component\Serializer\Tests\Fixtures\Attributes; | ||
|
|
||
| use Symfony\Component\Serializer\Attribute\Context; | ||
| use Symfony\Component\Serializer\Normalizer\SanitizeDenormalizer; | ||
|
|
||
| /** | ||
| * @author Mohamed Senoussi <lesfootix@gmail.com> | ||
| */ | ||
| class SanitizeDummyWithCustomSanitizer | ||
| { | ||
| public function __construct( | ||
| #[Context(denormalizationContext: [ SanitizeDenormalizer::SANITIZE_HTML => true, SanitizeDenormalizer::SANITIZER => 'custom' ])] | ||
| public string $foo | ||
| ) {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| <?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\Component\Serializer\Tests\Fixtures\Attributes; | ||
|
|
||
| use Symfony\Component\Serializer\Attribute\Context; | ||
| use Symfony\Component\Serializer\Normalizer\SanitizeDenormalizer; | ||
|
|
||
| /** | ||
| * @author Mohamed Senoussi <lesfootix@gmail.com> | ||
| */ | ||
| class SanitizeDummyWithInvalidArray | ||
| { | ||
| public function __construct( | ||
| /** | ||
| * @var int[] | ||
| */ | ||
| #[Context(denormalizationContext: [ SanitizeDenormalizer::SANITIZE_HTML => true ])] | ||
| public array $foo | ||
| ) {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| <?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\Component\Serializer\Tests\Fixtures\Attributes; | ||
|
|
||
| use Symfony\Component\Serializer\Attribute\Context; | ||
| use Symfony\Component\Serializer\Normalizer\SanitizeDenormalizer; | ||
|
|
||
| /** | ||
| * @author Mohamed Senoussi <lesfootix@gmail.com> | ||
| */ | ||
| class SanitizeDummyWithInvalidType | ||
| { | ||
| public function __construct( | ||
| #[Context(denormalizationContext: [ SanitizeDenormalizer::SANITIZE_HTML => true ])] | ||
| public int $foo | ||
| ) {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?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\Component\Serializer\Tests\Fixtures\Attributes; | ||
|
|
||
| /** | ||
| * @author Mohamed Senoussi <lesfootix@gmail.com> | ||
| */ | ||
| class SanitizeDummyWithObject | ||
| { | ||
| public function __construct( | ||
| public string $id, | ||
| public SanitizeDummy $object, | ||
| ) {} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this denormalizer needs to be removed from the config when the html-sanitizer component is not enabled (see how we handle other cross-component features in FrameworkExtension).