signalapp / Signal-Android

A private messenger for Android.
https://signal.org
GNU Affero General Public License v3.0
25.45k stars 6.1k forks source link

Alphanumeric addresses in SMS messages #665

Closed evgeniuz closed 10 years ago

evgeniuz commented 10 years ago

My carrier and some other companies sometimes send SMS with alphanumeric addresses. When TextSecure receives them, it shows sender as Anonymous. Also, it won't import them from system database (i. e. SMS will not show in TextSecure in any way).

I have saved few such messages on my phone (Galaxy Nexus) and I know a tiny bit about Android development, so if there anything I can check/send to help resolve the issue, I will be glad to help.

evgeniuz commented 10 years ago

I've found that problem arises from RecipientFactory.parseRecipient method that calls validation functions up until isWellFormedSmsAddress which returns false for alphanumeric addresses, since they cannot be used as recipient, only as sender. parseRecipient gets called only from getRecipientsFromString.

So, to solve this problem I think recipients need to be differentiated somehow from senders. I. e. completely bypass number verification when importing/receiving SMS, just store it as is. And then in conversation if we try to send a reply it would show error, just like stock SMS/MMS app is doing currently.

I'm just not sure how to best approach it. I can propose to add additional parameter skipValidation to getRecipientsFromString, add override without this parameter (default should be false). And call with this parameter true when importing/receiving.

moxie0 commented 10 years ago

Can you provide an example of an address you're receiving messages from?

evgeniuz commented 10 years ago

@moxie0 Addresses like "Citrus" or "life:)". I set a breakpoint in parseRecipient and I can see address there, but it won't pass validation, so that function throws exception.

moxie0 commented 10 years ago

Can you post the stack trace of the parse error you see when receiving a message from an address like this?

evgeniuz commented 10 years ago

This is what I get when importing SMS from system database:

02-25 23:31:01.283    7067-7091/org.thoughtcrime.securesms W/SmsMigrator﹕ org.thoughtcrime.securesms.recipients.RecipientFormattingException: Recipient: Citrus is badly formatted.
            at org.thoughtcrime.securesms.recipients.RecipientFactory.parseRecipient(RecipientFactory.java:132)
            at org.thoughtcrime.securesms.recipients.RecipientFactory.getRecipientsFromString(RecipientFactory.java:69)
            at org.thoughtcrime.securesms.database.SmsMigrator.getOurRecipients(SmsMigrator.java:146)
            at org.thoughtcrime.securesms.database.SmsMigrator.migrateDatabase(SmsMigrator.java:207)
            at org.thoughtcrime.securesms.service.ApplicationMigrationService$ImportRunnable.run(ApplicationMigrationService.java:158)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1080)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:573)
            at java.lang.Thread.run(Thread.java:841)

And this is what I get when receiving SMS:

02-25 23:34:23.806    7067-7095/org.thoughtcrime.securesms W/SmsDatabase﹕ org.thoughtcrime.securesms.recipients.RecipientFormattingException: Recipient: life:) is badly formatted.
            at org.thoughtcrime.securesms.recipients.RecipientFactory.parseRecipient(RecipientFactory.java:132)
            at org.thoughtcrime.securesms.recipients.RecipientFactory.getRecipientsFromString(RecipientFactory.java:69)
            at org.thoughtcrime.securesms.database.SmsDatabase.insertMessageInbox(SmsDatabase.java:278)
            at org.thoughtcrime.securesms.database.EncryptingSmsDatabase.insertMessageInbox(EncryptingSmsDatabase.java:81)
            at org.thoughtcrime.securesms.service.SmsReceiver.storeStandardMessage(SmsReceiver.java:100)
            at org.thoughtcrime.securesms.service.SmsReceiver.storeMessage(SmsReceiver.java:202)
            at org.thoughtcrime.securesms.service.SmsReceiver.handleReceiveMessage(SmsReceiver.java:212)
            at org.thoughtcrime.securesms.service.SmsReceiver.process(SmsReceiver.java:219)
            at org.thoughtcrime.securesms.service.SendReceiveService$SendReceiveWorkItem.run(SendReceiveService.java:246)
            at org.thoughtcrime.securesms.util.WorkerThread.run(WorkerThread.java:46)
TiltMeSenpai commented 10 years ago

Is this a duplicate of issue #693 ?

evgeniuz commented 10 years ago

@jano017 I've created a pull request with fix for this issue and GitHub created a new ticket for it.

kkingori commented 10 years ago

Having this exact issue. Textsecure lumps all my mobile money, carrier, bank, service providers e.t.c sms into one "Anonymous" sender. Mobile money is pretty much essential where I'm from and digging through a pile of texts to find pertinent info is a deal breaker.

pheenyx commented 10 years ago

One odd thing I encountered is that while most SMS from my banks and providers and DHL go to the Anonymous thread, I still get SMS from the sender "SPK_SPN" in its own thread. I don't have that name in my contacts, so I guess it came through the parser as a valid number!?

Still, please merge the proposed fix to the playstore version as soon as possible. I'd really like to get these SMS separated again.

evgeniuz commented 10 years ago

@pheenyx isWellFormedSmsAddress works on SPK_SPN is because of how extractNetworkPortion works. It discards non-dialable chars up to DTMF pause/wait. But N is dialable char, it's ASCII code is 0x4e, which corresponds to GSM wild character, which is dialable: http://developer.android.com/reference/android/telephony/PhoneNumberUtils.html#WILD So, in this case it works by accident.

pheenyx commented 10 years ago

@evgeniuz cool thanks for the clarification. Anyway I'd rather like all the senders in separate threads by design, not by accident. So thanks for your pull request.

donjoe0 commented 10 years ago

Same issue here: all service "numbers" get lumped together under "Anonymous". This makes TextSecure unacceptable as an SMS app replacement.

dnel commented 10 years ago

I'm also affected by this, I get a few text alerts from different businesses and they are all grouped together under 'Anonymous'. I have to infer who the sender is by the apparent topic in the contents as they are not in my contacts list and often don't identify themselves in the message body.

marsianer commented 10 years ago

Importing the Android SMS Database is also not possible, when the sender has a alphanumeric addresse. All this messages are skipped at the moment and can't be used in TextTecure.

kiwigitro commented 10 years ago

I also have this problem.

donjoe0 commented 10 years ago

Could someone at least label this as a bug so it shows up on the bugs list? I for one have completely stopped trying to use TextSecure as my new push messaging app (or to suggest it to any of my contacts) because of this issue - I think the greatest attraction TextSecure has to offer regular users by comparison with WhatsApp and its clones is the integration of push messaging and SMS into a single app. But if even basic SMS-related functionalities don't work as expected, this attraction disappears and there is not much reason left to go through the pain of abandoning one's WA conversation histories and try to convince one's friends to switch as well.

marsianer commented 10 years ago

+1 for @donjoe0

AyumuKasuga commented 10 years ago

+1 @donjoe0

bvorak commented 10 years ago

+1 for fixing this

mcginty commented 10 years ago

Agreed, this must be highly annoying. I looked a bit into some other open source messaging apps, Cyanogenmod's MMS app has some complex logic for parsing and identifying legit MMS/SMS targets that we might be able to base ours off of: https://github.com/CyanogenMod/android_packages_apps_Mms/blob/cm-11.0/src/com/android/mms/data/Contact.java#L780

According to their comments, some carriers tend to strip out useful characters causing messages to potentially get sent to incorrect addresses, which is something we definitely want to avoid. So at least unless we can confirm that's not the case, we do need to have some kind of verification/assurance that the target address is a legitimate endpoint.

pheenyx commented 10 years ago

@mcginty Isn't this topic about the messages with an alphanumeric source identification? I agree with you on target numbers that should be valid numbers, but here the source "number" is alphanumeric, which is fine in my opinion. These are legit messages, and i guess nobody answers them.

marsianer commented 10 years ago

Can we support the devs for fixing this issue?

kkingori commented 10 years ago

Pulled down and built it with the fix by @moxie0 and while now it does recognize alphanumeric senders during import and also when receiving sms, they seem to be getting random names. For example the electric company "Kenya Power" is recognized as 'Free Music", mobile money "MPESA" is now "Bonga" and so on.

moxie0 commented 10 years ago

Are those names truly random, or are they present somewhere else like in your contacts?

kkingori commented 10 years ago

on closer inspection, the names are those of alphanumeric senders during import but they are assigned to the wrong threads. This persists even after deleting the thread, new sms still have the previously assigned wrong senders. However if no import is done after install new sms have the correct alphanumeric sender. Hope this helps.