Bug 891756 - [sms][mms] Gecko needs to return proper error code to notify Gaia the address is invalid (part 1, "InvalidAddressError"). r=vicamo, a=leo+
authorGene Lian <clian@mozilla.com>
Thu, 11 Jul 2013 11:23:49 +0800
changeset 119795 6ef8f8777c69aceed14f2e9a7f3ceb093a433e2a
parent 119794 09349a5128bddc0e8f649c1ac9440dff6c65119a
child 119796 c7e0efd51559cc7636e4b7baf44b66a94d38d564
push id996
push userryanvm@gmail.com
push dateTue, 23 Jul 2013 18:57:33 +0000
reviewersvicamo, leo
bugs891756
milestone18.1
Bug 891756 - [sms][mms] Gecko needs to return proper error code to notify Gaia the address is invalid (part 1, "InvalidAddressError"). r=vicamo, a=leo+
dom/mobilemessage/interfaces/nsIMobileMessageCallback.idl
dom/mobilemessage/src/MobileMessageCallback.cpp
dom/mobilemessage/src/ipc/SmsIPCService.cpp
dom/mobilemessage/src/ril/MmsService.js
dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
dom/system/gonk/RadioInterfaceLayer.js
embedding/android/GeckoSmsManager.java
mobile/android/base/GeckoSmsManager.java
--- a/dom/mobilemessage/interfaces/nsIMobileMessageCallback.idl
+++ b/dom/mobilemessage/interfaces/nsIMobileMessageCallback.idl
@@ -8,31 +8,32 @@ dictionary SmsThreadListItem
 {
   unsigned long long id;
   DOMString senderOrReceiver;
   unsigned long long timestamp;
   DOMString body;
   unsigned long long unreadCount;
 };
 
-[scriptable, builtinclass, uuid(4b48fdca-b476-45e9-8f17-15c534b37845)]
+[scriptable, builtinclass, uuid(a22d9aae-ee0a-11e2-949e-e770d0d3883f)]
 interface nsIMobileMessageCallback : nsISupports
 {
   /**
    * All SMS related errors.
    * Make sure to keep this list in sync with the list in:
    * embedding/android/GeckoSmsManager.java
    */
   const unsigned short SUCCESS_NO_ERROR          = 0;
   const unsigned short NO_SIGNAL_ERROR           = 1;
   const unsigned short NOT_FOUND_ERROR           = 2;
   const unsigned short UNKNOWN_ERROR             = 3;
   const unsigned short INTERNAL_ERROR            = 4;
   const unsigned short NO_SIM_CARD_ERROR         = 5;
   const unsigned short RADIO_DISABLED_ERROR      = 6;
+  const unsigned short INVALID_ADDRESS_ERROR     = 7;
 
   /**
    * |message| can be nsIDOMMoz{Mms,Sms}Message.
    */
   void notifyMessageSent(in nsISupports message);
   void notifySendMessageFailed(in long error);
 
   /**
--- a/dom/mobilemessage/src/MobileMessageCallback.cpp
+++ b/dom/mobilemessage/src/MobileMessageCallback.cpp
@@ -85,16 +85,19 @@ MobileMessageCallback::NotifyError(int32
       mDOMRequest->FireError(NS_LITERAL_STRING("InternalError"));
       break;
     case nsIMobileMessageCallback::NO_SIM_CARD_ERROR:
       mDOMRequest->FireError(NS_LITERAL_STRING("NoSimCardError"));
       break;
     case nsIMobileMessageCallback::RADIO_DISABLED_ERROR:
       mDOMRequest->FireError(NS_LITERAL_STRING("RadioDisabledError"));
       break;
+    case nsIMobileMessageCallback::INVALID_ADDRESS_ERROR:
+      mDOMRequest->FireError(NS_LITERAL_STRING("InvalidAddressError"));
+      break;
     default: // SUCCESS_NO_ERROR is handled above.
       MOZ_NOT_REACHED("Should never get here!");
       return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
--- a/dom/mobilemessage/src/ipc/SmsIPCService.cpp
+++ b/dom/mobilemessage/src/ipc/SmsIPCService.cpp
@@ -256,17 +256,17 @@ GetSendMmsMessageRequestFromParams(const
 }
 
 NS_IMETHODIMP
 SmsIPCService::Send(const JS::Value& aParameters,
                     nsIMobileMessageCallback *aRequest)
 {
   SendMmsMessageRequest req;
   if (!GetSendMmsMessageRequestFromParams(aParameters, req)) {
-    return NS_ERROR_UNEXPECTED;
+    return NS_ERROR_INVALID_ARG;
   }
   return SendRequest(SendMessageRequest(req), aRequest);
 }
 
 NS_IMETHODIMP
 SmsIPCService::Retrieve(int32_t aId, nsIMobileMessageCallback *aRequest)
 {
   return SendRequest(RetrieveMessageRequest(aId), aRequest);
--- a/dom/mobilemessage/src/ril/MmsService.js
+++ b/dom/mobilemessage/src/ril/MmsService.js
@@ -1530,54 +1530,65 @@ MmsService.prototype = {
   },
 
   /**
    * A utility function to convert the MmsParameters dictionary object
    * to a database-savable message.
    *
    * @param aParams
    *        The MmsParameters dictionay object.
+   * @param aMessage (output)
+   *        The database-savable message.
+   * Return the error code by veryfying if the |aParams| is valid or not.
    *
    * Notes:
    *
    * OMA-TS-MMS-CONF-V1_3-20110913-A section 10.2.2 "Message Content Encoding":
    *
    * A name for multipart object SHALL be encoded using name-parameter for Content-Type
    * header in WSP multipart headers. In decoding, name-parameter of Content-Type SHALL
    * be used if available. If name-parameter of Content-Type is not available, filename
    * parameter of Content-Disposition header SHALL be used if available. If neither
    * name-parameter of Content-Type header nor filename parameter of Content-Disposition
    * header is available, Content-Location header SHALL be used if available.
    */
-  createSavableFromParams: function createSavableFromParams(aParams) {
+  createSavableFromParams: function createSavableFromParams(aParams, aMessage) {
     if (DEBUG) debug("createSavableFromParams: aParams: " + JSON.stringify(aParams));
-    let message = {};
+
+    let isAddrValid = true;
     let smil = aParams.smil;
 
-    // |message.headers|
-    let headers = message["headers"] = {};
+    // |aMessage.headers|
+    let headers = aMessage["headers"] = {};
     let receivers = aParams.receivers;
     if (receivers.length != 0) {
       let headersTo = headers["to"] = [];
       for (let i = 0; i < receivers.length; i++) {
         let normalizedAddress = PhoneNumberUtils.normalize(receivers[i], false);
         if (DEBUG) debug("createSavableFromParams: normalize phone number " +
                          "from " + receivers[i] + " to " + normalizedAddress);
 
         headersTo.push({"address": normalizedAddress, "type": "PLMN"});
+
+        // Check if the address is valid to send MMS.
+        if (!PhoneNumberUtils.isPlainPhoneNumber(normalizedAddress)) {
+          if (DEBUG) debug("Error! Address is invalid to send MMS: " +
+                           normalizedAddress);
+          isAddrValid = false;
+        }
       }
     }
     if (aParams.subject) {
       headers["subject"] = aParams.subject;
     }
 
-    // |message.parts|
+    // |aMessage.parts|
     let attachments = aParams.attachments;
     if (attachments.length != 0 || smil) {
-      let parts = message["parts"] = [];
+      let parts = aMessage["parts"] = [];
 
       // Set the SMIL part if needed.
       if (smil) {
         let part = {
           "headers": {
             "content-type": {
               "media": "application/smil",
               "params": {
@@ -1622,31 +1633,68 @@ MmsService.prototype = {
           },
           "content": content
         };
         parts.push(part);
       }
     }
 
     // The following attributes are needed for saving message into DB.
-    message["type"] = "mms";
-    message["deliveryStatusRequested"] = true;
-    message["timestamp"] = Date.now();
-    message["receivers"] = receivers;
+    aMessage["type"] = "mms";
+    aMessage["deliveryStatusRequested"] = true;
+    aMessage["timestamp"] = Date.now();
+    aMessage["receivers"] = receivers;
 
-    if (DEBUG) debug("createSavableFromParams: message: " + JSON.stringify(message));
-    return message;
+    if (DEBUG) debug("createSavableFromParams: aMessage: " +
+                     JSON.stringify(aMessage));
+
+    return isAddrValid ? Ci.nsIMobileMessageCallback.SUCCESS_NO_ERROR
+                       : Ci.nsIMobileMessageCallback.INVALID_ADDRESS_ERROR;
   },
 
   // nsIMmsService
 
   send: function send(aParams, aRequest) {
     if (DEBUG) debug("send: aParams: " + JSON.stringify(aParams));
-    if (aParams.receivers.length == 0) {
-      aRequest.notifySendMessageFailed(Ci.nsIMobileMessageCallback.INTERNAL_ERROR);
+
+    // Note that the following sanity checks for |aParams| should be consistent
+    // with the checks in SmsIPCService.GetSendMmsMessageRequestFromParams.
+
+    // Check if |aParams| is valid.
+    if (aParams == null || typeof aParams != "object") {
+      if (DEBUG) debug("Error! 'aParams' should be a non-null object.");
+      throw Cr.NS_ERROR_INVALID_ARG;
+      return;
+    }
+
+    // Check if |receivers| is valid.
+    if (!Array.isArray(aParams.receivers)) {
+      if (DEBUG) debug("Error! 'receivers' should be an array.");
+      throw Cr.NS_ERROR_INVALID_ARG;
+      return;
+    }
+
+    // Check if |subject| is valid.
+    if (aParams.subject != null && typeof aParams.subject != "string") {
+      if (DEBUG) debug("Error! 'subject' should be a string if passed.");
+      throw Cr.NS_ERROR_INVALID_ARG;
+      return;
+    }
+
+    // Check if |smil| is valid.
+    if (aParams.smil != null && typeof aParams.smil != "string") {
+      if (DEBUG) debug("Error! 'smil' should be a string if passed.");
+      throw Cr.NS_ERROR_INVALID_ARG;
+      return;
+    }
+
+    // Check if |attachments| is valid.
+    if (!Array.isArray(aParams.attachments)) {
+      if (DEBUG) debug("Error! 'attachments' should be an array.");
+      throw Cr.NS_ERROR_INVALID_ARG;
       return;
     }
 
     let self = this;
 
     let sendTransactionCb = function sendTransactionCb(aDomMessage, aErrorCode) {
       if (DEBUG) debug("The error code of sending transaction: " + aErrorCode);
 
@@ -1663,59 +1711,65 @@ MmsService.prototype = {
         .setMessageDelivery(aDomMessage.id,
                             null,
                             isSentSuccess ? DELIVERY_SENT : DELIVERY_ERROR,
                             isSentSuccess ? null : DELIVERY_STATUS_ERROR,
                             function notifySetDeliveryResult(aRv, aDomMessage) {
         if (DEBUG) debug("Marking the delivery state/staus is done. Notify sent or failed.");
         // TODO bug 832140 handle !Components.isSuccessCode(aRv)
         if (!isSentSuccess) {
-          if (DEBUG) debug("Send MMS fail. aParams.receivers = " +
-                           JSON.stringify(aParams.receivers));
+          if (DEBUG) debug("Sending MMS failed.");
           aRequest.notifySendMessageFailed(aErrorCode);
           Services.obs.notifyObservers(aDomMessage, kSmsFailedObserverTopic, null);
           return;
         }
 
-        if (DEBUG) debug("Send MMS successful. aParams.receivers = " +
-                         JSON.stringify(aParams.receivers));
+        if (DEBUG) debug("Sending MMS succeeded.");
 
         // Notifying observers the MMS message is sent.
         self.broadcastSentMessageEvent(aDomMessage);
 
         // Return the request after sending the MMS message successfully.
         aRequest.notifyMessageSent(aDomMessage);
       });
     };
 
-    let savableMessage = this.createSavableFromParams(aParams);
+    let savableMessage = {};
+    let errorCode = this.createSavableFromParams(aParams, savableMessage);
     gMobileMessageDatabaseService
       .saveSendingMessage(savableMessage,
                           function notifySendingResult(aRv, aDomMessage) {
       if (DEBUG) debug("Saving sending message is done. Start to send.");
 
       // TODO bug 832140 handle !Components.isSuccessCode(aRv)
       Services.obs.notifyObservers(aDomMessage, kSmsSendingObserverTopic, null);
 
+      if (errorCode !== Ci.nsIMobileMessageCallback.SUCCESS_NO_ERROR) {
+        if (DEBUG) debug("Error! The params for sending MMS are invalid.");
+        sendTransactionCb(aDomMessage, errorCode);
+        return;
+      }
+
       // For radio disabled error.
       if (gMmsConnection.radioDisabled) {
         if (DEBUG) debug("Error! Radio is disabled when sending MMS.");
         sendTransactionCb(aDomMessage,
                           Ci.nsIMobileMessageCallback.RADIO_DISABLED_ERROR);
         return;
       }
 
       // For SIM card is not ready.
       if (gRIL.rilContext.cardState != "ready") {
         if (DEBUG) debug("Error! SIM card is not ready when sending MMS.");
         sendTransactionCb(aDomMessage,
                           Ci.nsIMobileMessageCallback.NO_SIM_CARD_ERROR);
         return;
       }
 
+      // This is the entry point starting to send MMS.
       let sendTransaction;
       try {
         sendTransaction = new SendTransaction(aDomMessage.id, savableMessage);
       } catch (e) {
         if (DEBUG) debug("Exception: fail to create a SendTransaction instance.");
         sendTransactionCb(aDomMessage,
                           Ci.nsIMobileMessageCallback.INTERNAL_ERROR);
         return;
--- a/dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
+++ b/dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
@@ -1242,17 +1242,17 @@ MobileMessageDatabaseService.prototype =
     }
     aMessage.deliveryIndex = [aMessage.delivery, timestamp];
 
     return this.saveRecord(aMessage, threadParticipants, aCallback);
   },
 
   saveSendingMessage: function saveSendingMessage(aMessage, aCallback) {
     if ((aMessage.type != "sms" && aMessage.type != "mms") ||
-        (aMessage.type == "sms" && !aMessage.receiver) ||
+        (aMessage.type == "sms" && aMessage.receiver == undefined) ||
         (aMessage.type == "mms" && !Array.isArray(aMessage.receivers)) ||
         aMessage.deliveryStatusRequested == undefined ||
         aMessage.timestamp == undefined) {
       if (aCallback) {
         aCallback.notify(Cr.NS_ERROR_FAILURE, null);
       }
       return;
     }
--- a/dom/system/gonk/RadioInterfaceLayer.js
+++ b/dom/system/gonk/RadioInterfaceLayer.js
@@ -2641,17 +2641,21 @@ RadioInterfaceLayer.prototype = {
     let id = gMobileMessageDatabaseService.saveSendingMessage(sendingMessage,
                                                               function notifyResult(rv, domMessage) {
       // TODO bug 832140 handle !Components.isSuccessCode(rv)
       Services.obs.notifyObservers(domMessage, kSmsSendingObserverTopic, null);
 
       // If the radio is disabled or the SIM card is not ready, just directly
       // return with the corresponding error code.
       let errorCode;
-      if (!this._radioEnabled) {
+      if (!PhoneNumberUtils.isPlainPhoneNumber(options.number)) {
+        debug("Error! Address is invalid when sending SMS: " +
+              options.number);
+        errorCode = Ci.nsIMobileMessageCallback.INVALID_ADDRESS_ERROR;
+      } else if (!this._radioEnabled) {
         debug("Error! Radio is disabled when sending SMS.");
         errorCode = Ci.nsIMobileMessageCallback.RADIO_DISABLED_ERROR;
       } else if (this.rilContext.cardState != "ready") {
         debug("Error! SIM card is not ready when sending SMS.");
         errorCode = Ci.nsIMobileMessageCallback.NO_SIM_CARD_ERROR;
       }
       if (errorCode) {
         gMobileMessageDatabaseService
@@ -2669,22 +2673,18 @@ RadioInterfaceLayer.prototype = {
 
       // Keep current SMS message info for sent/delivered notifications
       options.envelopeId = this.createSmsEnvelope({
           request: request,
           sms: domMessage,
           requestStatusReport: options.requestStatusReport
       });
 
-      if (PhoneNumberUtils.isPlainPhoneNumber(options.number)) {
-        this.worker.postMessage(options);
-      } else {
-        debug('Number ' + options.number + ' is not sendable.');
-        this.handleSmsSendFailed(options);
-      }
+      // This is the entry point starting to send SMS.
+      this.worker.postMessage(options);
 
     }.bind(this));
   },
 
   registerDataCallCallback: function registerDataCallCallback(callback) {
     if (this._datacall_callbacks) {
       if (this._datacall_callbacks.indexOf(callback) != -1) {
         throw new Error("Already registered this callback!");
--- a/embedding/android/GeckoSmsManager.java
+++ b/embedding/android/GeckoSmsManager.java
@@ -303,16 +303,17 @@ public class GeckoSmsManager
    */
   public final static int kNoError                = 0;
   public final static int kNoSignalError          = 1;
   public final static int kNotFoundError          = 2;
   public final static int kUnknownError           = 3;
   public final static int kInternalError          = 4;
   public final static int kNoSimCardError         = 5;
   public final static int kRadioDisabledError     = 6;
+  public final static int kInvalidAddressError    = 7;
 
   private final static int kMaxMessageSize    = 160;
 
   private final static Uri kSmsContentUri     = Uri.parse("content://sms");
   private final static Uri kSmsSentContentUri = Uri.parse("content://sms/sent");
 
   private final static int kSmsTypeInbox      = 1;
   private final static int kSmsTypeSentbox    = 2;
--- a/mobile/android/base/GeckoSmsManager.java
+++ b/mobile/android/base/GeckoSmsManager.java
@@ -289,21 +289,25 @@ public class GeckoSmsManager
   public final static String ACTION_SMS_SENT      = "org.mozilla.gecko.SMS_SENT";
   public final static String ACTION_SMS_DELIVERED = "org.mozilla.gecko.SMS_DELIVERED";
 
   /*
    * Make sure that the following error codes are in sync with |ErrorType| in:
    * dom/mobilemessage/src/Types.h
    * The error code are owned by the DOM.
    */
-  public final static int kNoError       = 0;
-  public final static int kNoSignalError = 1;
-  public final static int kNotFoundError = 2;
-  public final static int kUnknownError  = 3;
-  public final static int kInternalError = 4;
+  public final static int kNoError             = 0;
+  public final static int kNoSignalError       = 1;
+  public final static int kNotFoundError       = 2;
+  public final static int kUnknownError        = 3;
+  public final static int kInternalError       = 4;
+  public final static int kNoSimCardError      = 5;
+  public final static int kRadioDisabledError  = 6;
+  public final static int kInvalidAddressError = 7;
+
 
   private final static int kMaxMessageSize    = 160;
 
   private final static Uri kSmsContentUri     = Uri.parse("content://sms");
   private final static Uri kSmsSentContentUri = Uri.parse("content://sms/sent");
 
   private final static int kSmsTypeInbox      = 1;
   private final static int kSmsTypeSentbox    = 2;