diff options
author | Francisco Pimenta <fpimenta@google.com> | 2017-07-12 09:52:58 -0700 |
---|---|---|
committer | Francisco Pimenta <fpimenta@google.com> | 2017-07-18 12:55:45 -0700 |
commit | 5f46e583b40d86212e7f9c77492768c071a2aa7c (patch) | |
tree | 70f41dfdfa589c6fadc063a7c60504656f206823 /android/OldPhoneNumberUtils.cpp | |
parent | 83ebca9ec9c24d0c291e76aa2ef853ad38ea9dfe (diff) |
Fix false positives comparing local and intl numbers
Previously when comparing local phone numbers with
international it was possible to match the wrong number
because the first digit of the area code was being treated
as a skippable trunk digit, e.g:
"550-123-4567" would be considered equal to "+14501234567".
Since there are two different algorithms (loose/old and strict)
there are two solutions with the same basic goal:
Only ignore mismatches if there's an actual extra digit which
could possibly be the trunk digit, i.e: "0550-123-4567" is acceptable
as equal to "+15501234567". NB: The US not having a trunk digit
to begin with is a different issue entirely - the code is agnostic on
which country has which trunk digits!
For "loose" matching we achieve this by checking the length of the
mismatch.
For "strict" matching we keep track of the supposed trunk digit
and compare it against the current position in match. So for the above
example 5 != 4.
Several new unit tests were added (including replicating tests for
OldPhoneNumberUtils). I broke down the c++ tests into smaller methods
for additional readability.
NB: By adding more tests I uncovered that two digit trunk prefixes
were never handled correctly for the "strict" matching case. "Loose"
matching is much more robust. I commented out the "strict" test cases.
Test: All unit tests pass for PhoneNumberUtilsTest,
OldPhoneNumberUtilsTest and DatabaseGeneralTest.
Bug: 63179537
Bug: 63178542
Bug: 62916088
Change-Id: Idc2a1c2c2f64ed29995208503de4755c521e1c82
Diffstat (limited to 'android/OldPhoneNumberUtils.cpp')
-rw-r--r-- | android/OldPhoneNumberUtils.cpp | 37 |
1 files changed, 6 insertions, 31 deletions
diff --git a/android/OldPhoneNumberUtils.cpp b/android/OldPhoneNumberUtils.cpp index 9c2f20e..9846a1c 100644 --- a/android/OldPhoneNumberUtils.cpp +++ b/android/OldPhoneNumberUtils.cpp @@ -154,33 +154,6 @@ static bool matchIntlPrefixAndCC(const char* a, int len) return state == 6 || state == 7 || state == 8; } -/** or -1 if both are negative */ -static int minPositive(int a, int b) -{ - if (a >= 0 && b >= 0) { - return (a < b) ? a : b; - } else if (a >= 0) { /* && b < 0 */ - return a; - } else if (b >= 0) { /* && a < 0 */ - return b; - } else { /* a < 0 && b < 0 */ - return -1; - } -} - -/** - * Return the offset into a of the first appearance of b, or -1 if there - * is no such character in a. - */ -static int indexOf(const char *a, char b) { - const char *ix = strchr(a, b); - - if (ix == NULL) - return -1; - else - return ix - a; -} - /** * Compare phone numbers a and b, return true if they're identical * enough for caller ID purposes. @@ -270,15 +243,15 @@ bool phone_number_compare_loose(const char* a, const char* b) * (for this, a '0' and a '00' prefix would have succeeded above) */ - if (matchIntlPrefix(a, ia + 1) && matchIntlPrefix(b, ib +1)) { + if (matchIntlPrefix(a, ia + 1) && matchIntlPrefix(b, ib + 1)) { return true; } - if (matchTrunkPrefix(a, ia + 1) && matchIntlPrefixAndCC(b, ib +1)) { + if (matchTrunkPrefix(a, ia + 1) && matchIntlPrefixAndCC(b, ib + 1)) { return true; } - if (matchTrunkPrefix(b, ib + 1) && matchIntlPrefixAndCC(a, ia +1)) { + if (matchTrunkPrefix(b, ib + 1) && matchIntlPrefixAndCC(a, ia + 1)) { return true; } @@ -292,7 +265,9 @@ bool phone_number_compare_loose(const char* a, const char* b) */ bool aPlusFirst = (*a == '+'); bool bPlusFirst = (*b == '+'); - if (ia < 4 && ib < 4 && (aPlusFirst || bPlusFirst) && !(aPlusFirst && bPlusFirst)) { + bool aIgnoreUnmatched = aPlusFirst && (ia - ib) >= 0 && (ia - ib) <= 1; + bool bIgnoreUnmatched = bPlusFirst && (ib - ia) >= 0 && (ib - ia) <= 1; + if (ia < 4 && ib < 4 && (aIgnoreUnmatched || bIgnoreUnmatched) && !(aPlusFirst && bPlusFirst)) { return true; } |