Merge pull request #1445 from alansley/SES1251_AppCrashOnNonAlphanumeric_REFIX

SES1251 - App crash on non alphanumeric RE-FIX

app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchInputLayout.kt

@@ -2,6 +2,8 @@ package org.thoughtcrime.securesms.home.search
 
 import android.content.Context
 import android.text.Editable
+import android.text.InputFilter
+import android.text.InputFilter.LengthFilter
 import android.text.TextWatcher
 import android.util.AttributeSet
 import android.view.KeyEvent
@@ -34,6 +36,7 @@ class GlobalSearchInputLayout @JvmOverloads constructor(
         binding.searchInput.onFocusChangeListener = this
         binding.searchInput.addTextChangedListener(this)
         binding.searchInput.setOnEditorActionListener(this)
+        binding.searchInput.setFilters( arrayOf<InputFilter>(LengthFilter(100)) ) // 100 char search limit
         binding.searchCancel.setOnClickListener(this)
         binding.searchClear.setOnClickListener(this)
     }

app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchViewModel.kt

@@ -24,8 +24,7 @@ class GlobalSearchViewModel @Inject constructor(private val searchRepository: Se
 
     private val executor = viewModelScope + SupervisorJob()
 
-    private val _result: MutableStateFlow<GlobalSearchResult> =
+    private val _result: MutableStateFlow<GlobalSearchResult> = MutableStateFlow(GlobalSearchResult.EMPTY)
-            MutableStateFlow(GlobalSearchResult.EMPTY)
 
     val result: StateFlow<GlobalSearchResult> = _result
 
@@ -41,13 +40,14 @@ class GlobalSearchViewModel @Inject constructor(private val searchRepository: Se
         _queryText
                 .buffer(onBufferOverflow = BufferOverflow.DROP_OLDEST)
                 .mapLatest { query ->
-                    if (query.trim().length < 2) {
+                    // Early exit on empty search query
+                    if (query.trim().isEmpty()) {
                         SearchResult.EMPTY
                     } else {
-                        // user input delay here in case we get a new query within a few hundred ms
+                        // User input delay in case we get a new query within a few hundred ms this
-                        // this coroutine will be cancelled and expensive query will not be run if typing quickly
+                        // coroutine will be cancelled and the expensive query will not be run.
-                        // first query of 2 characters will be instant however
                         delay(300)
+
                         val settableFuture = SettableFuture<SearchResult>()
                         searchRepository.query(query.toString(), settableFuture::set)
                         try {
@@ -64,6 +64,4 @@ class GlobalSearchViewModel @Inject constructor(private val searchRepository: Se
                 }
                 .launchIn(executor)
     }
-
-
 }

app/src/main/java/org/thoughtcrime/securesms/search/SearchRepository.java

@@ -4,12 +4,8 @@ import android.content.Context;
 import android.database.Cursor;
 import android.database.DatabaseUtils;
 import android.database.MergeCursor;
-import android.text.TextUtils;
-
 import androidx.annotation.NonNull;
-
 import com.annimon.stream.Stream;
-
 import org.session.libsession.messaging.contacts.Contact;
 import org.session.libsession.utilities.Address;
 import org.session.libsession.utilities.GroupRecord;
@@ -27,37 +23,25 @@ import org.thoughtcrime.securesms.database.model.ThreadRecord;
 import org.thoughtcrime.securesms.search.model.MessageResult;
 import org.thoughtcrime.securesms.search.model.SearchResult;
 import org.thoughtcrime.securesms.util.Stopwatch;
-
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Executor;
-
 import kotlin.Pair;
 
-/**
+// Class to manage data retrieval for search
- * Manages data retrieval for search.
- */
 public class SearchRepository {
-
   private static final String TAG = SearchRepository.class.getSimpleName();
 
   private static final Set<Character> BANNED_CHARACTERS = new HashSet<>();
   static {
-    // Several ranges of invalid ASCII characters
+    // Construct a list containing several ranges of invalid ASCII characters
-    for (int i = 33; i <= 47; i++) {
+    // See: https://www.ascii-code.com/
-      BANNED_CHARACTERS.add((char) i);
+    for (int i = 33; i <= 47; i++)   { BANNED_CHARACTERS.add((char) i); } // !, ", #, $, %, &, ', (, ), *, +, ,, -, ., /
-    }
+    for (int i = 58; i <= 64; i++)   { BANNED_CHARACTERS.add((char) i); } // :, ;, <, =, >, ?, @
-    for (int i = 58; i <= 64; i++) {
+    for (int i = 91; i <= 96; i++)   { BANNED_CHARACTERS.add((char) i); } // [, \, ], ^, _, `
-      BANNED_CHARACTERS.add((char) i);
+    for (int i = 123; i <= 126; i++) { BANNED_CHARACTERS.add((char) i); } // {, |, }, ~
-    }
-    for (int i = 91; i <= 96; i++) {
-      BANNED_CHARACTERS.add((char) i);
-    }
-    for (int i = 123; i <= 126; i++) {
-      BANNED_CHARACTERS.add((char) i);
-    }
   }
 
   private final Context                context;
@@ -86,35 +70,25 @@ public class SearchRepository {
   }
 
   public void query(@NonNull String query, @NonNull Callback<SearchResult> callback) {
-    if (TextUtils.isEmpty(query)) {
+    // If the sanitized search is empty then abort without search
+    String cleanQuery = sanitizeQuery(query).trim();
+    if (cleanQuery.isEmpty()) {
       callback.onResult(SearchResult.EMPTY);
       return;
     }
 
     executor.execute(() -> {
       Stopwatch timer = new Stopwatch("FtsQuery");
-
-      String cleanQuery = sanitizeQuery(query);
-
-      // If the search is for a single character and it was stripped by `sanitizeQuery` then abort
-      // the search for an empty string to avoid SQLite error.
-      if (cleanQuery.length() == 0)
-      {
-        Log.d(TAG, "Aborting empty search query.");
-        timer.stop(TAG);
-        return;
-      }
-
       timer.split("clean");
 
       Pair<CursorList<Contact>, List<String>> contacts = queryContacts(cleanQuery);
-      timer.split("contacts");
+      timer.split("Contacts");
 
       CursorList<GroupRecord> conversations = queryConversations(cleanQuery, contacts.getSecond());
-      timer.split("conversations");
+      timer.split("Conversations");
 
       CursorList<MessageResult> messages = queryMessages(cleanQuery);
-      timer.split("messages");
+      timer.split("Messages");
 
       timer.stop(TAG);
 
@@ -123,23 +97,20 @@ public class SearchRepository {
   }
 
   public void query(@NonNull String query, long threadId, @NonNull Callback<CursorList<MessageResult>> callback) {
-    if (TextUtils.isEmpty(query)) {
+    // If the sanitized search query is empty then abort the search
+    String cleanQuery = sanitizeQuery(query).trim();
+    if (cleanQuery.isEmpty()) {
       callback.onResult(CursorList.emptyList());
       return;
     }
 
     executor.execute(() -> {
-      // If the sanitized search query is empty then abort the search to prevent SQLite errors.
-      String cleanQuery = sanitizeQuery(query).trim();
-      if (cleanQuery.isEmpty()) { return; }
-            
       CursorList<MessageResult> messages = queryMessages(cleanQuery, threadId);
       callback.onResult(messages);
     });
   }
 
   private Pair<CursorList<Contact>, List<String>> queryContacts(String query) {
-
     Cursor contacts = contactDatabase.queryContactsByName(query);
     List<Address> contactList = new ArrayList<>();
     List<String> contactStrings = new ArrayList<>();
@@ -166,11 +137,10 @@ public class SearchRepository {
     MergeCursor merged = new MergeCursor(new Cursor[]{addressThreads, individualRecipients});
 
     return new Pair<>(new CursorList<>(merged, new ContactModelBuilder(contactDatabase, threadDatabase)), contactStrings);
-
   }
 
   private CursorList<GroupRecord> queryConversations(@NonNull String query, List<String> matchingAddresses) {
-    List<String>  numbers   = contactAccessor.getNumbersForThreadSearchFilter(context, query);
+    List<String> numbers = contactAccessor.getNumbersForThreadSearchFilter(context, query);
     String localUserNumber = TextSecurePreferences.getLocalNumber(context);
     if (localUserNumber != null) {
       matchingAddresses.remove(localUserNumber);
@@ -189,9 +159,7 @@ public class SearchRepository {
       membersGroupList.close();
     }
 
-
     Cursor conversations = threadDatabase.getFilteredConversationList(new ArrayList<>(addresses));
-
     return conversations != null ? new CursorList<>(conversations, new GroupModelBuilder(threadDatabase, groupDatabase))
             : CursorList.emptyList();
   }
@@ -256,9 +224,7 @@ public class SearchRepository {
 
     private final Context context;
 
-    RecipientModelBuilder(@NonNull Context context) {
+    RecipientModelBuilder(@NonNull Context context) { this.context = context; }
-      this.context = context;
-    }
 
     @Override
     public Recipient build(@NonNull Cursor cursor) {
@@ -301,9 +267,7 @@ public class SearchRepository {
 
     private final Context context;
 
-    MessageModelBuilder(@NonNull Context context) {
+    MessageModelBuilder(@NonNull Context context) { this.context = context; }
-      this.context = context;
-    }
 
     @Override
     public MessageResult build(@NonNull Cursor cursor) {

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

@@ -37,12 +37,10 @@ public class Stopwatch {
       for (int i = 1; i < splits.size(); i++) {
         out.append(splits.get(i).label).append(": ");
         out.append(splits.get(i).time - splits.get(i - 1).time);
-        out.append("  ");
+        out.append("ms  ");
       }
-
+      out.append("total: ").append(splits.get(splits.size() - 1).time - startTime).append("ms.");
-      out.append("total: ").append(splits.get(splits.size() - 1).time - startTime);
     }
-
     Log.d(tag, out.toString());
   }