fix bug 2015059: where the native sqlite comparison method phone_number_compare() treated all alpha addresses as the same phone number.
- changed phone_number_compare() to handle alpha chars. Before this change the algorithm skipped over all non-dialable chars for
comparison, and thus treating "foo" and "bar" as the same addresses. now we don't skip any char that's alpha. This would treat
"foo" and "bar" as separate addresses, as well as treating "1-800-flowers" and "800-flowers" as the same number.
diff --git a/android/PhoneNumberUtils.cpp b/android/PhoneNumberUtils.cpp
index 47dd279..d469ed7 100644
--- a/android/PhoneNumberUtils.cpp
+++ b/android/PhoneNumberUtils.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <ctype.h>
#include <string.h>
namespace android {
@@ -84,12 +85,21 @@
}
}
-/** True if c is ISO-LATIN characters 0-9, *, # , + */
-static bool isNonSeparator(char ch)
+/**
+ * True if ch is ISO-LATIN characters 0-9, *, # , +
+ * Note this method current does not account for the WILD char 'N'
+ */
+static bool isDialable(char ch)
{
return ('0' <= ch && ch <= '9') || ch == '*' || ch == '#' || ch == '+';
}
+/** Returns true if ch is not dialable or alpha char */
+static bool isSeparator(char ch)
+{
+ return !isDialable(ch) && (isalpha(ch) == 0);
+}
+
/**
* Try to store the pointer to "new_ptr" which does not have trunk prefix.
*
@@ -114,7 +124,7 @@
*new_len = len - (i + 1);
}
return true;
- } else if (isNonSeparator(ch)) {
+ } else if (isDialable(ch)) {
return false;
}
}
@@ -146,18 +156,18 @@
if (ch == '+') state = 1;
else if (ch == '0') state = 2;
else if (ch == '1') state = 8;
- else if (isNonSeparator(ch)) return -1;
+ else if (isDialable(ch)) return -1;
break;
case 2:
if (ch == '0') state = 3;
else if (ch == '1') state = 4;
- else if (isNonSeparator(ch)) return -1;
+ else if (isDialable(ch)) return -1;
break;
case 4:
if (ch == '1') state = 5;
- else if (isNonSeparator(ch)) return -1;
+ else if (isDialable(ch)) return -1;
break;
case 1:
@@ -183,14 +193,14 @@
} else {
state++;
}
- } else if (isNonSeparator(ch)) {
+ } else if (isDialable(ch)) {
return -1;
}
}
break;
case 8:
if (ch == '6') state = 9;
- else if (isNonSeparator(ch)) return -1;
+ else if (isDialable(ch)) return -1;
break;
case 9:
if (ch == '6') {
@@ -230,7 +240,7 @@
// Ignore just one digit, assuming it is trunk prefix.
trunk_prefix_was_read = true;
}
- } else if (isNonSeparator(ch[i])) {
+ } else if (isDialable(ch[i])) {
// Trunk prefix is a digit, not "*", "#"...
return false;
}
@@ -321,11 +331,11 @@
bool skip_compare = false;
char ch_a = a[i_a];
char ch_b = b[i_b];
- if (!isNonSeparator(ch_a)) {
+ if (isSeparator(ch_a)) {
i_a--;
skip_compare = true;
}
- if (!isNonSeparator(ch_b)) {
+ if (isSeparator(ch_b)) {
i_b--;
skip_compare = true;
}
@@ -357,7 +367,7 @@
bool may_be_namp = true;
while (i_a >= 0) {
const char ch_a = a[i_a];
- if (isNonSeparator(ch_a)) {
+ if (isDialable(ch_a)) {
if (may_be_namp && tryGetISODigit(ch_a) == 1) {
may_be_namp = false;
} else {
@@ -368,7 +378,7 @@
}
while (i_b >= 0) {
const char ch_b = b[i_b];
- if (isNonSeparator(ch_b)) {
+ if (isDialable(ch_b)) {
if (may_be_namp && tryGetISODigit(ch_b) == 1) {
may_be_namp = false;
} else {
diff --git a/android/PhoneNumberUtilsTest.cpp b/android/PhoneNumberUtilsTest.cpp
index b033f9d..a33f10a 100644
--- a/android/PhoneNumberUtilsTest.cpp
+++ b/android/PhoneNumberUtilsTest.cpp
@@ -126,6 +126,23 @@
EXPECT_EQ("650-000-3456", "16500003456");
EXPECT_EQ("16610001234", "6610001234");
+ // We also need to compare two alpha addresses to make sure two different strings
+ // aren't treated as the same addresses. This is relevant to SMS as SMS sender may
+ // contain all alpha chars.
+ EXPECT_NE("abcd", "bcde");
+
+ // in the U.S. people often use alpha in the phone number to easily remember it
+ // (e.g. 800-flowers would be dialed as 800-356-9377). Since we accept this form of
+ // phone number in Contacts and others, we should make sure the comparison method
+ // handle them.
+ EXPECT_EQ("1-800-flowers", "800-flowers");
+
+ // TODO: we currently do not support this comparison. It maybe nice to support this
+ // TODO: in the future.
+ // EXPECT_EQ("1-800-flowers", "1-800-356-9377")
+
+ EXPECT_NE("1-800-flowers", "1-800-abcdefg");
+
// Currently we cannot get this test through (Japanese trunk prefix is 0,
// but there is no sensible way to know it now (as of 2009-6-12)...
// EXPECT_NE("290-1234-5678", "+819012345678");
