Wrong validation for Canada number when validating it agains US region.

[VladimirAmiorkov]

trafficstars

Hi,

POD Version - 3.3.4

I have noticed that the API PhoneNumberKit.isValidPhoneNumber(_:withRegion:ignoreType:) is returning incorrect true value when used to lookup if a Canadian number is valid for the US region. In the case where validating a CA (Canada) number to US validation, it should return false but it returns true. Here is an example:

let phoneNumberKit = PhoneNumberKit()
let isCanadaNumberValid = phoneNumberKit.isValidPhoneNumber("14164431000", withRegion: "CA", ignoreType: true) /// returns true
let isUSNumberValid = phoneNumberKit.isValidPhoneNumber("14164431000", withRegion: "US", ignoreType: true) /// returns true

It should not be possible that the same number is valid for both US and CA region.

As a workaround I have found this approach of parsing the phone and then manually validating it using PhoneNumberKit.parse(_:withRegion:ignoreType:):

func isValid(phoneNumber: String, withRegion region: String) -> Bool {
    do {
        let phone = try phoneNumberKit.parse(phoneNumber, withRegion: region)
        let phoneNumberCountryCode = phoneNumberKit.getRegionCode(of: phone)
        
        return phoneNumberCountryCode == region
    } catch {
       return false
   }
}

let isCanadaNumberValid = isValid("14164431000", withRegion: "US", ignoreType: true) /// returns false
let isUSNumberValid = isValid("14164431000", withRegion: "CA", ignoreType: true) /// returns true

[bguidolim]

Hey @VladimirAmiorkov

Could you please check if it's happening in the latest version?

[VladimirAmiorkov]

@bguidolim I will try, it would have been very easy to test if the pod was with the latest version. Is there a reason why the pod is 3.3.x and not 3.4.x ?

[VladimirAmiorkov]

@bguidolim yes this same issue can bee seen in the latest git version. Here is the output of my test:

(lldb) po phoneNumberKit.isValidPhoneNumber("14164431000", withRegion: "US", ignoreType: true)
true

(lldb) po phoneNumberKit.isValidPhoneNumber("14164431000", withRegion: "CA", ignoreType: true)
true

(lldb) po phoneNumberKit.getRegionCode(of: phoneNumberKit.parse("14164431000"))
▿ Optional<String>
  - some : "CA"

[bguidolim]

@bguidolim I will try, it would have been very easy to test if the pod was with the latest version. Is there a reason why the pod is 3.3.x and not 3.4.x ?

Yes. I don't have access to publish on Cocoapods official specs repo, I need access from @marmelroy but he is not answering anymore.

[bguidolim]

@bguidolim yes this same issue can bee seen in the latest git version. Here is the output of my test:

(lldb) po phoneNumberKit.isValidPhoneNumber("14164431000", withRegion: "US", ignoreType: true)
true

(lldb) po phoneNumberKit.isValidPhoneNumber("14164431000", withRegion: "CA", ignoreType: true)
true

(lldb) po phoneNumberKit.getRegionCode(of: phoneNumberKit.parse("14164431000"))
▿ Optional<String>
  - some : "CA"

I think this is because of the fallback function that tries to parse the phone number from different regions with the same international code. I'm looking into this issue already, maybe I should add an option strict when validating phone numbers, if set as true, ignore the fallback attempt.

[VladimirAmiorkov]

I think this is because of the fallback function that tries to parse the phone number from different regions with the same international code. I'm looking into this issue already, maybe I should add an option strict when validating phone numbers, if set as true, ignore the fallback attempt.

Sounds like a good plan. Thank you for looking into this, much appreciated.

Yes. I don't have access to publish on Cocoapods official specs repo, I need access from @marmelroy but he is not answering anymore.

That's too bad, hope someone can give you access as using pods from Cocoapods is way better and safer.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[VladimirAmiorkov]

@bguidolim any news on this issue. I see that it was marked with "stale".

[bguidolim]

@VladimirAmiorkov I think the PR #583 might solve this issue. Could you please check using the latest version?

[VladimirAmiorkov]

@bguidolim Hi thanks for looking into this.

Just tested with 3.5.0 and the same issue still persists, Canadian number compared agains US with isValidPhoneNumber gives true:

let phoneNumberKit = PhoneNumberKit()
let isCanadaNumberValid = phoneNumberKit.isValidPhoneNumber("14164431000", withRegion: "CA", ignoreType: true) /// returns true
let isUSNumberValid = phoneNumberKit.isValidPhoneNumber("14164431000", withRegion: "US", ignoreType: true) /// returns true

[bguidolim]

Seems it's an expected result:

https://libphonenumber.appspot.com/phonenumberparser?number=14164431000&country=US https://libphonenumber.appspot.com/phonenumberparser?number=14164431000&country=CA

[VladimirAmiorkov]

@bguidolim Hmm, I am a bit confused by the API isValidPhoneNumber it has withRegion which I proved.

In the links you shared above, the first one (US) has:

• Result from isValidNumberForRegion() | false and the other (CA) has
• Result from isValidNumberForRegion() | true

Doesn't that mean that the isValidPhoneNumber should behave the same, as you provide a region ?

And also as mentioned I found workaround using PhoneNumberKit.parse(_:withRegion:ignoreType:) which gives for this number Canada as a region code. So if what you are saying is true then this API is misbehaving or is confusing to understand what it is doing. This is my workaround to this entire scenario:

func isValid(phoneNumber: String, withRegion region: String) -> Bool {
    do {
        let phone = try phoneNumberKit.parse(phoneNumber, withRegion: region)
        let phoneNumberCountryCode = phoneNumberKit.getRegionCode(of: phone)
        
        return phoneNumberCountryCode == region
    } catch {
       return false
   }
}

let isCanadaNumberValid = isValid("14164431000", withRegion: "US") /// returns false
let isUSNumberValid = isValid("14164431000", withRegion: "CA") /// returns true

[bguidolim]

Hey @VladimirAmiorkov

The library is not misbehaving in this case, as you can see in the links I've sent you, isValidNumber returns true for both (US and CA).

The check per region is just "not implemented", but since you already have a solution for this, you can add it as an additional check and create a PR.

I would love to review and merge it. :)

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.