Isolate cached events with hashed dsn subfolder (#2038)

marandaneto · web-flow · commit 3f1055651082 · 2022-05-19T09:40:30.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,10 @@
 - Include application permissions in Android events ([#2018](https://github.com/getsentry/sentry-java/pull/2018))
 - Automatically create transactions for UI events ([#1975](https://github.com/getsentry/sentry-java/pull/1975))
 
+### Fixes
+
+- Isolate cached events with hashed DSN subfolder ([#2038](https://github.com/getsentry/sentry-java/pull/2038))
+
 ### Changed
 
 - Update sentry-native to 0.4.17 ([#2033](https://github.com/getsentry/sentry-java/pull/2033))
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
@@ -299,9 +299,7 @@ private static void readDefaultOptionValues(
   private static void initializeCacheDirs(
       final @NotNull Context context, final @NotNull SentryAndroidOptions options) {
     final File cacheDir = new File(context.getCacheDir(), "sentry");
-    final File profilingTracesDir = new File(cacheDir, "profiling_traces");
     options.setCacheDirPath(cacheDir.getAbsolutePath());
-    options.setProfilingTracesDirPath(profilingTracesDir.getAbsolutePath());
   }
 
   private static boolean isNdkAvailable(final @NotNull BuildInfoProvider buildInfoProvider) {
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java
@@ -73,7 +73,7 @@ private void init() {
       options.getLogger().log(SentryLevel.INFO, "Profiling is disabled in options.");
       return;
     }
-    if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) {
+    if (tracesFilesDirPath == null) {
       options
           .getLogger()
           .log(
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/NdkIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/NdkIntegration.java
@@ -40,7 +40,7 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions
     // up by another integration (EnvelopeFileObserverIntegration).
     if (enabled && sentryNdkClass != null) {
       final String cachedDir = this.options.getCacheDirPath();
-      if (cachedDir == null || cachedDir.isEmpty()) {
+      if (cachedDir == null) {
         this.options.getLogger().log(SentryLevel.ERROR, "No cache dir path is defined in options.");
         disableNdkIntegration(this.options);
         return;
diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt
@@ -128,7 +128,7 @@ class AndroidTransactionProfilerTest {
     @Test
     fun `profiler evaluates profilingTracesDirPath options only on first transaction profiling`() {
         fixture.options.apply {
-            profilingTracesDirPath = ""
+            cacheDirPath = null
         }
 
         // We create the profiler, and nothing goes wrong
@@ -174,7 +174,7 @@ class AndroidTransactionProfilerTest {
     @Test
     fun `profiler on tracesDirPath null`() {
         fixture.options.apply {
-            profilingTracesDirPath = null
+            cacheDirPath = null
         }
         val profiler = fixture.getSut(context)
         profiler.onTransactionStart(fixture.transaction1)
@@ -185,7 +185,7 @@ class AndroidTransactionProfilerTest {
     @Test
     fun `profiler on tracesDirPath empty`() {
         fixture.options.apply {
-            profilingTracesDirPath = ""
+            cacheDirPath = null
         }
         val profiler = fixture.getSut(context)
         profiler.onTransactionStart(fixture.transaction1)
diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml
@@ -45,7 +45,7 @@
             android:name=".ThirdActivityFragment"
             android:exported="false" />
 
-      <activity
+        <activity
             android:name=".GesturesActivity"
             android:exported="false" />
 
diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -1238,7 +1238,6 @@ public class io/sentry/SentryOptions {
 	public fun setMaxTraceFileSize (J)V
 	public fun setPrintUncaughtStackTrace (Z)V
 	public fun setProfilingEnabled (Z)V
-	public fun setProfilingTracesDirPath (Ljava/lang/String;)V
 	public fun setProguardUuid (Ljava/lang/String;)V
 	public fun setProxy (Lio/sentry/SentryOptions$Proxy;)V
 	public fun setReadTimeoutMillis (I)V
@@ -2963,6 +2962,7 @@ public final class io/sentry/util/Platform {
 
 public final class io/sentry/util/StringUtils {
 	public static fun byteCountToString (J)Ljava/lang/String;
+	public static fun calculateStringHash (Ljava/lang/String;Lio/sentry/ILogger;)Ljava/lang/String;
 	public static fun capitalize (Ljava/lang/String;)Ljava/lang/String;
 	public static fun getStringAfterDot (Ljava/lang/String;)Ljava/lang/String;
 	public static fun removeSurrounding (Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;
diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java
@@ -24,7 +24,7 @@ public interface SendFireAndForgetFactory {
     SendFireAndForget create(@NotNull IHub hub, @NotNull SentryOptions options);
 
     default boolean hasValidPath(final @Nullable String dirPath, final @NotNull ILogger logger) {
-      if (dirPath == null || dirPath.isEmpty()) {
+      if (dirPath == null) {
         logger.log(SentryLevel.INFO, "No cached dir path is defined in options.");
         return false;
       }
diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java
@@ -220,23 +220,23 @@ private static boolean initConfigurations(final @NotNull SentryOptions options)
     // eg release, distinctId, sentryClientName
 
     // this should be after setting serializers
-    if (options.getOutboxPath() != null) {
-      final File outboxDir = new File(options.getOutboxPath());
+    final String outboxPath = options.getOutboxPath();
+    if (outboxPath != null) {
+      final File outboxDir = new File(outboxPath);
       outboxDir.mkdirs();
     } else {
       logger.log(SentryLevel.INFO, "No outbox dir path is defined in options.");
     }
 
-    if (options.getCacheDirPath() != null && !options.getCacheDirPath().isEmpty()) {
-      final File cacheDir = new File(options.getCacheDirPath());
+    final String cacheDirPath = options.getCacheDirPath();
+    if (cacheDirPath != null) {
+      final File cacheDir = new File(cacheDirPath);
       cacheDir.mkdirs();
       options.setEnvelopeDiskCache(EnvelopeCache.create(options));
     }
 
     final String profilingTracesDirPath = options.getProfilingTracesDirPath();
-    if (options.isProfilingEnabled()
-        && profilingTracesDirPath != null
-        && !profilingTracesDirPath.isEmpty()) {
+    if (options.isProfilingEnabled() && profilingTracesDirPath != null) {
 
       final File profilingTracesDir = new File(profilingTracesDirPath);
       profilingTracesDir.mkdirs();
diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java
@@ -10,6 +10,7 @@
 import io.sentry.transport.NoOpEnvelopeCache;
 import io.sentry.transport.NoOpTransportGate;
 import io.sentry.util.Platform;
+import io.sentry.util.StringUtils;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -55,6 +56,9 @@ public class SentryOptions {
    */
   private @Nullable String dsn;
 
+  /** dsnHash is used as a subfolder of cacheDirPath to isolate events when rotating DSNs */
+  private @Nullable String dsnHash;
+
   /**
    * Controls how many seconds to wait before shutting down. Sentry SDKs send events from a
    * background queue and this queue is given a certain amount to drain pending events Default is
@@ -295,9 +299,6 @@ public class SentryOptions {
   /** Control if profiling is enabled or not for transactions */
   private boolean profilingEnabled = false;
 
-  /** The cache dir. path for caching profiling traces */
-  private @Nullable String profilingTracesDirPath;
-
   /** Max trace file size in bytes. */
   private long maxTraceFileSize = 5 * 1024 * 1024;
 
@@ -384,8 +385,10 @@ public void addIntegration(@NotNull Integration integration) {
    *
    * @param dsn the DSN
    */
-  public void setDsn(@Nullable String dsn) {
+  public void setDsn(final @Nullable String dsn) {
     this.dsn = dsn;
+
+    dsnHash = StringUtils.calculateStringHash(this.dsn, logger);
   }
 
   /**
@@ -607,7 +610,11 @@ public void setBeforeBreadcrumb(@Nullable BeforeBreadcrumbCallback beforeBreadcr
    * @return the cache dir. path or null if not set
    */
   public @Nullable String getCacheDirPath() {
-    return cacheDirPath;
+    if (cacheDirPath == null || cacheDirPath.isEmpty()) {
+      return null;
+    }
+
+    return dsnHash != null ? new File(cacheDirPath, dsnHash).getAbsolutePath() : cacheDirPath;
   }
 
   /**
@@ -616,18 +623,19 @@ public void setBeforeBreadcrumb(@Nullable BeforeBreadcrumbCallback beforeBreadcr
    * @return the outbox path or null if not set
    */
   public @Nullable String getOutboxPath() {
-    if (cacheDirPath == null || cacheDirPath.isEmpty()) {
+    final String cacheDirPath = getCacheDirPath();
+    if (cacheDirPath == null) {
       return null;
     }
-    return cacheDirPath + File.separator + "outbox";
+    return new File(cacheDirPath, "outbox").getAbsolutePath();
   }
 
   /**
    * Sets the cache dir. path
    *
    * @param cacheDirPath the cache dir. path
    */
-  public void setCacheDirPath(@Nullable String cacheDirPath) {
+  public void setCacheDirPath(final @Nullable String cacheDirPath) {
     this.cacheDirPath = cacheDirPath;
   }
 
@@ -1496,16 +1504,11 @@ public void setProfilingEnabled(boolean profilingEnabled) {
    * @return the profiling traces dir. path or null if not set
    */
   public @Nullable String getProfilingTracesDirPath() {
-    return profilingTracesDirPath;
-  }
-
-  /**
-   * Sets the profiling traces dir. path
-   *
-   * @param profilingTracesDirPath the profiling traces dir. path
-   */
-  public void setProfilingTracesDirPath(@Nullable String profilingTracesDirPath) {
-    this.profilingTracesDirPath = profilingTracesDirPath;
+    final String cacheDirPath = getCacheDirPath();
+    if (cacheDirPath == null) {
+      return null;
+    }
+    return new File(cacheDirPath, "profiling_traces").getAbsolutePath();
   }
 
   /**
diff --git a/sentry/src/main/java/io/sentry/util/StringUtils.java b/sentry/src/main/java/io/sentry/util/StringUtils.java
@@ -1,5 +1,11 @@
 package io.sentry.util;
 
+import io.sentry.ILogger;
+import io.sentry.SentryLevel;
+import java.math.BigInteger;
+import java.nio.charset.Charset;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.text.CharacterIterator;
 import java.text.StringCharacterIterator;
 import java.util.Locale;
@@ -10,6 +16,8 @@
 @ApiStatus.Internal
 public final class StringUtils {
 
+  private static final Charset UTF_8 = Charset.forName("UTF-8");
+
   private StringUtils() {}
 
   public static @Nullable String getStringAfterDot(final @Nullable String str) {
@@ -72,4 +80,45 @@ private StringUtils() {}
     }
     return String.format(Locale.ROOT, "%.1f %cB", bytes / 1000.0, ci.current());
   }
+
+  /**
+   * Calculates the SHA-1 String hash
+   *
+   * @param str the String
+   * @param logger the Logger
+   * @return The hashed String or null otherwise
+   */
+  public static @Nullable String calculateStringHash(
+      final @Nullable String str, final @NotNull ILogger logger) {
+    if (str == null || str.isEmpty()) {
+      return null;
+    }
+
+    try {
+      // getInstance() method is called with algorithm SHA-1
+      final MessageDigest md = MessageDigest.getInstance("SHA-1");
+
+      // digest() method is called
+      // to calculate message digest of the input string
+      // returned as array of byte
+      final byte[] messageDigest = md.digest(str.getBytes(UTF_8));
+
+      // Convert byte array into signum representation
+      final BigInteger no = new BigInteger(1, messageDigest);
+
+      // Convert message digest into hex value
+      final StringBuilder stringBuilder = new StringBuilder(no.toString(16));
+
+      // return the HashText
+      return stringBuilder.toString();
+    }
+
+    // For specifying wrong message digest algorithms
+    catch (NoSuchAlgorithmException e) {
+      logger.log(SentryLevel.INFO, "SHA-1 isn't available to calculate the hash.", e);
+    } catch (Throwable e) {
+      logger.log(SentryLevel.INFO, "string: %s could not calculate its hash", e, str);
+    }
+    return null;
+  }
 }
diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt
@@ -26,6 +26,7 @@ import io.sentry.protocol.SentryTransaction
 import io.sentry.protocol.User
 import io.sentry.test.callMethod
 import io.sentry.util.HintUtils
+import io.sentry.util.StringUtils
 import java.io.File
 import java.nio.file.Files
 import java.util.Queue
@@ -1358,7 +1359,7 @@ class HubTest {
 
     @Test
     fun `isCrashedLastRun does not delete native marker if auto session is enabled`() {
-        val nativeMarker = File(file.absolutePath, EnvelopeCache.NATIVE_CRASH_MARKER_FILE)
+        val nativeMarker = File(hashedFolder(), EnvelopeCache.NATIVE_CRASH_MARKER_FILE)
         nativeMarker.mkdirs()
         nativeMarker.createNewFile()
         val hub = generateHub() as Hub
@@ -1369,7 +1370,7 @@ class HubTest {
 
     @Test
     fun `isCrashedLastRun deletes the native marker if auto session is disabled`() {
-        val nativeMarker = File(file.absolutePath, EnvelopeCache.NATIVE_CRASH_MARKER_FILE)
+        val nativeMarker = File(hashedFolder(), EnvelopeCache.NATIVE_CRASH_MARKER_FILE)
         nativeMarker.mkdirs()
         nativeMarker.createNewFile()
         val hub = generateHub {
@@ -1380,9 +1381,11 @@ class HubTest {
         assertFalse(nativeMarker.exists())
     }
 
+    private val dsnTest = "https://key@sentry.io/proj"
+
     private fun generateHub(optionsConfiguration: Sentry.OptionsConfiguration<SentryOptions>? = null): IHub {
         val options = SentryOptions().apply {
-            dsn = "https://key@sentry.io/proj"
+            dsn = dsnTest
             cacheDirPath = file.absolutePath
             setSerializer(mock())
             tracesSampleRate = 1.0
@@ -1402,4 +1405,10 @@ class HubTest {
         sut.bindClient(mockClient)
         return Pair(sut, mockClient)
     }
+
+    private fun hashedFolder(): String {
+        val hash = StringUtils.calculateStringHash(dsnTest, mock())
+        val fileHashFolder = File(file.absolutePath, hash!!)
+        return fileHashFolder.absolutePath
+    }
 }
diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt
@@ -1,6 +1,7 @@
 package io.sentry
 
 import com.nhaarman.mockitokotlin2.mock
+import io.sentry.util.StringUtils
 import java.io.File
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -269,6 +270,18 @@ class SentryOptionsTest {
     }
 
     @Test
+    fun `when setting dsn, calculates hash and add as subfolder of caching dirs`() {
+        val dsn = "http://key@localhost/proj"
+        val hash = StringUtils.calculateStringHash(dsn, mock())
+        val options = SentryOptions().apply {
+            setDsn(dsn)
+            cacheDirPath = "${File.separator}test"
+        }
+
+        assertEquals("${File.separator}test${File.separator}${hash}${File.separator}outbox", options.outboxPath)
+        assertEquals("${File.separator}test${File.separator}${hash}${File.separator}profiling_traces", options.profilingTracesDirPath)
+    }
+
     fun `when options are initialized, idleTimeout is 3000`() {
         assertEquals(3000L, SentryOptions().idleTimeout)
     }
diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt
diff --git a/sentry/src/test/java/io/sentry/util/StringUtilsTest.kt b/sentry/src/test/java/io/sentry/util/StringUtilsTest.kt

Original file line number	Diff line number	Diff line change
`@@ -299,9 +299,7 @@ private static void readDefaultOptionValues(`
`299`	`299`	`private static void initializeCacheDirs(`
`300`	`300`	`final @NotNull Context context, final @NotNull SentryAndroidOptions options) {`
`301`	`301`	`final File cacheDir = new File(context.getCacheDir(), "sentry");`
`302`		`- final File profilingTracesDir = new File(cacheDir, "profiling_traces");`
`303`	`302`	`options.setCacheDirPath(cacheDir.getAbsolutePath());`
`304`		`- options.setProfilingTracesDirPath(profilingTracesDir.getAbsolutePath());`
`305`	`303`	`}`
`306`	`304`
`307`	`305`	`private static boolean isNdkAvailable(final @NotNull BuildInfoProvider buildInfoProvider) {`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ private void init() {`
`73`	`73`	`options.getLogger().log(SentryLevel.INFO, "Profiling is disabled in options.");`
`74`	`74`	`return;`
`75`	`75`	`}`
`76`		`- if (tracesFilesDirPath == null \|\| tracesFilesDirPath.isEmpty()) {`
	`76`	`+ if (tracesFilesDirPath == null) {`
`77`	`77`	`options`
`78`	`78`	`.getLogger()`
`79`	`79`	`.log(`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ public interface SendFireAndForgetFactory {`
`24`	`24`	`SendFireAndForget create(@NotNull IHub hub, @NotNull SentryOptions options);`
`25`	`25`
`26`	`26`	`default boolean hasValidPath(final @Nullable String dirPath, final @NotNull ILogger logger) {`
`27`		`- if (dirPath == null \|\| dirPath.isEmpty()) {`
	`27`	`+ if (dirPath == null) {`
`28`	`28`	`logger.log(SentryLevel.INFO, "No cached dir path is defined in options.");`
`29`	`29`	`return false;`
`30`	`30`	`}`