Do not attempt to parse empty json files in MetaDataStore (#3735)

* Do not attempt to parse empty json files in MetaDataStore

* Safely delete corrupt files in MetaDataStore

Author: mrober

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

@@ -17,10 +17,14 @@
 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;
 
+@SuppressWarnings("ResultOfMethodCallIgnored") // Convenient use of files.
 public class MetaDataStoreTest extends CrashlyticsTestCase {
 
   private static final String SESSION_ID_1 = "session1";
@@ -40,7 +44,7 @@ public class MetaDataStoreTest extends CrashlyticsTestCase {
   private static final String ESCAPED = "\ttest\nvalue";
 
   private FileStore fileStore;
-  private CrashlyticsBackgroundWorker worker = new CrashlyticsBackgroundWorker(Runnable::run);
+  private final CrashlyticsBackgroundWorker worker = new CrashlyticsBackgroundWorker(Runnable::run);
 
   private MetaDataStore storeUnderTest;
 
@@ -83,7 +87,7 @@ public void testWriteUserData_singleField() {
   public void testWriteUserData_null() {
     storeUnderTest.writeUserData(SESSION_ID_1, metadataWithUserId(SESSION_ID_1, null).getUserId());
     UserMetadata userData = UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, worker);
-    assertEquals(null, userData.getUserId());
+    assertNull(userData.getUserId());
   }
 
   public void testWriteUserData_emptyString() {
@@ -112,6 +116,24 @@ 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 {
+    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() {
     UserMetadata userData = UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, worker);
     assertNull(userData.getUserId());
@@ -237,6 +259,24 @@ 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 {
+    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() {
     final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
     assertEquals(0, readKeys.size());

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.");
     }
@@ -68,8 +68,9 @@ public void writeUserData(String sessionId, String userId) {
   @Nullable
   public String readUserId(String sessionId) {
     final File f = getUserDataFileForSession(sessionId);
-    if (!f.exists()) {
+    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.");
     }
@@ -114,7 +117,8 @@ public Map<String, String> readKeyData(String sessionId) {
   Map<String, String> readKeyData(String sessionId, boolean isInternal) {
     final File f =
         isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
-    if (!f.exists()) {
+    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());
+    }
+  }
 }