Skip to content

Commit

Permalink
[QA] Replace SecureRandom with vendored Random (#3783)
Browse files Browse the repository at this point in the history
* Replace SecureRandom with vendored Random

* Changelog

* Fix failing test
  • Loading branch information
romtsn authored Oct 14, 2024
1 parent f79c9c1 commit 2ab34eb
Show file tree
Hide file tree
Showing 13 changed files with 509 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixes

- Deprecate `enableTracing` option ([#3777](https://github.com/getsentry/sentry-java/pull/3777))
- Vendor `java.util.Random` and replace `java.security.SecureRandom` usages ([#3783](https://github.com/getsentry/sentry-java/pull/3783))

## 7.15.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
import io.sentry.protocol.SentryTransaction;
import io.sentry.protocol.User;
import io.sentry.util.HintUtils;
import io.sentry.util.Random;
import java.io.File;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -83,7 +83,7 @@ public final class AnrV2EventProcessor implements BackfillingEventProcessor {

private final @NotNull SentryExceptionFactory sentryExceptionFactory;

private final @Nullable SecureRandom random;
private final @Nullable Random random;

public AnrV2EventProcessor(
final @NotNull Context context,
Expand All @@ -96,7 +96,7 @@ public AnrV2EventProcessor(
final @NotNull Context context,
final @NotNull SentryAndroidOptions options,
final @NotNull BuildInfoProvider buildInfoProvider,
final @Nullable SecureRandom random) {
final @Nullable Random random) {
this.context = ContextUtils.getApplicationContext(context);
this.options = options;
this.buildInfoProvider = buildInfoProvider;
Expand Down Expand Up @@ -180,7 +180,7 @@ private boolean sampleReplay(final @NotNull SentryEvent event) {

try {
// we have to sample here with the old sample rate, because it may change between app launches
final @NotNull SecureRandom random = this.random != null ? this.random : new SecureRandom();
final @NotNull Random random = this.random != null ? this.random : new Random();
final double replayErrorSampleRateDouble = Double.parseDouble(replayErrorSampleRate);
if (replayErrorSampleRateDouble < random.nextDouble()) {
options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ import io.sentry.transport.ICurrentDateProvider
import io.sentry.util.FileUtils
import io.sentry.util.HintUtils
import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
import io.sentry.util.Random
import java.io.Closeable
import java.io.File
import java.security.SecureRandom
import java.util.LinkedList
import java.util.concurrent.atomic.AtomicBoolean
import kotlin.LazyThreadSafetyMode.NONE
Expand Down Expand Up @@ -78,7 +78,7 @@ public class ReplayIntegration(
private var hub: IHub? = null
private var recorder: Recorder? = null
private var gestureRecorder: GestureRecorder? = null
private val random by lazy { SecureRandom() }
private val random by lazy { Random() }
private val rootViewsSpy by lazy(NONE) { RootViewsSpy.install() }

// TODO: probably not everything has to be thread-safe here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ import io.sentry.android.replay.util.submitSafely
import io.sentry.protocol.SentryId
import io.sentry.transport.ICurrentDateProvider
import io.sentry.util.FileUtils
import io.sentry.util.Random
import java.io.File
import java.security.SecureRandom
import java.util.Date
import java.util.concurrent.ScheduledExecutorService

internal class BufferCaptureStrategy(
private val options: SentryOptions,
private val hub: IHub?,
private val dateProvider: ICurrentDateProvider,
private val random: SecureRandom,
private val random: Random,
executor: ScheduledExecutorService? = null,
replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
) : BaseCaptureStrategy(options, hub, dateProvider, executor = executor, replayCacheProvider = replayCacheProvider) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package io.sentry.android.replay.util

import java.security.SecureRandom
import io.sentry.util.Random

internal fun SecureRandom.sample(rate: Double?): Boolean {
internal fun Random.sample(rate: Double?): Boolean {
if (rate != null) {
return !(rate < this.nextDouble()) // bad luck
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import io.sentry.android.replay.capture.BufferCaptureStrategyTest.Fixture.Compan
import io.sentry.protocol.SentryId
import io.sentry.transport.CurrentDateProvider
import io.sentry.transport.ICurrentDateProvider
import io.sentry.util.Random
import org.awaitility.kotlin.await
import org.junit.Rule
import org.junit.rules.TemporaryFolder
Expand All @@ -35,7 +36,6 @@ import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import java.io.File
import java.security.SecureRandom
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
Expand Down Expand Up @@ -97,7 +97,7 @@ class BufferCaptureStrategyTest {
options,
hub,
dateProvider,
SecureRandom(),
Random(),
mock {
doAnswer { invocation ->
(invocation.arguments[0] as Runnable).run()
Expand Down
13 changes: 13 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -5808,6 +5808,19 @@ public final class io/sentry/util/PropagationTargetsUtils {
public static fun contain (Ljava/util/List;Ljava/net/URI;)Z
}

public final class io/sentry/util/Random : java/io/Serializable {
public fun <init> ()V
public fun <init> (J)V
public fun nextBoolean ()Z
public fun nextBytes ([B)V
public fun nextDouble ()D
public fun nextFloat ()F
public fun nextInt ()I
public fun nextInt (I)I
public fun nextLong ()J
public fun setSeed (J)V
}

public final class io/sentry/util/SampleRateUtils {
public fun <init> ()V
public static fun isValidProfilesSampleRate (Ljava/lang/Double;)Z
Expand Down
6 changes: 3 additions & 3 deletions sentry/src/main/java/io/sentry/SentryClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
import io.sentry.util.CheckInUtils;
import io.sentry.util.HintUtils;
import io.sentry.util.Objects;
import io.sentry.util.Random;
import io.sentry.util.TracingUtils;
import java.io.Closeable;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -40,7 +40,7 @@ public final class SentryClient implements ISentryClient, IMetricsClient {

private final @NotNull SentryOptions options;
private final @NotNull ITransport transport;
private final @Nullable SecureRandom random;
private final @Nullable Random random;
private final @NotNull SortBreadcrumbsByDate sortBreadcrumbsByDate = new SortBreadcrumbsByDate();
private final @NotNull IMetricsAggregator metricsAggregator;

Expand All @@ -67,7 +67,7 @@ public boolean isEnabled() {
? new MetricsAggregator(options, this)
: NoopMetricsAggregator.getInstance();

this.random = options.getSampleRate() == null ? null : new SecureRandom();
this.random = options.getSampleRate() == null ? null : new Random();
}

private boolean shouldApplyScopeData(
Expand Down
8 changes: 4 additions & 4 deletions sentry/src/main/java/io/sentry/TracesSampler.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.sentry;

import io.sentry.util.Objects;
import java.security.SecureRandom;
import io.sentry.util.Random;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.TestOnly;
Expand All @@ -10,14 +10,14 @@ final class TracesSampler {
private static final @NotNull Double DEFAULT_TRACES_SAMPLE_RATE = 1.0;

private final @NotNull SentryOptions options;
private final @NotNull SecureRandom random;
private final @NotNull Random random;

public TracesSampler(final @NotNull SentryOptions options) {
this(Objects.requireNonNull(options, "options are required"), new SecureRandom());
this(Objects.requireNonNull(options, "options are required"), new Random());
}

@TestOnly
TracesSampler(final @NotNull SentryOptions options, final @NotNull SecureRandom random) {
TracesSampler(final @NotNull SentryOptions options, final @NotNull Random random) {
this.options = options;
this.random = random;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ public void setReplayId(@NotNull SentryId replayId) {

@SuppressWarnings("FutureReturnValueIgnored")
private void serializeToDisk(final @NotNull Runnable task) {
if (Thread.currentThread().getName().contains("SentryExecutor")) {
// we're already on the sentry executor thread, so we can just execute it directly
task.run();
return;
}

try {
options
.getExecutorService()
Expand Down
4 changes: 2 additions & 2 deletions sentry/src/main/java/io/sentry/metrics/MetricsHelper.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.sentry.metrics;

import io.sentry.MeasurementUnit;
import java.security.SecureRandom;
import io.sentry.util.Random;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -27,7 +27,7 @@ public final class MetricsHelper {
private static final char TAGS_ESCAPE_CHAR = '\\';

private static long FLUSH_SHIFT_MS =
(long) (new SecureRandom().nextFloat() * (ROLLUP_IN_SECONDS * 1000f));
(long) (new Random().nextFloat() * (ROLLUP_IN_SECONDS * 1000f));

public static long getTimeBucketKey(final long timestampMs) {
final long seconds = timestampMs / 1000;
Expand Down
Loading

0 comments on commit 2ab34eb

Please sign in to comment.