typestack / class-validator

Decorator-based property validation for classes.
MIT License
11.01k stars 797 forks source link

fix: `@IsPhoneNumber` decorator does not match E.164 format #417

Open ihorTymofieiev opened 5 years ago

ihorTymofieiev commented 5 years ago

Description

Hi, I use the class-validator library to validate request data on a Node.js app. I have problems with validating phone numbers with intl. prefix (use 'ZZ' as a region)!

Example

class PhoneNumberWithIssue {
  @IsNotEmpty({ message: 'ERROR_EMPTY_PHONE_NUMBER_FIELD' })
  @IsPhoneNumber('ZZ', { message: 'ERROR_INVALID_PHONE_NUMBER' })
  phoneNumber!: string;
}

const invalidPhoneNumber = new PhoneNumberWithIssue();
invalidPhoneNumber.phoneNumber = "+380981111111++++++++++=====++++=+";
validate(invalidPhoneNumber).then(errors => {
    if (errors.length > 0) {
        console.log("validation failed. errors: ", errors);
    } else {
        console.log("validation succeed"); // Validation is succeeded, but the phone number is not valid!
    }
});

Dependencies

vlapo commented 5 years ago

Looks like this is the way how to libphonenumber library parse strings. From javadocs:

Parses a string and returns it as a phone number in proto buffer format. The method is quite lenient and looks for a number in the input text (raw input) and does not check whether the string is definitely only a phone number. To do this, it ignores punctuation and white-space, as well as any text before the number (e.g. a leading "Tel: ") and trims the non-number bits. It will accept a number in any format (E164, national, international etc), assuming it can be interpreted with the defaultRegion supplied. It also attempts to convert any alpha characters into digits if it thinks this is a vanity number of the type "1800 MICROSOFT".

So the question is. Is there any possible option how to do more strict parse? I dont think so https://github.com/typestack/class-validator/pull/421/ is good way how to handle this problem E164 is not only valid format.

ihorTymofieiev commented 5 years ago

Hmm, then #421 is not a solution. I will try to make a more universal validation.

vlapo commented 5 years ago

There are plenty of ports https://github.com/google/libphonenumber library to another programming languages. I hope someone have a similar issue. Maybe check official issue tracker for https://github.com/google/libphonenumber first. Some solution could be there. :) I will try check this too.

ihorTymofieiev commented 5 years ago

https://github.com/google/libphonenumber ignores all characters after the phone number. For them, this is normal. You can find notes about this in their documentation and as a comment in the source code.

rubiin commented 5 years ago

@vlapo can we just use something like this https://github.com/validatorjs/validator.js/blob/master/lib/isMobilePhone.js

NoNameProvided commented 4 years ago

The related PR has been closed as out-of-date because the libary used to parse phone numbers has changed. This issue needs to be re-tested with the new lib.

ihorTymofieiev commented 4 years ago

The issue still reproduces.

@NoNameProvided

Dependencies

NoNameProvided commented 4 years ago

Thanks for confirming.

After reading through the conversation here, I am not sure what to do here. (I am not familiar with E.164) Does the upstream lib recognizes that number format? (You can test here: https://catamphetamine.gitlab.io/libphonenumber-js/) If not then first we should open an issue upstream to see if they are willing to fix it. If if does recognize it what options we need to pass into the lib to enable the support?

kamronbatman commented 2 years ago

For anyone reading this like I am and wondering why this hasn't been fixed, here is a quick workaround: is-e164-phone-number.decorator.ts

import { registerDecorator } from 'class-validator';
import { parsePhoneNumberFromString } from 'libphonenumber-js';
import { ValidationOptions } from 'class-validator/types/decorator/ValidationOptions';

export function IsE164PhoneNumber(
  validationOptions?: ValidationOptions,
): PropertyDecorator {
  return function (object: object, propertyName: string) {
    registerDecorator({
      name: 'isE164PhoneNumber',
      target: object.constructor,
      propertyName: propertyName,
      options: validationOptions,
      validator: {
        validate(value: any) {
          const phoneNumber = parsePhoneNumberFromString(value);
          return phoneNumber.number == value && phoneNumber.isValid();
        },
      },
    });
  };
}

Usage:

export class UserDto {
  @IsE164PhoneNumber()
  phoneNumber: string;
}
NoNameProvided commented 1 year ago

I think this will be fixed in the next release. If you provide a region code and the parsed phone number represents a different region the validation will fail from now on.

huykon commented 11 months ago

@NoNameProvided how can I validate for multiple region code?