Address possible issues with AvatarMigrationJob.

signalapp · Oct 18, 2019 · 59d03cb · 59d03cb
1 parent f9f9ae6
commit 59d03cb
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 7 deletions.
diff --git a/src/org/thoughtcrime/securesms/migrations/AvatarMigrationJob.java b/src/org/thoughtcrime/securesms/migrations/AvatarMigrationJob.java
@@ -6,13 +6,16 @@
 import org.thoughtcrime.securesms.jobmanager.Data;
 import org.thoughtcrime.securesms.jobmanager.Job;
 import org.thoughtcrime.securesms.logging.Log;
+import org.thoughtcrime.securesms.phonenumbers.NumberUtil;
 import org.thoughtcrime.securesms.profiles.AvatarHelper;
 import org.thoughtcrime.securesms.recipients.Recipient;
+import org.thoughtcrime.securesms.util.GroupUtil;
 import org.thoughtcrime.securesms.util.Util;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.util.regex.Pattern;
 
 /**
  * Previously, we used a recipient's address as the filename for their avatar. We want to use
@@ -24,6 +27,8 @@ public class AvatarMigrationJob extends MigrationJob {
 
   private static final String TAG = Log.tag(AvatarMigrationJob.class);
 
+  private static final Pattern NUMBER_PATTERN = Pattern.compile("^[0-9\\-+]+$");
+
   AvatarMigrationJob() {
     this(new Parameters.Builder().build());
   }
@@ -47,29 +52,40 @@ public void performMigration() {
     File   oldDirectory = new File(context.getFilesDir(), "avatars");
     File[] files        = oldDirectory.listFiles();
 
+    if (files == null) {
+      Log.w(TAG, "Unable to read directory, and therefore unable to migrate any avatars.");
+      return;
+    }
+
     Log.i(TAG, "Preparing to move " + files.length + " avatars.");
 
     for (File file : files) {
       try {
-        Recipient recipient = Recipient.external(context, file.getName());
-        byte[]    data      = Util.readFully(new FileInputStream(file));
-
-        AvatarHelper.setAvatar(context, recipient.getId(), data);
+        if (isValidFileName(file.getName())) {
+          Recipient recipient = Recipient.external(context, file.getName());
+          byte[]    data      = Util.readFully(new FileInputStream(file));
+
+          AvatarHelper.setAvatar(context, recipient.getId(), data);
+        } else {
+          Log.w(TAG, "Invalid file name! Can't migrate this file. It'll just get deleted.");
+        }
       } catch (IOException e) {
         Log.w(TAG, "Failed to copy avatar file. Skipping it.", e);
       } finally {
         file.delete();
       }
     }
-
-    oldDirectory.delete();
   }
 
   @Override
   boolean shouldRetry(@NonNull Exception e) {
     return false;
   }
 
+  private static boolean isValidFileName(@NonNull String name) {
+    return NUMBER_PATTERN.matcher(name).matches() || GroupUtil.isEncodedGroup(name) || NumberUtil.isValidEmail(name);
+  }
+
   public static class Factory implements Job.Factory<AvatarMigrationJob> {
     @Override
     public @NonNull AvatarMigrationJob create(@NonNull Parameters parameters, @NonNull Data data) {

diff --git a/src/org/thoughtcrime/securesms/migrations/MigrationJob.java b/src/org/thoughtcrime/securesms/migrations/MigrationJob.java
@@ -41,7 +41,7 @@ abstract class MigrationJob extends Job {
       return Result.success();
     } catch (RuntimeException e) {
       Log.w(TAG, JobLogger.format(this, "Encountered a runtime exception."), e);
-      throw e;
+      throw new FailedMigrationError(e);
     } catch (Exception e) {
       if (shouldRetry(e)) {
         Log.w(TAG, JobLogger.format(this, "Encountered a retryable exception."), e);