diff --git a/firebase-crashlytics/CHANGELOG.md b/firebase-crashlytics/CHANGELOG.md index 55cc6d10f0a..937ec9a3752 100644 --- a/firebase-crashlytics/CHANGELOG.md +++ b/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 diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CLSUUIDTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CLSUUIDTest.java index 4480fe25e34..b0477b6f195 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CLSUUIDTest.java +++ b/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 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)); } } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CLSUUID.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CLSUUID.java index ad0f5978522..45801134fe1 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CLSUUID.java +++ b/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. - * - *

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; } } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java index 45b1d914632..813f80d17ef 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java @@ -222,7 +222,7 @@ public Task 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. diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java index 16ef34b4f74..0e9c4b8ee98 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java +++ b/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);