bug #59134 [HttpKernel] Denormalize request data using the csv format when using "#[MapQueryString]" or "#[MapRequestPayload]" (except for content data) (ovidiuenache)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[HttpKernel] Denormalize request data using the csv format when using "#[MapQueryString]" or "#[MapRequestPayload]" (except for content data)

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | #59104 #50904
| License       | MIT

This fixes the scenario mentioned above where using `#[MapQueryString]` or `#[MapRequestPayload]` (except request content) with at least one readonly property with a type mismatch error leads to getting an object with uninitialized properties. The only properties that are set are the ones that are assigned a value via a request parameter and are NOT readonly. Moreover, if the corresponding query parameter is not provided for non-readonly property A and there is at least one other readonly property B that has a type mismatch then A will still be returned as uninitialized (even if it has a default value).

Shortly put, the instantiation fails and the values of the properties cannot be set later on.

Examples
```
class FilterDto {
    public function __construct(
        public readonly ?int $id = null,
        public readonly ?string $name = null,
        public ?string $description = null,
    ) {
    }
}

GET https://127.0.0.1:8000/test?id=x&name=test
id: ? ?int
name: ? ?string
description: ? ?string

GET https://127.0.0.1:8000/test?id=x&name=test&description=desc
id: ? ?int
name: ? ?string
description: "desc"
```

The complete list of steps to reproduce this is provided in #59104.

The reason why this happens is because we are disabling the type enforcement of the denormalizer in the `Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestPayloadValueResolver` class and when we eventually end up in the `validateAndDenormalize` method of the `Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer` class we ignore the type mismatch because of:
```
if ($context[self::DISABLE_TYPE_ENFORCEMENT] ?? $this->defaultContext[self::DISABLE_TYPE_ENFORCEMENT] ?? false) {
    return $data;
}
```

Thus, we get a type error when trying to create the object and we fall back to
`$reflectionClass->newInstanceWithoutConstructor();`

Then, based on the provided request data, we attempt to set the values of the properties but this process fails for the readonly properties so they stay uninitialized.

As discussed with `@nicolas`-grekas during [the hackathon at SymfonyCon Vienna 2024](https://live.symfony.com/2024-vienna-con/) the solution here is to stop disabling the type enforcement of the denormalizer. However, this alone is not enough because then we won't be able to use anything but string since this is the type that comes in the request so we also need to set the denormalization format to either `csv` or `xml`.

This comment from `the Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer` sheds some light on why:
```
// In XML and CSV all basic datatypes are represented as strings, it is e.g. not possible to determine,
// if a value is meant to be a string, float, int or a boolean value from the serialized representation.
// That's why we have to transform the values, if one of these non-string basic datatypes is expected.
```

We avoid `xml` due to some special formatting that occurs so the proposed solution uses `csv`.

Basically, we start using type enforcement + csv format where non-string values are transformed.

Commits
-------

e0957a0b33b [HttpKernel] Denormalize request data using the csv format when using "#[MapQueryString]" or "#[MapRequestPayload]" (except for content data)

Author: fabpot

Controller/ArgumentResolver/RequestPayloadValueResolver.php

@@ -40,11 +40,9 @@
 class RequestPayloadValueResolver implements ValueResolverInterface, EventSubscriberInterface
 {
     /**
-     * @see \Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT
      * @see DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS
      */
     private const CONTEXT_DENORMALIZE = [
-        'disable_type_enforcement' => true,
         'collect_denormalization_errors' => true,
     ];
 
@@ -165,7 +163,7 @@ private function mapQueryString(Request $request, string $type, MapQueryString $
             return null;
         }
 
-        return $this->serializer->denormalize($data, $type, null, $attribute->serializationContext + self::CONTEXT_DENORMALIZE);
+        return $this->serializer->denormalize($data, $type, 'csv', $attribute->serializationContext + self::CONTEXT_DENORMALIZE);
     }
 
     private function mapRequestPayload(Request $request, string $type, MapRequestPayload $attribute): ?object
@@ -179,7 +177,7 @@ private function mapRequestPayload(Request $request, string $type, MapRequestPay
         }
 
         if ($data = $request->request->all()) {
-            return $this->serializer->denormalize($data, $type, null, $attribute->serializationContext + self::CONTEXT_DENORMALIZE);
+            return $this->serializer->denormalize($data, $type, 'csv', $attribute->serializationContext + self::CONTEXT_DENORMALIZE);
         }
 
         if ('' === $data = $request->getContent()) {

Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php

@@ -22,6 +22,7 @@
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\PropertyAccess\Exception\InvalidTypeException;
+use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
 use Symfony\Component\Serializer\Encoder\JsonEncoder;
 use Symfony\Component\Serializer\Encoder\XmlEncoder;
 use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
@@ -394,6 +395,38 @@ public function testQueryStringValidationPassed()
         $this->assertEquals([$payload], $event->getArguments());
     }
 
+    public function testQueryStringParameterTypeMismatch()
+    {
+        $query = ['price' => 'not a float'];
+
+        $normalizer = new ObjectNormalizer(null, null, null, new ReflectionExtractor());
+        $serializer = new Serializer([$normalizer], ['json' => new JsonEncoder()]);
+
+        $validator = $this->createMock(ValidatorInterface::class);
+        $validator->expects($this->never())->method('validate');
+
+        $resolver = new RequestPayloadValueResolver($serializer, $validator);
+
+        $argument = new ArgumentMetadata('invalid', RequestPayload::class, false, false, null, false, [
+            MapQueryString::class => new MapQueryString(),
+        ]);
+
+        $request = Request::create('/', 'GET', $query);
+
+        $kernel = $this->createMock(HttpKernelInterface::class);
+        $arguments = $resolver->resolve($request, $argument);
+        $event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
+
+        try {
+            $resolver->onKernelControllerArguments($event);
+            $this->fail(sprintf('Expected "%s" to be thrown.', HttpException::class));
+        } catch (HttpException $e) {
+            $validationFailedException = $e->getPrevious();
+            $this->assertInstanceOf(ValidationFailedException::class, $validationFailedException);
+            $this->assertSame('This value should be of type float.', $validationFailedException->getViolations()[0]->getMessage());
+        }
+    }
+
     public function testRequestInputValidationPassed()
     {
         $input = ['price' => '50'];
@@ -422,6 +455,38 @@ public function testRequestInputValidationPassed()
         $this->assertEquals([$payload], $event->getArguments());
     }
 
+    public function testRequestInputTypeMismatch()
+    {
+        $input = ['price' => 'not a float'];
+
+        $normalizer = new ObjectNormalizer(null, null, null, new ReflectionExtractor());
+        $serializer = new Serializer([$normalizer], ['json' => new JsonEncoder()]);
+
+        $validator = $this->createMock(ValidatorInterface::class);
+        $validator->expects($this->never())->method('validate');
+
+        $resolver = new RequestPayloadValueResolver($serializer, $validator);
+
+        $argument = new ArgumentMetadata('invalid', RequestPayload::class, false, false, null, false, [
+            MapRequestPayload::class => new MapRequestPayload(),
+        ]);
+
+        $request = Request::create('/', 'POST', $input);
+
+        $kernel = $this->createMock(HttpKernelInterface::class);
+        $arguments = $resolver->resolve($request, $argument);
+        $event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
+
+        try {
+            $resolver->onKernelControllerArguments($event);
+            $this->fail(sprintf('Expected "%s" to be thrown.', HttpException::class));
+        } catch (HttpException $e) {
+            $validationFailedException = $e->getPrevious();
+            $this->assertInstanceOf(ValidationFailedException::class, $validationFailedException);
+            $this->assertSame('This value should be of type float.', $validationFailedException->getViolations()[0]->getMessage());
+        }
+    }
+
     public function testItThrowsOnVariadicArgument()
     {
         $serializer = new Serializer();