DrHyde / perl-modules-Number-Phone

Number::Phone and friends
24 stars 31 forks source link

Module fails to properly validate numbers with extensions #92

Open quanah opened 5 years ago

quanah commented 5 years ago

I was hoping to use this module to validate phone numbers stored in an LDAP database against ITU E.123 format (which is what the LDAP RFC specifies should be used). However, E.123 allows for extensions, and this module does not handle that well at all.

123 4567 ext. 409: valid

The above is seen as "valid" because it has a total of 9 digits.

Then we get to things that are valid, but treated as undefined, as they don't have 9 numbers:

(123) 456-7899 ext. 125: undefined
123-4567 ext. 15: undefined
123-4567 ext. 1193: undefined

There's a secondary issue here, where area code (at least in LDAP) is considered fully optional, which causes validation to fail for what most Americans at least would see as a perfectly valid local number:

123-4567
davidcantrelluk2 commented 5 years ago

Extensions aren't handled at all, partly because it's not defined as "ext. XXX" or "extension XXX", but as either of those in the local language. I could possibly add support with the proviso that English is specially priveleged by being the default, but I'd have to think hard about how to do it without breaking other modules that are on the CPAN that provide national implementations for countries such as France or Japan. In retrospect it was probably an error to just blindly strip out non-number characters when parsing - it would be better to strip out punctuation characters but complain about letters.

Another reason they're not supported is that this grew out of a telecoms routing and billing application, where extensions were completely irrelevant :-)

I have no intention of ever supporting ambiguous numbers except to extract the local part from complete numbers. Frankly it is silly to ever publish just the local part of a number given the existence of overlays and mobile phones, such that an awful lot of users don't know what the local area code is.

quanah commented 5 years ago

In my case, I'm using a data from a university, where the local area code is not relevant, and may or may not be necessary for dialing.

Generally given the above, I think wording needs to be added to what this module does, as "is_valid" is generally not meaningful for plenty of completely valid numbers.

DrHyde commented 5 years ago

I'll do that.

davidcantrelluk2 commented 5 years ago

https://github.com/DrHyde/perl-modules-Number-Phone/commit/927015a8fd89b978de0327db98952c38b40e4171 will be in the next release.

quanah commented 5 years ago

Thanks! Now if I could find a module that does what I need. ;)

DrHyde commented 5 years ago

Maybe write a small subclass whose constructor would extract the extension and insert your area code into a number, pass that on to Number::Phone::NANP->new(), and which also over-rides methods like format() to add the extension back in when needed.