Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -233,5 +234,9 @@

->set('serializer.normalizer.number', NumberNormalizer::class)
->tag('serializer.normalizer', ['built_in' => true, 'priority' => -915])

->set('serializer.denormalizer.sanitize', SanitizeDenormalizer::class)
Copy link
Member

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).

->args([service('service_container')])
Copy link
Member

Choose a reason for hiding this comment

The 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
Expand Up @@ -89,6 +89,7 @@
use Symfony\Component\Serializer\Normalizer\FormErrorNormalizer;
use Symfony\Component\Serializer\Normalizer\JsonSerializableNormalizer;
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
use Symfony\Component\Serializer\Normalizer\SanitizeDenormalizer;
use Symfony\Component\Serializer\Normalizer\TranslatableNormalizer;
use Symfony\Component\Serializer\Serializer;
use Symfony\Component\Translation\DependencyInjection\TranslatorPass;
Expand Down Expand Up @@ -2961,4 +2962,17 @@ private function assertCachePoolServiceDefinitionIsCreated(ContainerBuilder $con
default => $this->fail('Unresolved adapter: '.$adapter),
};
}

public function testSanitizeDenormalizerRegistered()
{
$container = $this->createContainerFromFile('full');

$definition = $container->getDefinition('serializer.denormalizer.sanitize');
$argument = $definition->getArgument(0);
$tag = $definition->getTag('serializer.normalizer');

$this->assertSame(SanitizeDenormalizer::class, $definition->getClass());
$this->assertSame('service_container', $argument->__toString());
$this->assertSame(-800, $tag[0]['priority']);
}
}
1 change: 1 addition & 0 deletions src/Symfony/Component/Serializer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ CHANGELOG
* Deprecate class aliases in the `Annotation` namespace, use attributes instead
* Deprecate getters in attribute classes in favor of public properties
* Deprecate `ClassMetadataFactoryCompiler`
* Add `SanitizeDenormalizer` to automatically sanitize string and array of string properties during deserialization.

7.3
---
Expand Down
131 changes: 131 additions & 0 deletions src/Symfony/Component/Serializer/Normalizer/SanitizeDenormalizer.php
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;

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])) {
Copy link
Member

Choose a reason for hiding this comment

The 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...).
This is again something that would be solved automatically once you stop processing the object-level data in that normalizer in favor of making it a proper scalar normalizer.

continue;
}

foreach ($property->getAttributes(Context::class) as $attribute) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not make any sense. You should read $context, not read attributes again on your own (this defeats all the benefits I listed in my previous review related to caching metadata or supporting all metadata formats)

$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)) {
Copy link
Member

Choose a reason for hiding this comment

The 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,
) {}
}
Loading
Loading