bug 2236527: need to scrub the phone number before sending a MMS message.
- add MMS address validity checking and scrubbing. If the phone number used to send a MMS message
contains invalid characters (non digits and non-valid separators), we now warn the user when he tries
to send the message. The prior validity checking code only checks against GSM SMS address, so it
would miss all the invalid chars for MMS address.
- assuming the MMS address is ok but contains separators that are not valid MMS address chars (as
far as the network is concerned), we scrub the number when creating the SendReq data structure for
MMS message.
- flag a few places where things are still not correct, but too risky to fix for MR1.
Change-Id: Ia80a2736f955b94a8cac51ffeb06617a95580ab8
diff --git a/src/com/android/mms/data/ContactList.java b/src/com/android/mms/data/ContactList.java
index 96fdc4e..aaab6dd 100644
--- a/src/com/android/mms/data/ContactList.java
+++ b/src/com/android/mms/data/ContactList.java
@@ -8,6 +8,7 @@
import com.android.mms.data.Contact.UpdateListener;
import com.android.mms.LogTag;
+import com.android.mms.ui.MessageUtils;
public class ContactList extends ArrayList<Contact> {
private static final long serialVersionUID = 1L;
@@ -100,15 +101,28 @@
}
public String[] getNumbers() {
+ return getNumbers(false /* don't scrub for MMS address */);
+ }
+
+ public String[] getNumbers(boolean scrubForMmsAddress) {
List<String> numbers = new ArrayList<String>();
String number;
for (Contact c : this) {
number = c.getNumber();
+
+ if (scrubForMmsAddress) {
+ // parse/scrub the address for valid MMS address. The returned number
+ // could be null if it's not a valid MMS address. We don't want to send
+ // a message to an invalid number, as the network may do its own stripping,
+ // and end up sending the message to a different number!
+ number = MessageUtils.parseMmsAddress(number);
+ }
+
// Don't add duplicate numbers. This can happen if a contact name has a comma.
// Since we use a comma as a delimiter between contacts, the code will consider
// the same recipient has been added twice. The recipients UI still works correctly.
// It's easiest to just make sure we only send to the same recipient once.
- if (!numbers.contains(number)) {
+ if (!TextUtils.isEmpty(number) && !numbers.contains(number)) {
numbers.add(number);
}
}
diff --git a/src/com/android/mms/data/WorkingMessage.java b/src/com/android/mms/data/WorkingMessage.java
index 7806728..4844b21 100644
--- a/src/com/android/mms/data/WorkingMessage.java
+++ b/src/com/android/mms/data/WorkingMessage.java
@@ -912,16 +912,20 @@
final SlideshowModel slideshow = mSlideshow;
final SendReq sendReq = makeSendReq(conv, mSubject);
- // Make sure the text in slide 0 is no longer holding onto a reference to the text
- // in the message text box.
- slideshow.prepareForSend();
+ if (sendReq != null) {
+ // Make sure the text in slide 0 is no longer holding onto a reference to the text
+ // in the message text box.
+ slideshow.prepareForSend();
- // Do the dirty work of sending the message off of the main UI thread.
- new Thread(new Runnable() {
- public void run() {
- sendMmsWorker(conv, mmsUri, persister, slideshow, sendReq);
- }
- }).start();
+ // Do the dirty work of sending the message off of the main UI thread.
+ new Thread(new Runnable() {
+ public void run() {
+ sendMmsWorker(conv, mmsUri, persister, slideshow, sendReq);
+ }
+ }).start();
+ } else {
+ Log.e(LogTag.TAG, "[WorkingMsg] send (mms): invalid SendReq, msg not sent!");
+ }
} else {
// Same rules apply as above.
final String msgText = mText.toString();
@@ -1083,7 +1087,12 @@
}
private static SendReq makeSendReq(Conversation conv, CharSequence subject) {
- String[] dests = conv.getRecipients().getNumbers();
+ String[] dests = conv.getRecipients().getNumbers(true /* scrub for MMS address */);
+
+ if (dests.length == 0) {
+ return null;
+ }
+
SendReq req = new SendReq();
EncodedStringValue[] encodedNumbers = EncodedStringValue.encodeStrings(dests);
if (encodedNumbers != null) {
diff --git a/src/com/android/mms/ui/ComposeMessageActivity.java b/src/com/android/mms/ui/ComposeMessageActivity.java
index ab88a80..8fa0694 100644
--- a/src/com/android/mms/ui/ComposeMessageActivity.java
+++ b/src/com/android/mms/ui/ComposeMessageActivity.java
@@ -492,10 +492,11 @@
return;
}
- if (mRecipientsEditor.hasInvalidRecipient()) {
- if (mRecipientsEditor.hasValidRecipient()) {
+ boolean isMms = mWorkingMessage.requiresMms();
+ if (mRecipientsEditor.hasInvalidRecipient(isMms)) {
+ if (mRecipientsEditor.hasValidRecipient(isMms)) {
String title = getResourcesString(R.string.has_invalid_recipient,
- mRecipientsEditor.formatInvalidNumbers());
+ mRecipientsEditor.formatInvalidNumbers(isMms));
new AlertDialog.Builder(this)
.setIcon(android.R.drawable.ic_dialog_alert)
.setTitle(title)
@@ -2010,9 +2011,9 @@
return;
}
- if (isRecipientsEditorVisible() && !mRecipientsEditor.hasValidRecipient()) {
- MessageUtils.showDiscardDraftConfirmDialog(this,
- new DiscardDraftListener());
+ if (isRecipientsEditorVisible() &&
+ !mRecipientsEditor.hasValidRecipient(mWorkingMessage.requiresMms())) {
+ MessageUtils.showDiscardDraftConfirmDialog(this, new DiscardDraftListener());
return;
}
diff --git a/src/com/android/mms/ui/MessageUtils.java b/src/com/android/mms/ui/MessageUtils.java
index 5b30fe6..421f6a8 100644
--- a/src/com/android/mms/ui/MessageUtils.java
+++ b/src/com/android/mms/ui/MessageUtils.java
@@ -94,6 +94,24 @@
private static final Map<String, String> sRecipientAddress =
new ConcurrentHashMap<String, String>(20 /* initial capacity */);
+
+ /**
+ * MMS address parsing data structures
+ */
+ // allowable phone number separators
+ private static final char[] NUMERIC_CHARS_SUGAR = {
+ '-', '.', ',', '(', ')', ' ', '/', '\\', '*', '#', '+'
+ };
+
+ private static HashMap numericSugarMap = new HashMap (NUMERIC_CHARS_SUGAR.length);
+
+ static {
+ for (int i = 0; i < NUMERIC_CHARS_SUGAR.length; i++) {
+ numericSugarMap.put(NUMERIC_CHARS_SUGAR[i], NUMERIC_CHARS_SUGAR[i]);
+ }
+ }
+
+
private MessageUtils() {
// Forbidden being instantiated.
}
@@ -833,6 +851,10 @@
return false;
}
+ // TODO: not sure if this is the right thing to use. Mms.isPhoneNumber() is
+ // intended for searching for things that look like they might be phone numbers
+ // in arbitrary text, not for validating whether something is in fact a phone number.
+ // It will miss many things that are legitimate phone numbers.
if (Mms.isPhoneNumber(string)) {
return false;
}
@@ -854,14 +876,90 @@
char[] chars = s.toCharArray();
for (int x = 0; x < chars.length; x++) {
char c = chars[x];
- if ((c >= 'a') && (c <= 'z')) continue;
- if ((c >= 'A') && (c <= 'Z')) continue;
- if ((c >= '0') && (c <= '9')) continue;
- return false;
+
+ if ((c >= 'a') && (c <= 'z')) {
+ continue;
+ }
+ if ((c >= 'A') && (c <= 'Z')) {
+ continue;
+ }
+ if ((c >= '0') && (c <= '9')) {
+ continue;
+ }
+
+ return false;
}
return true;
}
+
+
+
+ /**
+ * Given a phone number, return the string without syntactic sugar, meaning parens,
+ * spaces, slashes, dots, dashes, etc. If the input string contains non-numeric
+ * non-punctuation characters, return null.
+ */
+ private static String parsePhoneNumberForMms(String address) {
+ StringBuilder builder = new StringBuilder();
+ int len = address.length();
+
+ for (int i = 0; i < len; i++) {
+ char c = address.charAt(i);
+
+ // accept the first '+' in the address
+ if (c == '+' && builder.length() == 0) {
+ builder.append(c);
+ continue;
+ }
+
+ if (Character.isDigit(c)) {
+ builder.append(c);
+ continue;
+ }
+
+ if (numericSugarMap.get(c) == null) {
+ return null;
+ }
+ }
+ return builder.toString();
+ }
+
+ /**
+ * Returns true if the address passed in is a valid MMS address.
+ */
+ public static boolean isValidMmsAddress(String address) {
+ String retVal = parseMmsAddress(address);
+ return (retVal != null);
+ }
+
+ /**
+ * parse the input address to be a valid MMS address.
+ * - if the address is an email address, leave it as is.
+ * - if the address can be parsed into a valid MMS phone number, return the parsed number.
+ * - if the address is a compliant alias address, leave it as is.
+ */
+ public static String parseMmsAddress(String address) {
+ // if it's a valid Email address, use that.
+ if (Mms.isEmailAddress(address)) {
+ return address;
+ }
+
+ // if we are able to parse the address to a MMS compliant phone number, take that.
+ String retVal = parsePhoneNumberForMms(address);
+ if (retVal != null) {
+ return retVal;
+ }
+
+ // if it's an alias compliant address, use that.
+ if (isAlias(address)) {
+ return address;
+ }
+
+ // it's not a valid MMS address, return null
+ return null;
+ }
+
private static void log(String msg) {
Log.d(TAG, "[MsgUtils] " + msg);
}
diff --git a/src/com/android/mms/ui/RecipientsEditor.java b/src/com/android/mms/ui/RecipientsEditor.java
index f897792..8b493f3 100644
--- a/src/com/android/mms/ui/RecipientsEditor.java
+++ b/src/com/android/mms/ui/RecipientsEditor.java
@@ -34,7 +34,6 @@
import android.text.TextUtils;
import android.text.TextWatcher;
import android.util.AttributeSet;
-import android.util.Log;
import android.view.inputmethod.EditorInfo;
import android.view.MotionEvent;
import android.view.ContextMenu.ContextMenuInfo;
@@ -136,26 +135,29 @@
return list;
}
- private boolean isValid(String number) {
- if (!MmsConfig.getMmsEnabled()) {
+ private boolean isValidAddress(String number, boolean isMms) {
+ if (isMms) {
+ return MessageUtils.isValidMmsAddress(number);
+ } else {
+ // TODO: this only check if the number is a valid GSM SMS address.
+ // if the address contains a dialable char, it considers it a well formed SMS addr.
+ // CDMA doesn't work that way and has a different parser for SMS address (see
+ // CdmaSmsAddress.parse(String address)). We should definitely fix this!!!
return PhoneNumberUtils.isWellFormedSmsAddress(number);
}
-
- return PhoneNumberUtils.isWellFormedSmsAddress(number)
- || Mms.isEmailAddress(number);
}
- public boolean hasValidRecipient() {
+ public boolean hasValidRecipient(boolean isMms) {
for (String number : mTokenizer.getNumbers()) {
- if (isValid(number))
+ if (isValidAddress(number, isMms))
return true;
}
return false;
}
- public boolean hasInvalidRecipient() {
+ public boolean hasInvalidRecipient(boolean isMms) {
for (String number : mTokenizer.getNumbers()) {
- if (!isValid(number)) {
+ if (!isValidAddress(number, isMms)) {
if (MmsConfig.getEmailGateway() == null) {
return true;
} else if (!MessageUtils.isAlias(number)) {
@@ -166,10 +168,10 @@
return false;
}
- public String formatInvalidNumbers() {
+ public String formatInvalidNumbers(boolean isMms) {
StringBuilder sb = new StringBuilder();
for (String number : mTokenizer.getNumbers()) {
- if (!isValid(number)) {
+ if (!isValidAddress(number, isMms)) {
if (sb.length() != 0) {
sb.append(", ");
}
