Escape addresses in thread, recipient pref, and group databases

Fixes #6847
// FREEBIE

src/org/thoughtcrime/securesms/database/Address.java

@@ -14,11 +14,14 @@ import com.google.i18n.phonenumbers.PhoneNumberUtil;
 import com.google.i18n.phonenumbers.Phonenumber;
 import com.google.i18n.phonenumbers.ShortNumberInfo;
 
+import org.thoughtcrime.securesms.util.DelimiterUtil;
 import org.thoughtcrime.securesms.util.GroupUtil;
 import org.thoughtcrime.securesms.util.NumberUtil;
 import org.thoughtcrime.securesms.util.TextSecurePreferences;
 import org.thoughtcrime.securesms.util.Util;
 
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -52,23 +55,35 @@ public class Address implements Parcelable, Comparable<Address> {
     this(in.readString());
   }
 
-  public static Address fromSerialized(@NonNull String serialized) {
+  public static @NonNull Address fromSerialized(@NonNull String serialized) {
     return new Address(serialized);
   }
 
-  public static List<Address> fromSerializedList(@NonNull String serialized, @NonNull String delimiter) {
+  public static Address fromExternal(@NonNull Context context, @Nullable String external) {
-    List<String>  elements  = Util.split(serialized, delimiter);
+    return new Address(new ExternalAddressFormatter(TextSecurePreferences.getLocalNumber(context)).format(external));
-    List<Address> addresses = new LinkedList<>();
+  }
 
-    for (String element : elements) {
+  public static @NonNull List<Address> fromSerializedList(@NonNull String serialized, char delimiter) {
-      addresses.add(Address.fromSerialized(element));
+    String[]      escapedAddresses = DelimiterUtil.split(serialized, delimiter);
+    List<Address> addresses        = new LinkedList<>();
+
+    for (String escapedAddress : escapedAddresses) {
+      addresses.add(Address.fromSerialized(DelimiterUtil.unescape(escapedAddress, delimiter)));
     }
 
     return addresses;
   }
 
-  public static Address fromExternal(@NonNull Context context, @Nullable String external) {
+  public static @NonNull String toSerializedList(@NonNull List<Address> addresses, char delimiter) {
-    return new Address(new ExternalAddressFormatter(TextSecurePreferences.getLocalNumber(context)).format(external));
+    Collections.sort(addresses);
+
+    List<String> escapedAddresses = new LinkedList<>();
+
+    for (Address address : addresses) {
+      escapedAddresses.add(DelimiterUtil.escape(address.serialize(), delimiter));
+    }
+
+    return Util.join(escapedAddresses, delimiter + "");
   }
 
   public static Address[] fromParcelable(Parcelable[] parcelables) {

src/org/thoughtcrime/securesms/database/GroupDatabase.java

@@ -17,7 +17,6 @@ import org.thoughtcrime.securesms.recipients.RecipientFactory;
 import org.thoughtcrime.securesms.recipients.Recipients;
 import org.thoughtcrime.securesms.util.BitmapUtil;
 import org.thoughtcrime.securesms.util.GroupUtil;
-import org.thoughtcrime.securesms.util.TextSecurePreferences;
 import org.thoughtcrime.securesms.util.Util;
 import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentPointer;
 
@@ -120,7 +119,7 @@ public class GroupDatabase extends Database {
     ContentValues contentValues = new ContentValues();
     contentValues.put(GROUP_ID, GroupUtil.getEncodedId(groupId));
     contentValues.put(TITLE, title);
-    contentValues.put(MEMBERS, Util.join(members, ","));
+    contentValues.put(MEMBERS, Address.toSerializedList(members, ','));
 
     if (avatar != null) {
       contentValues.put(AVATAR_ID, avatar.getId());
@@ -185,7 +184,7 @@ public class GroupDatabase extends Database {
 
   public void updateMembers(byte[] id, List<Address> members) {
     ContentValues contents = new ContentValues();
-    contents.put(MEMBERS, Util.join(members, ","));
+    contents.put(MEMBERS, Address.toSerializedList(members, ','));
     contents.put(ACTIVE, 1);
 
     databaseHelper.getWritableDatabase().update(TABLE_NAME, contents, GROUP_ID + " = ?",
@@ -197,7 +196,7 @@ public class GroupDatabase extends Database {
     currentMembers.remove(source);
 
     ContentValues contents = new ContentValues();
-    contents.put(MEMBERS, Util.join(currentMembers, ","));
+    contents.put(MEMBERS, Address.toSerializedList(currentMembers, ','));
 
     databaseHelper.getWritableDatabase().update(TABLE_NAME, contents, GROUP_ID + " = ?",
                                                 new String[] {GroupUtil.getEncodedId(id)});
@@ -213,13 +212,8 @@ public class GroupDatabase extends Database {
                                                           null, null, null);
 
       if (cursor != null && cursor.moveToFirst()) {
-        List<Address> results = new LinkedList<>();
+        String serializedMembers = cursor.getString(cursor.getColumnIndexOrThrow(MEMBERS));
-
+        return Address.fromSerializedList(serializedMembers, ',');
-        for (String member : Util.split(cursor.getString(cursor.getColumnIndexOrThrow(MEMBERS)), ",")) {
-          results.add(Address.fromSerialized(member));
-        }
-
-        return results;
       }
 
       return new LinkedList<>();
@@ -307,7 +301,7 @@ public class GroupDatabase extends Database {
     {
       this.id                = id;
       this.title             = title;
-      this.members           = Address.fromSerializedList(members, ",");
+      this.members           = Address.fromSerializedList(members, ',');
       this.avatar            = avatar;
       this.avatarId          = avatarId;
       this.avatarKey         = avatarKey;

src/org/thoughtcrime/securesms/database/RecipientPreferenceDatabase.java

@@ -14,10 +14,10 @@ import org.greenrobot.eventbus.EventBus;
 import org.thoughtcrime.securesms.color.MaterialColor;
 import org.thoughtcrime.securesms.recipients.RecipientFactory;
 import org.thoughtcrime.securesms.recipients.Recipients;
-import org.thoughtcrime.securesms.util.Util;
 import org.whispersystems.libsignal.util.guava.Optional;
 
 import java.util.Arrays;
+import java.util.List;
 
 public class RecipientPreferenceDatabase extends Database {
 
@@ -86,14 +86,12 @@ public class RecipientPreferenceDatabase extends Database {
   }
 
   public Optional<RecipientsPreferences> getRecipientsPreferences(@NonNull Address[] addresses) {
-    Arrays.sort(addresses);
-
     SQLiteDatabase database = databaseHelper.getReadableDatabase();
     Cursor         cursor   = null;
 
     try {
       cursor = database.query(TABLE_NAME, null, ADDRESSES + " = ?",
-                              new String[] {Util.join(addresses, " ")},
+                              new String[] {Address.toSerializedList(Arrays.asList(addresses), ' ')},
                               null, null, null);
 
       if (cursor != null && cursor.moveToNext()) {
@@ -187,11 +185,12 @@ public class RecipientPreferenceDatabase extends Database {
 
     database.beginTransaction();
 
-    int updated = database.update(TABLE_NAME, contentValues, ADDRESSES + " = ?",
+    List<Address> addresses           = recipients.getAddressesList();
-                                  new String[] {Util.join(recipients.getAddresses(), " ")});
+    String        serializedAddresses = Address.toSerializedList(addresses, ' ');
+    int           updated             = database.update(TABLE_NAME, contentValues, ADDRESSES + " = ?", new String[]{serializedAddresses});
 
     if (updated < 1) {
-      contentValues.put(ADDRESSES, Util.join(recipients.getAddresses(), " "));
+      contentValues.put(ADDRESSES, serializedAddresses);
       database.insert(TABLE_NAME, null, contentValues);
     }
 
@@ -211,13 +210,13 @@ public class RecipientPreferenceDatabase extends Database {
     private final int           defaultSubscriptionId;
     private final int           expireMessages;
 
-    public RecipientsPreferences(boolean blocked, long muteUntil,
+    RecipientsPreferences(boolean blocked, long muteUntil,
-                                 @NonNull VibrateState vibrateState,
+                          @NonNull VibrateState vibrateState,
-                                 @Nullable Uri notification,
+                          @Nullable Uri notification,
-                                 @Nullable MaterialColor color,
+                          @Nullable MaterialColor color,
-                                 boolean seenInviteReminder,
+                          boolean seenInviteReminder,
-                                 int defaultSubscriptionId,
+                          int defaultSubscriptionId,
-                                 int expireMessages)
+                          int expireMessages)
     {
       this.blocked               = blocked;
       this.muteUntil             = muteUntil;
@@ -267,21 +266,16 @@ public class RecipientPreferenceDatabase extends Database {
     private final Context context;
     private final Cursor cursor;
 
-    public BlockedReader(Context context, Cursor cursor) {
+    BlockedReader(Context context, Cursor cursor) {
       this.context = context;
       this.cursor  = cursor;
     }
 
     public @NonNull Recipients getCurrent() {
-      String    serialized  = cursor.getString(cursor.getColumnIndexOrThrow(ADDRESSES));
+      String        serialized  = cursor.getString(cursor.getColumnIndexOrThrow(ADDRESSES));
-      String[]  addresses   = serialized.split(" ");
+      List<Address> addressList = Address.fromSerializedList(serialized, ' ');
-      Address[] addressList = new Address[addresses.length];
-
-      for (int i=0;i<addresses.length;i++) {
-        addressList[i] = Address.fromSerialized(addresses[i]);
-      }
 
-      return RecipientFactory.getRecipientsFor(context, addressList, false);
+      return RecipientFactory.getRecipientsFor(context, addressList.toArray(new Address[0]), false);
     }
 
     public @Nullable Recipients getNext() {
@@ -297,7 +291,7 @@ public class RecipientPreferenceDatabase extends Database {
 
     private final Recipients recipients;
 
-    public RecipientPreferenceEvent(Recipients recipients) {
+    RecipientPreferenceEvent(Recipients recipients) {
       this.recipients = recipients;
     }
 

src/org/thoughtcrime/securesms/database/ThreadDatabase.java

@@ -23,7 +23,6 @@ import android.database.MergeCursor;
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteOpenHelper;
 import android.net.Uri;
-import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.text.TextUtils;
 import android.util.Log;
@@ -39,9 +38,11 @@ import org.thoughtcrime.securesms.mms.Slide;
 import org.thoughtcrime.securesms.mms.SlideDeck;
 import org.thoughtcrime.securesms.recipients.RecipientFactory;
 import org.thoughtcrime.securesms.recipients.Recipients;
+import org.thoughtcrime.securesms.util.DelimiterUtil;
 import org.thoughtcrime.securesms.util.Util;
 import org.whispersystems.libsignal.InvalidMessageException;
 
+import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
@@ -92,7 +93,7 @@ public class ThreadDatabase extends Database {
     long date                   = System.currentTimeMillis();
 
     contentValues.put(DATE, date - date % 1000);
-    contentValues.put(ADDRESSES, Util.join(addresses, " "));
+    contentValues.put(ADDRESSES, Address.toSerializedList(Arrays.asList(addresses), ' '));
 
     if (recipientCount > 1)
       contentValues.put(TYPE, distributionType);
@@ -288,7 +289,7 @@ public class ThreadDatabase extends Database {
 
       int i= 0;
       for (Address address : addresses) {
-        selectionArgs[i++] = address.serialize();
+        selectionArgs[i++] = DelimiterUtil.escape(address.serialize(), ' ');
       }
 
       cursors.add(db.query(TABLE_NAME, null, selection, selectionArgs, null, null, DATE + " DESC"));
@@ -410,11 +411,11 @@ public class ThreadDatabase extends Database {
   }
 
   public long getThreadIdIfExistsFor(Recipients recipients) {
-    Address[] addresses = recipients.getAddresses();
+    List<Address>  addresses     = Arrays.asList(recipients.getAddresses());
-    SQLiteDatabase db      = databaseHelper.getReadableDatabase();
+    SQLiteDatabase db            = databaseHelper.getReadableDatabase();
-    String where           = ADDRESSES + " = ?";
+    String         where         = ADDRESSES + " = ?";
-    String[] recipientsArg = new String[] {Util.join(addresses, " ")};
+    String[]       recipientsArg = new String[]{Address.toSerializedList(addresses, ' ')};
-    Cursor cursor          = null;
+    Cursor         cursor        = null;
 
     try {
       cursor = db.query(TABLE_NAME, new String[]{ID}, where, recipientsArg, null, null, null);
@@ -437,7 +438,7 @@ public class ThreadDatabase extends Database {
     Address[]      addresses     = recipients.getAddresses();
     SQLiteDatabase db            = databaseHelper.getReadableDatabase();
     String         where         = ADDRESSES + " = ?";
-    String[]       recipientsArg = new String[]{Util.join(addresses, " ")};
+    String[]       recipientsArg = new String[]{Address.toSerializedList(Arrays.asList(addresses), ' ')};
     Cursor         cursor        = null;
 
     try {
@@ -462,8 +463,8 @@ public class ThreadDatabase extends Database {
       cursor = db.query(TABLE_NAME, null, ID + " = ?", new String[] {threadId+""}, null, null, null);
 
       if (cursor != null && cursor.moveToFirst()) {
-        Address[] addresses = getAddressesFromSerialized(cursor.getString(cursor.getColumnIndexOrThrow(ADDRESSES)));
+        List<Address> addresses = Address.fromSerializedList(cursor.getString(cursor.getColumnIndexOrThrow(ADDRESSES)), ' ');
-        return RecipientFactory.getRecipientsFor(context, addresses, false);
+        return RecipientFactory.getRecipientsFor(context, addresses.toArray(new Address[0]), false);
       }
     } finally {
       if (cursor != null)
@@ -527,17 +528,6 @@ public class ThreadDatabase extends Database {
     return thumbnail != null ? thumbnail.getThumbnailUri() : null;
   }
 
-  private @NonNull Address[] getAddressesFromSerialized(String serializedAddresses) {
-    String[]  serializedAddressParts = serializedAddresses.split(" ");
-    Address[] addresses              = new Address[serializedAddressParts.length];
-
-    for (int i=0;i<serializedAddressParts.length;i++) {
-      addresses[i] = Address.fromSerialized(serializedAddressParts[i]);
-    }
-
-    return addresses;
-  }
-
   public static interface ProgressListener {
     public void onProgress(int complete, int total);
   }
@@ -571,9 +561,9 @@ public class ThreadDatabase extends Database {
     }
 
     public ThreadRecord getCurrent() {
-      long       threadId   = cursor.getLong(cursor.getColumnIndexOrThrow(ThreadDatabase.ID));
+      long          threadId   = cursor.getLong(cursor.getColumnIndexOrThrow(ThreadDatabase.ID));
-      Address[]  addresses  = getAddressesFromSerialized(cursor.getString(cursor.getColumnIndexOrThrow(ThreadDatabase.ADDRESSES)));
+      List<Address> addresses  = Address.fromSerializedList(cursor.getString(cursor.getColumnIndexOrThrow(ThreadDatabase.ADDRESSES)), ' ');
-      Recipients recipients = RecipientFactory.getRecipientsFor(context, addresses, true);
+      Recipients    recipients = RecipientFactory.getRecipientsFor(context, addresses.toArray(new Address[0]), true);
 
       DisplayRecord.Body body = getPlaintextBody(cursor);
       long date               = cursor.getLong(cursor.getColumnIndexOrThrow(ThreadDatabase.DATE));

src/org/thoughtcrime/securesms/util/DelimiterUtil.java

@@ -0,0 +1,21 @@
+package org.thoughtcrime.securesms.util;
+
+
+import java.util.regex.Pattern;
+
+public class DelimiterUtil {
+
+  public static String escape(String value, char delimiter) {
+    return value.replace("" + delimiter, "\\" + delimiter);
+  }
+
+  public static String unescape(String value, char delimiter) {
+    return value.replace("\\" + delimiter, "" + delimiter);
+  }
+
+  public static String[] split(String value, char delimiter) {
+    String regex = "(?<!\\\\)" + Pattern.quote(delimiter + "");
+    return value.split(regex);
+  }
+
+}

src/org/thoughtcrime/securesms/util/Util.java

@@ -72,20 +72,6 @@ public class Util {
 
   public static Handler handler = new Handler(Looper.getMainLooper());
 
-  public static String join(List<Address> list, String delimiter) {
-    return join(list.toArray(new Address[0]), delimiter);
-  }
-
-  public static String join(Address[] list, String delimiter) {
-    List<String> stringList = new LinkedList<>();
-
-    for (Address address : list) {
-      stringList.add(address.serialize());
-    }
-
-    return join(stringList, delimiter);
-  }
-
   public static String join(String[] list, String delimiter) {
     return join(Arrays.asList(list), delimiter);
   }

test/unitTest/java/org/thoughtcrime/securesms/util/DelimiterUtilTest.java

@@ -0,0 +1,56 @@
+package org.thoughtcrime.securesms.util;
+
+
+import org.junit.Before;
+import org.junit.Test;
+
+import static junit.framework.Assert.assertEquals;
+
+public class DelimiterUtilTest {
+
+  @Before
+  public void setup() {}
+
+  @Test
+  public void testEscape() {
+    assertEquals(DelimiterUtil.escape("MTV Music", ' '), "MTV\\ Music");
+    assertEquals(DelimiterUtil.escape("MTV  Music", ' '), "MTV\\ \\ Music");
+
+    assertEquals(DelimiterUtil.escape("MTV,Music", ','), "MTV\\,Music");
+    assertEquals(DelimiterUtil.escape("MTV,,Music", ','), "MTV\\,\\,Music");
+
+    assertEquals(DelimiterUtil.escape("MTV Music", '+'), "MTV Music");
+  }
+
+  @Test
+  public void testSplit() {
+    String[] parts = DelimiterUtil.split("MTV\\ Music", ' ');
+    assertEquals(parts.length, 1);
+    assertEquals(parts[0], "MTV\\ Music");
+
+    parts = DelimiterUtil.split("MTV Music", ' ');
+    assertEquals(parts.length, 2);
+    assertEquals(parts[0], "MTV");
+    assertEquals(parts[1], "Music");
+  }
+
+  @Test
+  public void testEscapeSplit() {
+    String   input        = "MTV Music";
+    String   intermediate = DelimiterUtil.escape(input, ' ');
+    String[] parts        = DelimiterUtil.split(intermediate, ' ');
+
+    assertEquals(parts.length, 1);
+    assertEquals(parts[0], "MTV\\ Music");
+    assertEquals(DelimiterUtil.unescape(parts[0], ' '), "MTV Music");
+
+    input        = "MTV\\ Music";
+    intermediate = DelimiterUtil.escape(input, ' ');
+    parts        = DelimiterUtil.split(intermediate, ' ');
+
+    assertEquals(parts.length, 1);
+    assertEquals(parts[0], "MTV\\\\ Music");
+    assertEquals(DelimiterUtil.unescape(parts[0], ' '), "MTV\\ Music");
+  }
+
+}