From 933946dcc9f1f0c551300b5a626dc7e79c40d9fe Mon Sep 17 00:00:00 2001 From: Gene Lian Date: Thu, 11 Jul 2013 11:23:49 +0800 Subject: [PATCH] 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+ --- .../interfaces/nsIMobileMessageCallback.idl | 3 +- .../src/MobileMessageCallback.cpp | 3 + dom/mobilemessage/src/gonk/MmsService.js | 94 +++++++++++++++---- .../src/gonk/MobileMessageDatabaseService.js | 2 +- dom/mobilemessage/src/ipc/SmsIPCService.cpp | 2 +- dom/system/gonk/RadioInterfaceLayer.js | 14 +-- embedding/android/GeckoSmsManager.java | 1 + mobile/android/base/GeckoSmsManager.java | 14 ++- 8 files changed, 98 insertions(+), 35 deletions(-) diff --git a/dom/mobilemessage/interfaces/nsIMobileMessageCallback.idl b/dom/mobilemessage/interfaces/nsIMobileMessageCallback.idl index 37b0c5a1777f..92cce6b83911 100644 --- a/dom/mobilemessage/interfaces/nsIMobileMessageCallback.idl +++ b/dom/mobilemessage/interfaces/nsIMobileMessageCallback.idl @@ -13,7 +13,7 @@ dictionary SmsThreadListItem unsigned long long unreadCount; }; -[scriptable, builtinclass, uuid(e73baef1-7a9f-48c1-8b04-20d9d16c4974)] +[scriptable, builtinclass, uuid(a22d9aae-ee0a-11e2-949e-e770d0d3883f)] interface nsIMobileMessageCallback : nsISupports { /** @@ -28,6 +28,7 @@ interface nsIMobileMessageCallback : nsISupports 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. diff --git a/dom/mobilemessage/src/MobileMessageCallback.cpp b/dom/mobilemessage/src/MobileMessageCallback.cpp index 8773d61c5bf4..fcf65f8f0451 100644 --- a/dom/mobilemessage/src/MobileMessageCallback.cpp +++ b/dom/mobilemessage/src/MobileMessageCallback.cpp @@ -91,6 +91,9 @@ MobileMessageCallback::NotifyError(int32_t aError) 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_CRASH("Should never get here!"); } diff --git a/dom/mobilemessage/src/gonk/MmsService.js b/dom/mobilemessage/src/gonk/MmsService.js index d594534a51b9..3cccc86c29b9 100644 --- a/dom/mobilemessage/src/gonk/MmsService.js +++ b/dom/mobilemessage/src/gonk/MmsService.js @@ -1549,6 +1549,9 @@ MmsService.prototype = { * * @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: * @@ -1561,13 +1564,14 @@ MmsService.prototype = { * 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"] = []; @@ -1577,16 +1581,23 @@ MmsService.prototype = { "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) { @@ -1641,22 +1652,59 @@ MmsService.prototype = { } // The following attributes are needed for saving message into DB. - message["type"] = "mms"; - message["deliveryStatusRequested"] = true; - message["timestamp"] = Date.now(); - message["receivers"] = receivers; - message["sender"] = this.getMsisdn(); + aMessage["type"] = "mms"; + aMessage["deliveryStatusRequested"] = true; + aMessage["timestamp"] = Date.now(); + aMessage["receivers"] = receivers; + aMessage["sender"] = this.getMsisdn(); - 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; } @@ -1683,15 +1731,13 @@ MmsService.prototype = { 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); @@ -1701,7 +1747,8 @@ MmsService.prototype = { }); }; - let savableMessage = this.createSavableFromParams(aParams); + let savableMessage = {}; + let errorCode = this.createSavableFromParams(aParams, savableMessage); gMobileMessageDatabaseService .saveSendingMessage(savableMessage, function notifySendingResult(aRv, aDomMessage) { @@ -1710,6 +1757,12 @@ MmsService.prototype = { // 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."); @@ -1726,6 +1779,7 @@ MmsService.prototype = { return; } + // This is the entry point starting to send MMS. let sendTransaction; try { sendTransaction = new SendTransaction(aDomMessage.id, savableMessage); diff --git a/dom/mobilemessage/src/gonk/MobileMessageDatabaseService.js b/dom/mobilemessage/src/gonk/MobileMessageDatabaseService.js index 384ed836990b..5f53d34c80c3 100644 --- a/dom/mobilemessage/src/gonk/MobileMessageDatabaseService.js +++ b/dom/mobilemessage/src/gonk/MobileMessageDatabaseService.js @@ -1230,7 +1230,7 @@ MobileMessageDatabaseService.prototype = { 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) { diff --git a/dom/mobilemessage/src/ipc/SmsIPCService.cpp b/dom/mobilemessage/src/ipc/SmsIPCService.cpp index 8ebd84e320d8..14eb23d60af7 100644 --- a/dom/mobilemessage/src/ipc/SmsIPCService.cpp +++ b/dom/mobilemessage/src/ipc/SmsIPCService.cpp @@ -221,7 +221,7 @@ SmsIPCService::Send(const JS::Value& aParameters, { SendMmsMessageRequest req; if (!GetSendMmsMessageRequestFromParams(aParameters, req)) { - return NS_ERROR_UNEXPECTED; + return NS_ERROR_INVALID_ARG; } return SendRequest(SendMessageRequest(req), aRequest); } diff --git a/dom/system/gonk/RadioInterfaceLayer.js b/dom/system/gonk/RadioInterfaceLayer.js index 6a0ce8c2082f..0ccb975ea642 100644 --- a/dom/system/gonk/RadioInterfaceLayer.js +++ b/dom/system/gonk/RadioInterfaceLayer.js @@ -3156,7 +3156,11 @@ RadioInterface.prototype = { // 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)) { + if (DEBUG) this.debug("Error! Address is invalid when sending SMS: " + + options.number); + errorCode = Ci.nsIMobileMessageCallback.INVALID_ADDRESS_ERROR; + } else if (!this._radioEnabled) { if (DEBUG) this.debug("Error! Radio is disabled when sending SMS."); errorCode = Ci.nsIMobileMessageCallback.RADIO_DISABLED_ERROR; } else if (this.rilContext.cardState != "ready") { @@ -3184,12 +3188,8 @@ RadioInterface.prototype = { requestStatusReport: options.requestStatusReport }); - if (PhoneNumberUtils.isPlainPhoneNumber(options.number)) { - this.worker.postMessage(options); - } else { - if (DEBUG) this.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)); }, diff --git a/embedding/android/GeckoSmsManager.java b/embedding/android/GeckoSmsManager.java index c22a89ea0672..b7988a571bec 100644 --- a/embedding/android/GeckoSmsManager.java +++ b/embedding/android/GeckoSmsManager.java @@ -308,6 +308,7 @@ public class GeckoSmsManager 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; diff --git a/mobile/android/base/GeckoSmsManager.java b/mobile/android/base/GeckoSmsManager.java index 0ab9963effeb..ebd978836f77 100644 --- a/mobile/android/base/GeckoSmsManager.java +++ b/mobile/android/base/GeckoSmsManager.java @@ -295,11 +295,15 @@ public class GeckoSmsManager * 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;