Skip to content

Commit 2bf11d2

Browse files
authored
New Crashlytics file system v3 to avoid long file names (#6069)
New Crashlytics file system v3 to avoid long file names. Shorten the Crashlytics folder name itself. If a file name is longer than 40 chars, hash it to a 40 char hash. Also use the new `ProcessDetailsProvider` to get the process name on older Android versions so they can take advantage of the process-aware file system.
1 parent 5fd9a73 commit 2bf11d2

File tree

4 files changed

+77
-31
lines changed

4 files changed

+77
-31
lines changed

firebase-crashlytics/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Unreleased
2-
2+
* [changed] Update the internal file system to handle long file names.
33

44
# 19.0.1
55
* [changed] Improve cold initialization time.

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static org.mockito.ArgumentMatchers.any;
2222
import static org.mockito.ArgumentMatchers.anyBoolean;
2323
import static org.mockito.ArgumentMatchers.anyInt;
24+
import static org.mockito.ArgumentMatchers.anyList;
2425
import static org.mockito.ArgumentMatchers.anyLong;
2526
import static org.mockito.ArgumentMatchers.anyString;
2627
import static org.mockito.ArgumentMatchers.eq;
@@ -265,7 +266,7 @@ public void testNonFatalEvent_addsNoKeysToEventWhenNoneAvailable() {
265266
reportingCoordinator.onBeginSession(sessionId, timestamp);
266267
reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
267268

268-
verify(mockEventAppBuilder, never()).setCustomAttributes(any());
269+
verify(mockEventAppBuilder, never()).setCustomAttributes(anyList());
269270
verify(mockEventAppBuilder, never()).build();
270271
verify(mockEventBuilder, never()).setApp(mockEventApp);
271272
verify(mockEventBuilder).build();
@@ -286,7 +287,7 @@ public void testNonFatalEvent_addRolloutsEvent() {
286287
reportingCoordinator.onBeginSession(sessionId, timestamp);
287288
reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
288289

289-
verify(mockEventAppBuilder, never()).setCustomAttributes(any());
290+
verify(mockEventAppBuilder, never()).setCustomAttributes(anyList());
290291
verify(mockEventAppBuilder, never()).build();
291292
verify(mockEventBuilder, never()).setApp(mockEventApp);
292293
verify(reportMetadata).getRolloutsState();
@@ -386,7 +387,7 @@ public void testFatalEvent_addsNoKeysToEventWhenNoneAvailable() {
386387
reportingCoordinator.onBeginSession(sessionId, timestamp);
387388
reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp);
388389

389-
verify(mockEventAppBuilder, never()).setCustomAttributes(any());
390+
verify(mockEventAppBuilder, never()).setCustomAttributes(anyList());
390391
verify(mockEventAppBuilder, never()).build();
391392
verify(mockEventBuilder, never()).setApp(mockEventApp);
392393
verify(mockEventBuilder).build();
@@ -407,7 +408,7 @@ public void testFatalEvent_addRolloutsToEvent() {
407408
reportingCoordinator.onBeginSession(sessionId, timestamp);
408409
reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp);
409410

410-
verify(mockEventAppBuilder, never()).setCustomAttributes(any());
411+
verify(mockEventAppBuilder, never()).setCustomAttributes(anyList());
411412
verify(mockEventAppBuilder, never()).build();
412413
verify(mockEventBuilder, never()).setApp(mockEventApp);
413414
verify(reportMetadata).getRolloutsState();
@@ -497,7 +498,8 @@ public void onReportSend_successfulReportsAreDeleted() {
497498
when(reportSender.enqueueReport(mockReport1, false)).thenReturn(successfulTask);
498499
when(reportSender.enqueueReport(mockReport2, false)).thenReturn(failedTask);
499500

500-
when(idManager.fetchTrueFid(any())).thenReturn(new FirebaseInstallationId("fid", "authToken"));
501+
when(idManager.fetchTrueFid(anyBoolean()))
502+
.thenReturn(new FirebaseInstallationId("fid", "authToken"));
501503
reportingCoordinator.sendReports(Runnable::run);
502504

503505
verify(reportSender).enqueueReport(mockReport1, false);
@@ -528,8 +530,8 @@ private void mockEventInteractions() {
528530
when(mockEventBuilder.build()).thenReturn(mockEvent);
529531
when(mockEvent.getApp()).thenReturn(mockEventApp);
530532
when(mockEventApp.toBuilder()).thenReturn(mockEventAppBuilder);
531-
when(mockEventAppBuilder.setCustomAttributes(any())).thenReturn(mockEventAppBuilder);
532-
when(mockEventAppBuilder.setInternalKeys(any())).thenReturn(mockEventAppBuilder);
533+
when(mockEventAppBuilder.setCustomAttributes(anyList())).thenReturn(mockEventAppBuilder);
534+
when(mockEventAppBuilder.setInternalKeys(anyList())).thenReturn(mockEventAppBuilder);
533535
when(mockEventAppBuilder.build()).thenReturn(mockEventApp);
534536
when(dataCapture.captureEventData(
535537
any(Throwable.class),

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/persistence/FileStoreTest.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.firebase.crashlytics.internal.persistence;
1616

17+
import static com.google.common.truth.Truth.assertThat;
1718
import static com.google.firebase.crashlytics.internal.persistence.FileStore.sanitizeName;
1819

1920
import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
@@ -31,6 +32,11 @@ protected void setUp() throws Exception {
3132
fileStore = new FileStore(getContext());
3233
}
3334

35+
public void testProcessName() {
36+
// If this test fails, the test setup is missing a process name so fileStore is using v1.
37+
assertThat(fileStore.processName).isEqualTo("com.google.firebase.crashlytics.test");
38+
}
39+
3440
public void testGetCommonFile() {
3541
File commonFile = fileStore.getCommonFile("testCommonFile");
3642
assertFalse(commonFile.exists());
@@ -128,9 +134,13 @@ public void testGetReports() throws Exception {
128134
assertEquals(0, fileStore.getNativeReports().size());
129135
}
130136

131-
public void testSanitizeName() {
132-
assertEquals(
133-
"com.google.my.awesome.app_big.stuff.Happens_Here123___",
134-
sanitizeName("com.google.my.awesome.app:big.stuff.Happens_Here123$%^"));
137+
public void testSanitizeShortName() {
138+
assertThat(sanitizeName("com.google.Process_Name123$%^"))
139+
.isEqualTo("com.google.Process_Name123___");
140+
}
141+
142+
public void testSanitizeLongName() {
143+
assertThat(sanitizeName("com.google.my.awesome.app:big.stuff.Happens_Here123$%^"))
144+
.isEqualTo("ef6c01fc7a1a8d10ecb062a81707f769d39d4210");
135145
}
136146
}

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/persistence/FileStore.java

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,12 @@
1414

1515
package com.google.firebase.crashlytics.internal.persistence;
1616

17-
import android.annotation.SuppressLint;
18-
import android.app.Application;
1917
import android.content.Context;
20-
import android.os.Build.VERSION;
21-
import android.os.Build.VERSION_CODES;
2218
import androidx.annotation.Nullable;
2319
import androidx.annotation.VisibleForTesting;
2420
import com.google.firebase.crashlytics.internal.Logger;
21+
import com.google.firebase.crashlytics.internal.ProcessDetailsProvider;
22+
import com.google.firebase.crashlytics.internal.common.CommonUtils;
2523
import java.io.File;
2624
import java.io.FilenameFilter;
2725
import java.util.Arrays;
@@ -52,14 +50,23 @@
5250
* convention, any use of new File(...) or similar outside of this class is a code smell.
5351
*/
5452
public class FileStore {
53+
54+
/** Deprecated old file system for systems that are not process aware. */
5555
private static final String CRASHLYTICS_PATH_V1 = ".com.google.firebase.crashlytics.files.v1";
56+
57+
/** Deprecated old file system, process aware. Use v3, there is no use for this system anymore. */
5658
private static final String CRASHLYTICS_PATH_V2 = ".com.google.firebase.crashlytics.files.v2";
59+
60+
/** Current file system, avoids long file names. */
61+
private static final String CRASHLYTICS_PATH_V3 = ".crashlytics.v3";
62+
5763
private static final String SESSIONS_PATH = "open-sessions";
5864
private static final String NATIVE_SESSION_SUBDIR = "native";
5965
private static final String REPORTS_PATH = "reports";
6066
private static final String PRIORITY_REPORTS_PATH = "priority-reports";
6167
private static final String NATIVE_REPORTS_PATH = "native-reports";
6268

69+
final String processName;
6370
private final File filesDir;
6471
private final File crashlyticsDir;
6572
private final File sessionsDir;
@@ -68,10 +75,12 @@ public class FileStore {
6875
private final File nativeReportsDir;
6976

7077
public FileStore(Context context) {
78+
processName =
79+
ProcessDetailsProvider.INSTANCE.getCurrentProcessDetails(context).getProcessName();
7180
filesDir = context.getFilesDir();
7281
String crashlyticsPath =
73-
useV2FileSystem()
74-
? CRASHLYTICS_PATH_V2 + File.pathSeparator + sanitizeName(Application.getProcessName())
82+
useV3FileSystem()
83+
? CRASHLYTICS_PATH_V3 + File.separator + sanitizeName(processName)
7584
: CRASHLYTICS_PATH_V1;
7685
crashlyticsDir = prepareBaseDir(new File(filesDir, crashlyticsPath));
7786
sessionsDir = prepareBaseDir(new File(crashlyticsDir, SESSIONS_PATH));
@@ -88,21 +97,35 @@ public void deleteAllCrashlyticsFiles() {
8897
/** Clean up files from previous file systems. */
8998
public void cleanupPreviousFileSystems() {
9099
// Clean up pre-versioned file systems.
91-
cleanupDir(new File(filesDir, ".com.google.firebase.crashlytics"));
92-
cleanupDir(new File(filesDir, ".com.google.firebase.crashlytics-ndk"));
93-
94-
// Clean up v1 file system.
95-
if (useV2FileSystem()) {
96-
cleanupDir(new File(filesDir, CRASHLYTICS_PATH_V1));
100+
cleanupFileSystemDir(".com.google.firebase.crashlytics");
101+
cleanupFileSystemDir(".com.google.firebase.crashlytics-ndk");
102+
103+
// Clean up old versioned file systems.
104+
if (useV3FileSystem()) {
105+
cleanupFileSystemDir(CRASHLYTICS_PATH_V1);
106+
// The v2 file system named dirs like ".com....v2:process_name"
107+
cleanupFileSystemDirs(CRASHLYTICS_PATH_V2 + File.pathSeparator);
97108
}
98109
}
99110

100-
private void cleanupDir(File dir) {
111+
private void cleanupFileSystemDir(String child) {
112+
File dir = new File(filesDir, child);
101113
if (dir.exists() && recursiveDelete(dir)) {
102114
Logger.getLogger().d("Deleted previous Crashlytics file system: " + dir.getPath());
103115
}
104116
}
105117

118+
private void cleanupFileSystemDirs(String prefix) {
119+
if (filesDir.exists()) {
120+
String[] list = filesDir.list((dir, name) -> name.startsWith(prefix));
121+
if (list != null) {
122+
for (String child : list) {
123+
cleanupFileSystemDir(child);
124+
}
125+
}
126+
}
127+
}
128+
106129
static boolean recursiveDelete(File fileOrDirectory) {
107130
File[] files = fileOrDirectory.listFiles();
108131
if (files != null) {
@@ -113,12 +136,16 @@ static boolean recursiveDelete(File fileOrDirectory) {
113136
return fileOrDirectory.delete();
114137
}
115138

116-
/** @return internal File used by Crashlytics, that is not specific to a session */
139+
/**
140+
* @return internal File used by Crashlytics, that is not specific to a session
141+
*/
117142
public File getCommonFile(String filename) {
118143
return new File(crashlyticsDir, filename);
119144
}
120145

121-
/** @return all common (non session specific) files matching the given filter. */
146+
/**
147+
* @return all common (non session specific) files matching the given filter.
148+
*/
122149
public List<File> getCommonFiles(FilenameFilter filter) {
123150
return safeArrayToList(crashlyticsDir.listFiles(filter));
124151
}
@@ -206,14 +233,21 @@ private static <T> List<T> safeArrayToList(@Nullable T[] array) {
206233
return (array == null) ? Collections.emptyList() : Arrays.asList(array);
207234
}
208235

209-
@SuppressLint("AnnotateVersionCheck")
210-
private static boolean useV2FileSystem() {
211-
return VERSION.SDK_INT >= VERSION_CODES.P;
236+
private boolean useV3FileSystem() {
237+
// If the process name is known, use the v3 file system.
238+
return !processName.isEmpty();
212239
}
213240

214-
/** Replace potentially unsafe chars with underscores to make a safe file name. */
241+
/**
242+
* Replace potentially unsafe chars with underscores to make a safe file name.
243+
*
244+
* <p>If the filename is too long, hash it to a short name.
245+
*/
215246
@VisibleForTesting
216247
static String sanitizeName(String filename) {
248+
if (filename.length() > 40) {
249+
return CommonUtils.sha1(filename);
250+
}
217251
return filename.replaceAll("[^a-zA-Z0-9.]", "_");
218252
}
219253
}

0 commit comments

Comments
 (0)