Improve handling of partially bi-directional text.

app/src/main/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducer.java

@@ -18,6 +18,7 @@ import org.signal.storageservice.protos.groups.local.DecryptedPendingMemberRemov
 import org.thoughtcrime.securesms.R;
 import org.thoughtcrime.securesms.groups.GV2AccessLevelUtil;
 import org.thoughtcrime.securesms.util.ExpirationUtil;
+import org.thoughtcrime.securesms.util.StringUtil;
 import org.whispersystems.libsignal.util.guava.Optional;
 import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupUtil;
 import org.whispersystems.signalservice.api.util.UuidUtil;
@@ -396,7 +397,7 @@ final class GroupsV2UpdateMessageProducer {
     boolean editorIsYou = change.getEditor().equals(selfUuidBytes);
 
     if (change.hasNewTitle()) {
-      String newTitle = change.getNewTitle().getValue();
+      String newTitle = StringUtil.isolateBidi(change.getNewTitle().getValue());
       if (editorIsYou) {
         updates.add(updateDescription(context.getString(R.string.MessageRecord_you_changed_the_group_name_to_s, newTitle)));
       } else {
@@ -407,7 +408,7 @@ final class GroupsV2UpdateMessageProducer {
 
   private void describeUnknownEditorNewTitle(@NonNull DecryptedGroupChange change, @NonNull List<UpdateDescription> updates) {
     if (change.hasNewTitle()) {
-      updates.add(updateDescription(context.getString(R.string.MessageRecord_the_group_name_has_changed_to_s, change.getNewTitle().getValue())));
+      updates.add(updateDescription(context.getString(R.string.MessageRecord_the_group_name_has_changed_to_s, StringUtil.isolateBidi(change.getNewTitle().getValue()))));
     }
   }
 

app/src/main/java/org/thoughtcrime/securesms/database/model/MessageRecord.java

@@ -39,6 +39,7 @@ import org.thoughtcrime.securesms.recipients.RecipientId;
 import org.thoughtcrime.securesms.util.Base64;
 import org.thoughtcrime.securesms.util.ExpirationUtil;
 import org.thoughtcrime.securesms.util.GroupUtil;
+import org.thoughtcrime.securesms.util.StringUtil;
 import org.whispersystems.libsignal.util.guava.Function;
 import org.whispersystems.signalservice.api.util.UuidUtil;
 
@@ -196,8 +197,8 @@ public abstract class MessageRecord extends DisplayRecord {
 
       if (profileChangeDetails.hasProfileNameChange()) {
         String displayName  = getIndividualRecipient().getDisplayName(context);
-        String newName      = ProfileName.fromSerialized(profileChangeDetails.getProfileNameChange().getNew()).toString();
+        String newName      = StringUtil.isolateBidi(ProfileName.fromSerialized(profileChangeDetails.getProfileNameChange().getNew()).toString());
-        String previousName = ProfileName.fromSerialized(profileChangeDetails.getProfileNameChange().getPrevious()).toString();
+        String previousName = StringUtil.isolateBidi(ProfileName.fromSerialized(profileChangeDetails.getProfileNameChange().getPrevious()).toString());
 
         if (getIndividualRecipient().isSystemContact()) {
           return context.getString(R.string.MessageRecord_changed_their_profile_name_from_to, displayName, previousName, newName);

app/src/main/java/org/thoughtcrime/securesms/messagerequests/MessageRequestsBottomView.java

@@ -65,7 +65,7 @@ public class MessageRequestsBottomView extends ConstraintLayout {
       if (recipient.isGroup()) {
         question.setText(R.string.MessageRequestBottomView_unblock_this_group_and_share_your_name_and_photo_with_its_members);
       } else {
-        String name = recipient.getProfileName().isEmpty() ? recipient.getDisplayName(getContext()) : recipient.getProfileName().getGivenName();
+        String name = recipient.getShortDisplayName(getContext());
         question.setText(HtmlCompat.fromHtml(getContext().getString(R.string.MessageRequestBottomView_do_you_want_to_let_s_message_you_wont_receive_any_messages_until_you_unblock_them, HtmlUtil.bold(name)), 0));
       }
       setActiveInactiveGroups(blockedButtons, normalButtons);
@@ -77,7 +77,7 @@ public class MessageRequestsBottomView extends ConstraintLayout {
           question.setText(R.string.MessageRequestBottomView_do_you_want_to_join_this_group_they_wont_know_youve_seen_their_messages_until_you_accept);
         }
       } else {
-        String name = recipient.getProfileName().isEmpty() ? recipient.getDisplayName(getContext()) : recipient.getProfileName().getGivenName();
+        String name = recipient.getShortDisplayName(getContext());
         question.setText(HtmlCompat.fromHtml(getContext().getString(R.string.MessageRequestBottomView_do_you_want_to_let_s_message_you_they_wont_know_youve_seen_their_messages_until_you_accept, HtmlUtil.bold(name)), 0));
       }
       setActiveInactiveGroups(normalButtons, blockedButtons);

app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java

@@ -26,19 +26,18 @@ import org.thoughtcrime.securesms.contacts.avatars.TransparentContactPhoto;
 import org.thoughtcrime.securesms.database.DatabaseFactory;
 import org.thoughtcrime.securesms.database.IdentityDatabase.VerifiedStatus;
 import org.thoughtcrime.securesms.database.RecipientDatabase;
-import org.thoughtcrime.securesms.database.RecipientDatabase.RecipientIdResult;
 import org.thoughtcrime.securesms.database.RecipientDatabase.RegisteredState;
 import org.thoughtcrime.securesms.database.RecipientDatabase.UnidentifiedAccessMode;
 import org.thoughtcrime.securesms.database.RecipientDatabase.VibrateState;
 import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
 import org.thoughtcrime.securesms.groups.GroupId;
-import org.thoughtcrime.securesms.jobs.DirectoryRefreshJob;
 import org.thoughtcrime.securesms.logging.Log;
 import org.thoughtcrime.securesms.notifications.NotificationChannels;
 import org.thoughtcrime.securesms.phonenumbers.NumberUtil;
 import org.thoughtcrime.securesms.phonenumbers.PhoneNumberFormatter;
 import org.thoughtcrime.securesms.profiles.ProfileName;
 import org.thoughtcrime.securesms.util.FeatureFlags;
+import org.thoughtcrime.securesms.util.StringUtil;
 import org.thoughtcrime.securesms.util.Util;
 import org.thoughtcrime.securesms.util.concurrent.SignalExecutors;
 import org.whispersystems.libsignal.util.guava.Optional;
@@ -400,18 +399,22 @@ public class Recipient {
   }
 
   public @NonNull String getDisplayName(@NonNull Context context) {
-    return Util.getFirstNonEmpty(getName(context),
+    String name = Util.getFirstNonEmpty(getName(context),
-                                 getProfileName().toString(),
+                                        getProfileName().toString(),
-                                 getDisplayUsername(),
+                                        getDisplayUsername(),
-                                 e164,
+                                        e164,
-                                 email,
+                                        email,
-                                 context.getString(R.string.Recipient_unknown));
+                                        context.getString(R.string.Recipient_unknown));
+
+    return StringUtil.isolateBidi(name);
   }
 
   public @NonNull String getShortDisplayName(@NonNull Context context) {
-    return Util.getFirstNonEmpty(getName(context),
+    String name = Util.getFirstNonEmpty(getName(context),
-                                 getProfileName().getGivenName(),
+                                        getProfileName().getGivenName(),
-                                 getDisplayName(context));
+                                        getDisplayName(context));
+
+    return StringUtil.isolateBidi(name);
   }
 
   public @NonNull MaterialColor getColor() {

app/src/main/java/org/thoughtcrime/securesms/util/GroupUtil.java

@@ -125,7 +125,7 @@ public final class GroupUtil {
         return description.toString();
       }
 
-      String title = groupContext.getName();
+      String title = StringUtil.isolateBidi(groupContext.getName());
 
       if (members != null && members.size() > 0) {
         description.append("\n");

app/src/main/java/org/thoughtcrime/securesms/util/StringUtil.java

@@ -2,10 +2,12 @@ package org.thoughtcrime.securesms.util;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.core.text.BidiFormatter;
 
 import com.google.android.collect.Sets;
 
 import java.nio.charset.StandardCharsets;
+import java.util.Collections;
 import java.util.Set;
 
 public final class StringUtil {
@@ -14,6 +16,27 @@ public final class StringUtil {
                                                                    '\u200F',  // right-to-left mark
                                                                    '\u2007'); // figure space
 
+  private static final class Bidi {
+    /** Override text direction  */
+    private static final Set<Integer> OVERRIDES = Sets.newHashSet("\u202a".codePointAt(0), /* LRE */
+                                                                  "\u202b".codePointAt(0), /* RLE */
+                                                                  "\u202d".codePointAt(0), /* LRO */
+                                                                  "\u202e".codePointAt(0)  /* RLO */);
+
+    /** Set direction and isolate surrounding text */
+    private static final Set<Integer> ISOLATES = Sets.newHashSet("\u2066".codePointAt(0), /* LRI */
+                                                                 "\u2067".codePointAt(0), /* RLI */
+                                                                 "\u2068".codePointAt(0)  /* FSI */);
+    /** Closes things in {@link #OVERRIDES} */
+    private static final int PDF = "\u202c".codePointAt(0);
+
+    /** Closes things in {@link #ISOLATES} */
+    private static final int PDI = "\u2069".codePointAt(0);
+
+    /** Auto-detecting isolate */
+    private static final int FSI = "\u2068".codePointAt(0);
+  }
+
   private StringUtil() {
   }
 
@@ -99,4 +122,58 @@ public final class StringUtil {
   public static @NonNull String codePointToString(int codePoint) {
     return new String(Character.toChars(codePoint));
   }
+
+  /**
+   * Isolates bi-directional text from influencing surrounding text. You should use this whenever
+   * you're injecting user-generated text into a larger string.
+   *
+   * You'd think we'd be able to trust {@link BidiFormatter}, but unfortunately it just misses some
+   * corner cases, so here we are.
+   *
+   * The general idea is just to balance out the opening and closing codepoints, and then wrap the
+   * whole thing in FSI/PDI to isolate it.
+   *
+   * For more details, see:
+   * https://www.w3.org/International/questions/qa-bidi-unicode-controls
+   */
+  public static @NonNull String isolateBidi(@NonNull String text) {
+    int overrideCount      = 0;
+    int overrideCloseCount = 0;
+    int isolateCount       = 0;
+    int isolateCloseCount  = 0;
+
+    for (int i = 0, len = text.codePointCount(0, text.length()); i < len; i++) {
+      int codePoint = text.codePointAt(i);
+
+      if (Bidi.OVERRIDES.contains(codePoint)) {
+        overrideCount++;
+      } else if (codePoint == Bidi.PDF) {
+        overrideCloseCount++;
+      } else if (Bidi.ISOLATES.contains(codePoint)) {
+        isolateCount++;
+      } else if (codePoint == Bidi.PDI) {
+        isolateCloseCount++;
+      }
+    }
+
+    StringBuilder suffix = new StringBuilder();
+
+    while (overrideCount > overrideCloseCount) {
+      suffix.appendCodePoint(Bidi.PDF);
+      overrideCloseCount++;
+    }
+
+    while (isolateCount > isolateCloseCount) {
+      suffix.appendCodePoint(Bidi.FSI);
+      isolateCloseCount++;
+    }
+
+    StringBuilder out = new StringBuilder();
+
+    return out.appendCodePoint(Bidi.FSI)
+              .append(text)
+              .append(suffix)
+              .appendCodePoint(Bidi.PDI)
+              .toString();
+  }
 }

app/src/test/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducerTest.java

@@ -28,6 +28,7 @@ import org.signal.storageservice.protos.groups.local.DecryptedPendingMemberRemov
 import org.signal.storageservice.protos.groups.local.DecryptedString;
 import org.signal.storageservice.protos.groups.local.DecryptedTimer;
 import org.thoughtcrime.securesms.testutil.MainThreadUtil;
+import org.thoughtcrime.securesms.util.StringUtil;
 import org.thoughtcrime.securesms.util.Util;
 import org.whispersystems.signalservice.api.util.UuidUtil;
 
@@ -43,6 +44,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.thoughtcrime.securesms.util.StringUtil.isolateBidi;
 
 @RunWith(RobolectricTestRunner.class)
 @Config(manifest = Config.NONE, application = Application.class)
@@ -603,7 +605,7 @@ public final class GroupsV2UpdateMessageProducerTest {
                                     .title("New title")
                                     .build();
 
-    assertThat(describeChange(change), is(singletonList("Alice changed the group name to \"New title\".")));
+    assertThat(describeChange(change), is(singletonList("Alice changed the group name to \"" + isolateBidi("New title") + "\".")));
   }
 
   @Test
@@ -612,7 +614,7 @@ public final class GroupsV2UpdateMessageProducerTest {
                                     .title("Title 2")
                                     .build();
 
-    assertThat(describeChange(change), is(singletonList("You changed the group name to \"Title 2\".")));
+    assertThat(describeChange(change), is(singletonList("You changed the group name to \"" + isolateBidi("Title 2") + "\".")));
   }
 
   @Test
@@ -621,7 +623,7 @@ public final class GroupsV2UpdateMessageProducerTest {
                                     .title("Title 3")
                                     .build();
 
-    assertThat(describeChange(change), is(singletonList("The group name has changed to \"Title 3\".")));
+    assertThat(describeChange(change), is(singletonList("The group name has changed to \"" + isolateBidi("Title 3") + "\".")));
   }
   
   // Avatar change
@@ -762,7 +764,7 @@ public final class GroupsV2UpdateMessageProducerTest {
 
     assertThat(describeChange(change), is(Arrays.asList(
       "Alice added Bob.",
-      "Alice changed the group name to \"Title\".",
+      "Alice changed the group name to \"" + isolateBidi("Title") + "\".",
       "Alice set the disappearing message timer to 5 minutes.",
       "Alice changed who can edit group membership to \"All members\".")));
   }
@@ -803,7 +805,7 @@ public final class GroupsV2UpdateMessageProducerTest {
 
     assertThat(describeChange(change), is(Arrays.asList(
       "Bob joined the group.",
-      "The group name has changed to \"Title 2\".",
+      "The group name has changed to \"" + isolateBidi("Title 2") + "\".",
       "The group avatar has been changed.",
       "The disappearing message timer has been set to 10 minutes.",
       "Who can edit group membership has been changed to \"All members\".")));
@@ -821,7 +823,7 @@ public final class GroupsV2UpdateMessageProducerTest {
     assertThat(describeChange(change), is(Arrays.asList(
       "Alice joined the group.",
       "Alice is now an admin.",
-      "The group name has changed to \"Updated title\".",
+      "The group name has changed to \"" + isolateBidi("Updated title") + "\".",
       "Alice is no longer in the group.")));
   }