Skip to content

Make the session id not depend on fetching the fid #6152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions firebase-crashlytics/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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];
}
Expand Down Expand Up @@ -103,7 +105,8 @@ private static byte[] convertLongToTwoByteBuffer(long value) {
return buf.array();
}

@Override
public String toString() {
return _clsId;
return sessionId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading