Skip to content

[Serializer] DateTimeNormalizer throws NotNormalizableValueException if using datetime_format U #50236

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

Closed
tgalcheva opened this issue May 4, 2023 · 3 comments

Comments

@tgalcheva
Copy link

Symfony version(s) affected

6.2.10

Description

Since the changes for the DateTimeNormallizer in this PR I am getting a NotNormalizableValueException when using datetime_format U (Seconds since the Unix Epoch) for denormalization - our timestamp value comes via API. The error message we now get is "The data is either not an string, an empty string, or null; you should pass a string that can be parsed with the passed format or a valid DateTime string".

The value used for creating DateTime object from timestamp is integer, and now the validation is set to explicitly accept only string values.

How to reproduce

<?php

namespace App\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;
use Symfony\Component\Serializer\SerializerInterface;

class DateTimeNormalizerErrorController extends AbstractController
{
    #[Route('/normalizer-bug-example', name: 'normalizer_bug_example')]
    public function __invoke(SerializerInterface $serializer): JsonResponse
    {
        $data = '{"exampleDate":1685716333}';  // Use timestamp to create DateTime
        // $data = '{"exampleDate":"13-05-2023"}';  // When using string value - everything works as expected

        // Throws NotNormalizableValueException
        $triggerErrorObject = $serializer->deserialize($data, ExampleEntity::class, 'json', [
            DateTimeNormalizer::FORMAT_KEY => 'U',  // When using string value - change the format to 'd-m-Y'
        ]);

        // dd($triggerErrorObject->getExampleDate());  // When using string value - this dumps a normal DateTime object

        return new JsonResponse($data, 200, [], true);
    }
}

class ExampleEntity
{
    private \DateTime $exampleDate;

    public function getExampleDate(): \DateTimeInterface
    {
        return $this->exampleDate;
    }

    public function setExampleDate(\DateTime $exampleDate): void
    {
        $this->exampleDate = $exampleDate;
    }
}

Possible Solution

Maybe revert the changes in src/Symfony/Component/Serializer/Normalizer/DateTimeNormalizer.php:94:

-       if (null === $data || (\is_string($data) && '' === trim($data))) {
+       if (null === $data || !\is_string($data) || '' === trim($data)) {

Or cover the case for timestamps in the validation.

Additional Context

No response

@stof
Copy link
Member

stof commented May 4, 2023

I'd rather cover the case of timestamp rather than reverting entirely the change (which provides a better handling of invalid values for any other format than U)

@tgalcheva
Copy link
Author

I'd rather cover the case of timestamp rather than reverting entirely the change (which provides a better handling of invalid values for any other format than U)

Yes, I agree.

Those were only two possible solutions. In my opinion - when using timestamp, based on the definition of the U formatting - number of seconds - this is a number, not needed to be cast to string. In the ideal case - this check should work in both cases 1685716333 and '1685716333'.

I can see the reason for using only string, because at some point the maximum supported integer will be reached, not soon, but some day. I think for the moment this is still widely use also with int timestamps, and this change will cause a lot of bugs and breaks.

@stof
Copy link
Member

stof commented May 4, 2023

@tgalcheva will you work on contributing a fix for that ?

nicolas-grekas added a commit that referenced this issue May 12, 2023
…tugmaks)

This PR was merged into the 5.4 branch.

Discussion
----------

[Serializer] Handle datetime deserialization in U format

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #50236
| License       | MIT
| Doc PR        | -

Fixes #50236

Commits
-------

a5d1ca6 [Serializer] Handle datetime deserialization in U format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants