From d0d85ea52c17171f840642cb2f94509fe23aa5dc Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Thu, 14 Feb 2013 19:15:40 -0800 Subject: [PATCH] Fix for duplicate message bug. --- .../securesms/service/SendReceiveService.java | 8 +- .../securesms/service/SmsReceiver.java | 53 +---------- .../securesms/service/SmsSender.java | 90 ++++++++++++++++--- 3 files changed, 83 insertions(+), 68 deletions(-) diff --git a/src/org/thoughtcrime/securesms/service/SendReceiveService.java b/src/org/thoughtcrime/securesms/service/SendReceiveService.java index 44bb9a821b..a4891fdf70 100644 --- a/src/org/thoughtcrime/securesms/service/SendReceiveService.java +++ b/src/org/thoughtcrime/securesms/service/SendReceiveService.java @@ -96,9 +96,9 @@ public class SendReceiveService extends Service { else if (intent.getAction().equals(RECEIVE_SMS_ACTION)) scheduleIntent(RECEIVE_SMS, intent); else if (intent.getAction().equals(SENT_SMS_ACTION)) - scheduleIntent(RECEIVE_SMS, intent); + scheduleIntent(SEND_SMS, intent); else if (intent.getAction().equals(DELIVERED_SMS_ACTION)) - scheduleIntent(RECEIVE_SMS, intent); + scheduleIntent(SEND_SMS, intent); else if (intent.getAction().equals(SEND_MMS_ACTION) || intent.getAction().equals(SEND_MMS_CONNECTIVITY_ACTION)) scheduleSecretRequiredIntent(SEND_MMS, intent); else if (intent.getAction().equals(RECEIVE_MMS_ACTION)) @@ -131,8 +131,8 @@ public class SendReceiveService extends Service { } private void initializeProcessors() { - smsReceiver = new SmsReceiver(this, toastHandler); - smsSender = new SmsSender(this); + smsReceiver = new SmsReceiver(this); + smsSender = new SmsSender(this, toastHandler); mmsReceiver = new MmsReceiver(this); mmsSender = new MmsSender(this, toastHandler); mmsDownloader = new MmsDownloader(this, toastHandler); diff --git a/src/org/thoughtcrime/securesms/service/SmsReceiver.java b/src/org/thoughtcrime/securesms/service/SmsReceiver.java index 7390b4e6f9..fd85e8ab79 100644 --- a/src/org/thoughtcrime/securesms/service/SmsReceiver.java +++ b/src/org/thoughtcrime/securesms/service/SmsReceiver.java @@ -16,17 +16,14 @@ */ package org.thoughtcrime.securesms.service; -import android.app.Activity; import android.content.Context; import android.content.Intent; import android.os.Bundle; import android.preference.PreferenceManager; -import android.telephony.SmsManager; import android.telephony.SmsMessage; import android.util.Log; import org.thoughtcrime.securesms.ApplicationPreferencesActivity; -import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.crypto.DecryptingQueue; import org.thoughtcrime.securesms.crypto.InvalidKeyException; import org.thoughtcrime.securesms.crypto.InvalidVersionException; @@ -39,8 +36,6 @@ import org.thoughtcrime.securesms.notifications.MessageNotifier; import org.thoughtcrime.securesms.protocol.Prefix; import org.thoughtcrime.securesms.protocol.WirePrefix; import org.thoughtcrime.securesms.recipients.Recipient; -import org.thoughtcrime.securesms.recipients.Recipients; -import org.thoughtcrime.securesms.service.SendReceiveService.ToastHandler; import org.thoughtcrime.securesms.sms.MultipartMessageHandler; public class SmsReceiver { @@ -48,11 +43,9 @@ public class SmsReceiver { private MultipartMessageHandler multipartMessageHandler = new MultipartMessageHandler(); private final Context context; - private final ToastHandler toastHandler; - public SmsReceiver(Context context, ToastHandler toastHandler) { + public SmsReceiver(Context context) { this.context = context; - this.toastHandler = toastHandler; } private String assembleSecureMessageFragments(String sender, String messageBody) { @@ -172,49 +165,9 @@ public class SmsReceiver { } } - private void handleSentMessage(Intent intent) { - long messageId = intent.getLongExtra("message_id", -1); - long type = intent.getLongExtra("type", -1); - int result = intent.getIntExtra("ResultCode", -31337); - - Log.w("SMSReceiverService", "Intent resultcode: " + result); - Log.w("SMSReceiverService", "Running sent callback: " + messageId + "," + type); - - if (result == Activity.RESULT_OK) { - DatabaseFactory.getSmsDatabase(context).markAsSent(messageId, type); - } else if (result == SmsManager.RESULT_ERROR_NO_SERVICE || result == SmsManager.RESULT_ERROR_RADIO_OFF) { - toastHandler - .obtainMessage(0, context.getString(R.string.SmsReceiver_currently_unable_to_send_your_sms_message)) - .sendToTarget(); - } else { - long threadId = DatabaseFactory.getSmsDatabase(context).getThreadIdForMessage(messageId); - Recipients recipients = DatabaseFactory.getThreadDatabase(context).getRecipientsForThreadId(context, threadId); - - DatabaseFactory.getSmsDatabase(context).markAsSentFailed(messageId); - MessageNotifier.notifyMessageDeliveryFailed(context, recipients, threadId); - } - } - - private void handleDeliveredMessage(Intent intent) { - long messageId = intent.getLongExtra("message_id", -1); - long type = intent.getLongExtra("type", -1); - byte[] pdu = intent.getByteArrayExtra("pdu"); - String format = intent.getStringExtra("format"); - SmsMessage message = SmsMessage.createFromPdu(pdu); - - if (message == null) { - return; - } - - DatabaseFactory.getSmsDatabase(context).markStatus(messageId, message.getStatus()); - } - public void process(MasterSecret masterSecret, Intent intent) { - if (intent.getAction().equals(SendReceiveService.RECEIVE_SMS_ACTION)) + if (intent.getAction().equals(SendReceiveService.RECEIVE_SMS_ACTION)) { handleReceiveMessage(masterSecret, intent); - else if (intent.getAction().equals(SendReceiveService.SENT_SMS_ACTION)) - handleSentMessage(intent); - else if (intent.getAction().equals(SendReceiveService.DELIVERED_SMS_ACTION)) - handleDeliveredMessage(intent); + } } } diff --git a/src/org/thoughtcrime/securesms/service/SmsSender.java b/src/org/thoughtcrime/securesms/service/SmsSender.java index b4b0ae7638..54a7103bb8 100644 --- a/src/org/thoughtcrime/securesms/service/SmsSender.java +++ b/src/org/thoughtcrime/securesms/service/SmsSender.java @@ -16,6 +16,7 @@ */ package org.thoughtcrime.securesms.service; +import android.app.Activity; import android.app.PendingIntent; import android.content.Context; import android.content.Intent; @@ -23,36 +24,56 @@ import android.database.Cursor; import android.net.Uri; import android.preference.PreferenceManager; import android.telephony.SmsManager; +import android.telephony.SmsMessage; import android.util.Log; import org.thoughtcrime.securesms.ApplicationPreferencesActivity; +import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.crypto.MasterCipher; import org.thoughtcrime.securesms.crypto.MasterSecret; import org.thoughtcrime.securesms.crypto.SessionCipher; import org.thoughtcrime.securesms.database.DatabaseFactory; import org.thoughtcrime.securesms.database.SmsDatabase; +import org.thoughtcrime.securesms.notifications.MessageNotifier; import org.thoughtcrime.securesms.protocol.KeyExchangeWirePrefix; import org.thoughtcrime.securesms.protocol.Prefix; import org.thoughtcrime.securesms.protocol.SecureMessageWirePrefix; import org.thoughtcrime.securesms.protocol.WirePrefix; import org.thoughtcrime.securesms.recipients.Recipient; +import org.thoughtcrime.securesms.recipients.Recipients; +import org.thoughtcrime.securesms.service.SendReceiveService.ToastHandler; import org.thoughtcrime.securesms.sms.MultipartMessageHandler; import org.thoughtcrime.securesms.sms.SmsTransportDetails; import org.thoughtcrime.securesms.util.InvalidMessageException; import java.util.ArrayList; +import java.util.HashSet; +import java.util.Set; public class SmsSender { private final MultipartMessageHandler multipartMessageHandler = new MultipartMessageHandler(); + private final Set pendingMessages = new HashSet(); private final Context context; + private final ToastHandler toastHandler; - public SmsSender(Context context) { - this.context = context; + public SmsSender(Context context, ToastHandler toastHandler) { + this.context = context; + this.toastHandler = toastHandler; } public void process(MasterSecret masterSecret, Intent intent) { + if (intent.getAction().equals(SendReceiveService.SEND_SMS_ACTION)) { + handleSendMessage(masterSecret, intent); + } else if (intent.getAction().equals(SendReceiveService.SENT_SMS_ACTION)) { + handleSentMessage(intent); + } else if (intent.getAction().equals(SendReceiveService.DELIVERED_SMS_ACTION)) { + handleDeliveredMessage(intent); + } + } + + private void handleSendMessage(MasterSecret masterSecret, Intent intent) { MasterCipher masterCipher = new MasterCipher(masterSecret); long messageId = intent.getLongExtra("message_id", -1); Cursor c = null; @@ -65,21 +86,23 @@ public class SmsSender { if (c != null && c.moveToFirst()) { do { - messageId = c.getLong(c.getColumnIndexOrThrow(SmsDatabase.ID)); - String body = c.getString(c.getColumnIndexOrThrow(SmsDatabase.BODY)); - String address = c.getString(c.getColumnIndexOrThrow(SmsDatabase.ADDRESS)); - String messageText = getClearTextBody(masterCipher, body); - long type = c.getLong(c.getColumnIndexOrThrow(SmsDatabase.TYPE)); + messageId = c.getLong(c.getColumnIndexOrThrow(SmsDatabase.ID)); + String body = c.getString(c.getColumnIndexOrThrow(SmsDatabase.BODY)); + String address = c.getString(c.getColumnIndexOrThrow(SmsDatabase.ADDRESS)); + String messageText = getClearTextBody(masterCipher, body); + long type = c.getLong(c.getColumnIndexOrThrow(SmsDatabase.TYPE)); - if (!SmsDatabase.Types.isPendingMessageType(type)) - continue; + if (!SmsDatabase.Types.isPendingMessageType(type)) + continue; - if (isSecureMessage(type)) - messageText = getAsymmetricEncrypt(masterSecret, messageText, address); + if (isSecureMessage(type)) + messageText = getAsymmetricEncrypt(masterSecret, messageText, address); - Log.w("SMSSenderService", "Actually delivering: " + messageId); - - deliverTextMessage(address, messageText, messageId, type); + if (!pendingMessages.contains(messageId)) { + Log.w("SMSSenderService", "Actually delivering: " + messageId); + pendingMessages.add(messageId); + deliverTextMessage(address, messageText, messageId, type); + } } while (c.moveToNext()); } } finally { @@ -88,6 +111,45 @@ public class SmsSender { } } + private void handleSentMessage(Intent intent) { + long messageId = intent.getLongExtra("message_id", -1); + long type = intent.getLongExtra("type", -1); + int result = intent.getIntExtra("ResultCode", -31337); + + Log.w("SMSReceiverService", "Intent resultcode: " + result); + Log.w("SMSReceiverService", "Running sent callback: " + messageId + "," + type); + + if (result == Activity.RESULT_OK) { + DatabaseFactory.getSmsDatabase(context).markAsSent(messageId, type); + } else if (result == SmsManager.RESULT_ERROR_NO_SERVICE || result == SmsManager.RESULT_ERROR_RADIO_OFF) { + toastHandler + .obtainMessage(0, context.getString(R.string.SmsReceiver_currently_unable_to_send_your_sms_message)) + .sendToTarget(); + } else { + long threadId = DatabaseFactory.getSmsDatabase(context).getThreadIdForMessage(messageId); + Recipients recipients = DatabaseFactory.getThreadDatabase(context).getRecipientsForThreadId(context, threadId); + + DatabaseFactory.getSmsDatabase(context).markAsSentFailed(messageId); + MessageNotifier.notifyMessageDeliveryFailed(context, recipients, threadId); + } + + pendingMessages.remove(messageId); + } + + private void handleDeliveredMessage(Intent intent) { + long messageId = intent.getLongExtra("message_id", -1); + long type = intent.getLongExtra("type", -1); + byte[] pdu = intent.getByteArrayExtra("pdu"); + String format = intent.getStringExtra("format"); + SmsMessage message = SmsMessage.createFromPdu(pdu); + + if (message == null) { + return; + } + + DatabaseFactory.getSmsDatabase(context).markStatus(messageId, message.getStatus()); + } + private String getClearTextBody(MasterCipher masterCipher, String body) { if (body.startsWith(Prefix.SYMMETRIC_ENCRYPT)) { try {