Merge 3ee8545 into 57aaf39

Author: mrober

firebase-crashlytics/CHANGELOG.md

@@ -2,6 +2,7 @@
 * [feature] Added the `isCrashlyticsCollectionEnabled` API to check if Crashlytics collection is enabled.
   ([Github #5919](//github.com/firebase/firebase-android-sdk/issues/5919))
 * [fixed] Ensure that on-demand fatal events are never processed on the main thread.
+* [changed] Internal changes to the way session IDs are generated.
 
 
 # 19.0.3

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CLSUUIDTest.java

@@ -14,63 +14,41 @@
 
 package com.google.firebase.crashlytics.internal.common;
 
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static com.google.common.truth.Truth.assertThat;
 
-import com.google.android.gms.tasks.Tasks;
-import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
-import com.google.firebase.installations.FirebaseInstallationsApi;
+import java.util.ArrayList;
+import org.junit.Test;
 
-public class CLSUUIDTest extends CrashlyticsTestCase {
-
-  private IdManager idManager;
-  private CLSUUID uuid;
-
-  protected void setUp() throws Exception {
-    super.setUp();
-    FirebaseInstallationsApi installationsApiMock = mock(FirebaseInstallationsApi.class);
-    when(installationsApiMock.getId()).thenReturn(Tasks.forResult("instanceId"));
-    idManager =
-        new IdManager(
-            getContext(),
-            getContext().getPackageName(),
-            installationsApiMock,
-            DataCollectionArbiterTest.MOCK_ARBITER_ENABLED);
-    uuid = new CLSUUID(idManager);
-  }
-
-  protected void tearDown() throws Exception {
-    super.tearDown();
-    uuid = null;
-  }
+public class CLSUUIDTest {
 
   /** Basic test of the CLSUUID string value. */
-  public void testToString() {
-    final String s = uuid.toString();
-    assertNotNull("The uuid string value should not be null", s);
-    assertEquals("The uuid string value should be 32 chars long", 32, s.length());
+  @Test
+  public void getSessionId() {
+    String sessionId = new CLSUUID().getSessionId();
+    assertThat(sessionId).isNotNull(); // The session id should not be null
+    assertThat(sessionId).hasLength(32); // The session id should be 32 chars long
   }
 
   /** Test that we don't get duplicate CLSUUID string values in a set of 100 uuid generated. */
-  public void testOrder() {
-    final String[] uuids = new String[100];
-    CLSUUID uuid = null;
+  @Test
+  public void sessionIdsInOrder() {
+    ArrayList<String> sessionIds = new ArrayList<>();
 
-    // Put 100 CLSUUID string values into an array.
+    // Put 100 CLSUUID string values into a list.
     for (int i = 0; i < 100; i++) {
-      uuid = new CLSUUID(idManager);
-      uuids[i] = uuid.toString();
+      sessionIds.add(new CLSUUID().getSessionId());
     }
 
-    // Assert that the other 99 CLSUUIDs don't match the current index in the loop.
-    for (int i = 0; i < uuids.length; i++) {
-      final String uuidAtIndex = uuids[i];
-      for (int j = 0; j < uuids.length; j++) {
-        if (i != j) {
-          assertFalse(
-              "We shouldn't have the same uuid at another index.", uuidAtIndex.equals(uuids[j]));
-        }
-      }
-    }
+    assertThat(sessionIds).isInOrder();
+    assertThat(sessionIds).containsNoDuplicates();
+  }
+
+  /** Test that we pad session ids properly. */
+  @Test
+  public void sessionIdsArePadded() {
+    String sessionId1 = new CLSUUID().getSessionId();
+    String sessionId2 = new CLSUUID().getSessionId();
+
+    assertThat(sessionId1.substring(20)).isEqualTo(sessionId2.substring(20));
   }
 }

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CLSUUID.java

@@ -18,44 +18,46 @@
 import java.nio.ByteOrder;
 import java.util.Date;
 import java.util.Locale;
+import java.util.UUID;
 import java.util.concurrent.atomic.AtomicLong;
 
 /**
  * A UUID whose lexicographical ordering is equivalent to a temporal ordering. Uniqueness is based
- * on timestamp, sequence identifier, processId, and a hash of the application installation
- * identifier.
- *
- * <p>Applications which generate colliding app installation identifiers are at risk of creating
- * colliding CLSUUIDs.
+ * on timestamp, sequence identifier, processId, and a random hash.
  */
 class CLSUUID {
-  private static final AtomicLong _sequenceNumber = new AtomicLong(0);
 
-  private static String _clsId;
+  // Padding to ensure the id is long enough, has only valid hex characters, and increase entropy.
+  private static final String ID_SHA =
+      CommonUtils.sha1(UUID.randomUUID().toString() + System.currentTimeMillis());
 
-  CLSUUID(IdManager idManager) {
-    final byte[] bytes = new byte[10];
+  private static final AtomicLong sequenceNumber = new AtomicLong(0);
+  private final String sessionId;
+
+  CLSUUID() {
+    byte[] bytes = new byte[10];
 
     this.populateTime(bytes);
     this.populateSequenceNumber(bytes);
     this.populatePID(bytes);
 
-    // sha1 it to ensure the string is long enough, has only valid hex characters, and to
-    // increase entropy.
-    final String idSha = CommonUtils.sha1(idManager.getInstallIds().getCrashlyticsInstallId());
-    final String timeSeqPid = CommonUtils.hexify(bytes);
+    String timeSeqPid = CommonUtils.hexify(bytes);
 
-    _clsId =
+    sessionId =
         String.format(
                 Locale.US,
                 "%s%s%s%s",
                 timeSeqPid.substring(0, 12),
                 timeSeqPid.substring(12, 16),
                 timeSeqPid.subSequence(16, 20),
-                idSha.substring(0, 12))
+                ID_SHA.substring(0, 12))
             .toUpperCase(Locale.US);
   }
 
+  public String getSessionId() {
+    return sessionId;
+  }
+
   private void populateTime(byte[] bytes) {
     final Date date = new Date();
     final long time = date.getTime();
@@ -73,7 +75,7 @@ private void populateTime(byte[] bytes) {
   }
 
   private void populateSequenceNumber(byte[] bytes) {
-    final byte[] sequenceBytes = convertLongToTwoByteBuffer(_sequenceNumber.incrementAndGet());
+    byte[] sequenceBytes = convertLongToTwoByteBuffer(sequenceNumber.incrementAndGet());
     bytes[6] = sequenceBytes[0];
     bytes[7] = sequenceBytes[1];
   }
@@ -103,7 +105,8 @@ private static byte[] convertLongToTwoByteBuffer(long value) {
     return buf.array();
   }
 
+  @Override
   public String toString() {
-    return _clsId;
+    return sessionId;
   }
 }

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java

@@ -222,7 +222,7 @@ public Task<Void> call() throws Exception {
 
                 doWriteAppExceptionMarker(timestampMillis);
                 doCloseSessions(settingsProvider);
-                doOpenSession(new CLSUUID(idManager).toString(), isOnDemand);
+                doOpenSession(new CLSUUID().getSessionId(), isOnDemand);
 
                 // If automatic data collection is disabled, we'll need to wait until the next run
                 // of the app.

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java

@@ -150,7 +150,7 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider)
       throw new IllegalStateException(MISSING_BUILD_ID_MSG);
     }
 
-    final String sessionIdentifier = new CLSUUID(idManager).toString();
+    String sessionIdentifier = new CLSUUID().getSessionId();
     try {
       crashMarker = new CrashlyticsFileMarker(CRASH_MARKER_FILE_NAME, fileStore);
       initializationMarker = new CrashlyticsFileMarker(INITIALIZATION_MARKER_FILE_NAME, fileStore);