Make the session id not depend on fetching the fid (firebase#6152)

Make generating a session id not depend on fetching the fid. This is needed so we can allow the common worker to initialize, and start accepting use actions, before any data collection, settings, or fid validation happens. The hash of the crashlytics installation id was used to pad the length and increase entropy. Instead of that, we use a random uuid now. Verified nothing depends on the source of that padding by manually setting it to all 0s and doing an end-to-end test. The way the session id is built is a little bit weird, using bytes. In the future, we could rewrite this to use proper string formatting.

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

@@ -211,7 +211,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

@@ -146,7 +146,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);