odolbeau / phone-number-bundle

Integrates libphonenumber into your Symfony application
MIT License
217 stars 43 forks source link

Letters in number break validation #123

Closed gndk closed 11 months ago

gndk commented 2 years ago

The parse() function "will convert letters into numbers where appropriate, and strip the others", as mentioned here https://github.com/giggsey/libphonenumber-for-php/issues/470#issuecomment-947456303.

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.

maxhelias commented 11 months ago

I proposed a fix here : https://github.com/odolbeau/phone-number-bundle/pull/161