Skip to content
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

throw error during serialization when send big data #178

Open
Lochar opened this issue Sep 13, 2024 · 5 comments
Open

throw error during serialization when send big data #178

Lochar opened this issue Sep 13, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@Lochar
Copy link

Lochar commented Sep 13, 2024

Hello,

During the test phase of my application, I had an error when I sent a very long data.

When the deserialization tries to hydrate my object, it generates this error:
NumberParseException > UnexpectedValueException (The string supplied is too long to be a phone number.)

The constraints are not used, because the application crashes before.
I added a Length constraint of 20 just for the example.
And the controller is empty for the same reason.

Is it intentional to throw this error at this point in the process?

Here is an example to reproduce (Symfony) :
PhoneController

<?php

namespace App\Controller;

use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Attribute\MapRequestPayload;
use Symfony\Component\Routing\Annotation\Route;

class PhoneController
{
    #[Route('phone', methods: [Request::METHOD_POST])]
    public function __invoke(
        #[MapRequestPayload]
        PhoneDto $dto,
    ): JsonResponse {}
}

PhoneDto

<?php

namespace App\Controller;

use libphonenumber\PhoneNumber;
use Misd\PhoneNumberBundle\Validator\Constraints\PhoneNumber as AssertPhoneNumber;
use Symfony\Component\Validator\Constraints\Length;

class PhoneDto
{
    public function __construct(
        #[AssertPhoneNumber(type: [AssertPhoneNumber::MOBILE])]
        #[Length(max: 20)]
        public PhoneNumber $phone,
    ) {}
}

Sended data :

{
    "phone": "010203040506070809010203040506070809"
}
@maxhelias maxhelias added the question Further information is requested label Sep 13, 2024
@maxhelias
Copy link
Collaborator

Hi!
It doesn't depend on the bundle but on the lib, and it looks intentional, see: https://github.com/giggsey/libphonenumber-for-php/blob/master/src/PhoneNumberUtil.php#L26-L29

@maxhelias maxhelias closed this as not planned Won't fix, can't repro, duplicate, stale Sep 13, 2024
@Lochar
Copy link
Author

Lochar commented Sep 13, 2024

Thanks for your response.

Yes I had indeed found where the error was originally coming from, but when deserializing, I thought it might be more interesting not to re-encapsulate the error with an UnexpectedValueException. But to return null (or something else), in order to let the Symfony validation process continue and finally return a ValidationFailedException.

@maxhelias
Copy link
Collaborator

Oh! I see what you mean, the MapRequestPayload deserializes before the validation of the data and the lib raises an exception during the parse in the denormalize well before the validation.

Returning null is not the solution, as it will bypass the validation process and result in behavior that is not what was expected.
And if we return the value initially requested, it won't work in some cases where the validation constraint isn't used...

@maxhelias maxhelias reopened this Sep 13, 2024
@maxhelias
Copy link
Collaborator

A good solution would be to add an option in the context to ignore or not the exception.

@maxhelias maxhelias added enhancement New feature or request and removed question Further information is requested labels Sep 13, 2024
@Lochar
Copy link
Author

Lochar commented Sep 13, 2024

Yes, there you have it, you understood my problem :)

Context could be part of the solution indeed, but we still need to find what the deserialize function could return instead.

I just tried this solution:

try {
    return $this->phoneNumberUtil->parse($data, $this->region);
} catch (NumberParseException) {
    $phoneNumber = new PhoneNumber();
    $phoneNumber->setRawInput($data);

    return $phoneNumber;
}

It seems to work in my case, but I don't think I know and have tested all the use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants