Safely delete corrupt files in MetaDataStore

Author: mrober

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java

@@ -17,7 +17,9 @@
 import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
 import com.google.firebase.crashlytics.internal.common.CrashlyticsBackgroundWorker;
 import com.google.firebase.crashlytics.internal.persistence.FileStore;
+import java.io.File;
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -114,10 +116,22 @@ public void testWriteUserData_readDifferentSession() {
     assertNull(userData.getUserId());
   }
 
+  public void testReadUserData_corruptData() throws IOException {
+    File file = storeUnderTest.getUserDataFileForSession(SESSION_ID_1);
+    try (PrintWriter printWriter = new PrintWriter(file)) {
+      printWriter.println("Matt says hi!");
+    }
+    UserMetadata userData = UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, worker);
+    assertNull(userData.getUserId());
+    assertFalse(file.exists());
+  }
+
   public void testReadUserData_emptyData() throws IOException {
-    storeUnderTest.getUserDataFileForSession(SESSION_ID_1).createNewFile();
+    File file = storeUnderTest.getUserDataFileForSession(SESSION_ID_1);
+    file.createNewFile();
     UserMetadata userData = UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, worker);
     assertNull(userData.getUserId());
+    assertFalse(file.exists());
   }
 
   public void testReadUserData_noStoredData() {
@@ -245,10 +259,22 @@ public void testWriteKeys_readSeparateFromUser() {
     assertEqualMaps(internalKeys, readInternalKeys);
   }
 
+  public void testReadKeys_corruptData() throws IOException {
+    File file = storeUnderTest.getKeysFileForSession(SESSION_ID_1);
+    try (PrintWriter printWriter = new PrintWriter(file)) {
+      printWriter.println("This is not json.");
+    }
+    final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
+    assertEquals(0, readKeys.size());
+    assertFalse(file.exists());
+  }
+
   public void testReadKeys_emptyStoredData() throws IOException {
-    storeUnderTest.getKeysFileForSession(SESSION_ID_1).createNewFile();
+    File file = storeUnderTest.getKeysFileForSession(SESSION_ID_1);
+    file.createNewFile();
     final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
     assertEquals(0, readKeys.size());
+    assertFalse(file.exists());
   }
 
   public void testReadKeys_noStoredData() {

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStore.java

@@ -59,7 +59,7 @@ public void writeUserData(String sessionId, String userId) {
       writer.write(userIdJson);
       writer.flush();
     } catch (Exception e) {
-      Logger.getLogger().e("Error serializing user metadata.", e);
+      Logger.getLogger().w("Error serializing user metadata.", e);
     } finally {
       CommonUtils.closeOrLog(writer, "Failed to close user metadata file.");
     }
@@ -70,6 +70,7 @@ public String readUserId(String sessionId) {
     final File f = getUserDataFileForSession(sessionId);
     if (!f.exists() || f.length() == 0) {
       Logger.getLogger().d("No userId set for session " + sessionId);
+      safeDeleteCorruptFile(f);
       return null;
     }
 
@@ -80,7 +81,8 @@ public String readUserId(String sessionId) {
       Logger.getLogger().d("Loaded userId " + userId + " for session " + sessionId);
       return userId;
     } catch (Exception e) {
-      Logger.getLogger().e("Error deserializing user metadata.", e);
+      Logger.getLogger().w("Error deserializing user metadata.", e);
+      safeDeleteCorruptFile(f);
     } finally {
       CommonUtils.closeOrLog(is, "Failed to close user metadata file.");
     }
@@ -101,7 +103,8 @@ public void writeKeyData(String sessionId, Map<String, String> keyData, boolean
       writer.write(keyDataString);
       writer.flush();
     } catch (Exception e) {
-      Logger.getLogger().e("Error serializing key/value metadata.", e);
+      Logger.getLogger().w("Error serializing key/value metadata.", e);
+      safeDeleteCorruptFile(f);
     } finally {
       CommonUtils.closeOrLog(writer, "Failed to close key/value metadata file.");
     }
@@ -115,6 +118,7 @@ Map<String, String> readKeyData(String sessionId, boolean isInternal) {
     final File f =
         isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
     if (!f.exists() || f.length() == 0) {
+      safeDeleteCorruptFile(f);
       return Collections.emptyMap();
     }
 
@@ -123,7 +127,8 @@ Map<String, String> readKeyData(String sessionId, boolean isInternal) {
       is = new FileInputStream(f);
       return jsonToKeysData(CommonUtils.streamToString(is));
     } catch (Exception e) {
-      Logger.getLogger().e("Error deserializing user metadata.", e);
+      Logger.getLogger().w("Error deserializing user metadata.", e);
+      safeDeleteCorruptFile(f);
     } finally {
       CommonUtils.closeOrLog(is, "Failed to close user metadata file.");
     }
@@ -177,4 +182,10 @@ private static String keysDataToJson(final Map<String, String> keyData) {
   private static String valueOrNull(JSONObject json, String key) {
     return !json.isNull(key) ? json.optString(key, null) : null;
   }
+
+  private static void safeDeleteCorruptFile(File file) {
+    if (file.exists() && file.delete()) {
+      Logger.getLogger().i("Deleted corrupt file: " + file.getAbsolutePath());
+    }
+  }
 }