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

Letters in number break validation #123

Closed
gndk opened this issue Sep 8, 2022 · 1 comment · Fixed by #161
Closed

Letters in number break validation #123

gndk opened this issue Sep 8, 2022 · 1 comment · Fixed by #161
Labels
bug Something isn't working

Comments

@gndk
Copy link

gndk commented Sep 8, 2022

The parse() function "will convert letters into numbers where appropriate, and strip the others", as mentioned here giggsey/libphonenumber-for-php#470 (comment).

For example, when using the default single text form type with the number +49 30 12345 abc, the validation succeeds. However it is not actually valid, but the parsing changes it to +49 30 12345222 after submitting, which is a valid number. Same for the country choice widget.

I think this should be considered a bug here.

The fix is quite simple and I applied it like this in my app. There simply needs to be an additional regex constraint for the number field to exclude all letters. This is also what giggsey recommends in the issue linked above:

If you want to ensure it's numeric before you try libphonenumber, I'd suggest using additional validation first.

Example for country choice widget:

$builder->add(
    'phoneNumber',
    PhoneNumberType::class,
    [
        'widget' => PhoneNumberType::WIDGET_COUNTRY_CHOICE,
        'number_options' => [
            'constraints' => [new Regex(pattern: '/[\p{L}]/u', message: 'The number can not contain letters.', match: false)],
        ],
    ],
);

image

I could not get this simple solution to work for the single text widget though, because the number is already parsed, when it gets to this additional constraint with code like this:

$builder->add(
    'phoneNumber',
    PhoneNumberType::class,
    [
        'widget' => PhoneNumberType::WIDGET_SINGLE_TEXT,
        'constraints' => [new Regex(pattern: '/[\p{L}]/u', message: 'The number can not contain letters.', match: false)],
    ],
);

invalidValue is already a PhoneNumber here, instead of string. This makes the Regex constraint fail for every input, even valid numbers without letters.

image

I played around with the PhoneNumberValidator, but the $value there is also already a parsed PhoneNumber. Which also probably means that everything inside https://github.com/odolbeau/phone-number-bundle/blob/master/src/Validator/Constraints/PhoneNumberValidator.php#L73 is never executed.

For some reason it is parsed multiple times, but I don't know Symfony Form/Validation well enough to understand why/when/where this happens. Must be either the Transformer or Normalizer.

@Nek- Nek- added the bug Something isn't working label Jan 5, 2023
@maxhelias
Copy link
Collaborator

I proposed a fix here : #161

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants