diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index e31db87a4e8..d25122d59ad 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -13,7 +13,7 @@ concurrency: jobs: analyze: name: Analyze - runs-on: macos-15 + runs-on: ["bitrise_pool_name:tahoe"] env: GRADLE_ENCRYPTION_KEY: ${{ secrets.GRADLE_ENCRYPTION_KEY }} diff --git a/.github/workflows/format-code.yml b/.github/workflows/format-code.yml index d236a64b0ce..ed0fa2539a0 100644 --- a/.github/workflows/format-code.yml +++ b/.github/workflows/format-code.yml @@ -5,7 +5,7 @@ on: jobs: format-code: name: Format Code - runs-on: ubuntu-latest + runs-on: ["bitrise_pool_name:tahoe"] steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/generate-javadocs.yml b/.github/workflows/generate-javadocs.yml index 7cb5c903ca3..4bc252f7a30 100644 --- a/.github/workflows/generate-javadocs.yml +++ b/.github/workflows/generate-javadocs.yml @@ -6,7 +6,7 @@ on: jobs: build-and-deploy-javadocs: name: Build and deploy Javadocs - runs-on: ubuntu-latest + runs-on: ["bitrise_pool_name:tahoe"] steps: - name: Checkout 🛎️ uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/integration-tests-macrobenchmark.yml b/.github/workflows/integration-tests-macrobenchmark.yml index 377db931fc6..7ba03815fc4 100644 --- a/.github/workflows/integration-tests-macrobenchmark.yml +++ b/.github/workflows/integration-tests-macrobenchmark.yml @@ -12,7 +12,7 @@ concurrency: jobs: macrobenchmark: name: Macrobenchmark - runs-on: ubuntu-latest + runs-on: ["bitrise_pool_name:tahoe"] # we copy the secret to the env variable in order to access it in the workflow env: diff --git a/.github/workflows/integration-tests-size.yml b/.github/workflows/integration-tests-size.yml index ebdad2a4d9a..530f5ea471d 100644 --- a/.github/workflows/integration-tests-size.yml +++ b/.github/workflows/integration-tests-size.yml @@ -13,7 +13,7 @@ concurrency: jobs: build: name: Build and Analyze SDK Size - runs-on: ubuntu-latest + runs-on: ["bitrise_pool_name:tahoe"] env: GRADLE_ENCRYPTION_KEY: ${{ secrets.GRADLE_ENCRYPTION_KEY }} diff --git a/.github/workflows/integration-tests-ui-critical.yml b/.github/workflows/integration-tests-ui-critical.yml index 27ecbff7d1c..c793151d3d7 100644 --- a/.github/workflows/integration-tests-ui-critical.yml +++ b/.github/workflows/integration-tests-ui-critical.yml @@ -20,7 +20,9 @@ env: jobs: build: name: Build - runs-on: ubuntu-latest + # Only the APK build runs on Bitrise; the Maestro job below needs KVM for the + # x86_64 emulator, which the macOS Bitrise VMs don't provide. + runs-on: ["bitrise_pool_name:tahoe"] env: GRADLE_ENCRYPTION_KEY: ${{ secrets.GRADLE_ENCRYPTION_KEY }} diff --git a/.github/workflows/integration-tests-ui.yml b/.github/workflows/integration-tests-ui.yml index e1f11ac1f8c..2483394aa14 100644 --- a/.github/workflows/integration-tests-ui.yml +++ b/.github/workflows/integration-tests-ui.yml @@ -12,7 +12,7 @@ concurrency: jobs: test: name: Ui tests - runs-on: ubuntu-latest + runs-on: ["bitrise_pool_name:tahoe"] # we copy the secret to the env variable in order to access it in the workflow env: @@ -81,11 +81,12 @@ jobs: # Skip on PRs from forks, which don't have access to the upload secret if: ${{ !cancelled() && env.SAUCE_USERNAME != null && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) }} run: | - shopt -s globstar nullglob - pngs=(artifacts/**/*.png) - if [ ${#pngs[@]} -gt 0 ]; then - mkdir -p replay-snapshots - cp "${pngs[@]}" replay-snapshots/ + # find instead of `shopt -s globstar`, which macOS's bash 3.2 doesn't support + mkdir -p replay-snapshots + if [ -d artifacts ]; then + find artifacts -type f -name '*.png' -exec cp {} replay-snapshots/ \; + fi + if [ -n "$(ls -A replay-snapshots)" ]; then sentry-cli snapshots upload ./replay-snapshots \ --app-id sentry-android-replay else diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 184de7ff3ba..9996b6dfa2f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -23,7 +23,7 @@ jobs: steps: - name: Get auth token id: token - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + uses: actions/create-github-app-token@64ab729a6294000f5aa4a4bc0b73d1f32bc45510 # v3.3.0 with: app-id: ${{ vars.SENTRY_RELEASE_BOT_CLIENT_ID }} private-key: ${{ secrets.SENTRY_RELEASE_BOT_PRIVATE_KEY }} diff --git a/CHANGELOG.md b/CHANGELOG.md index f7bfcc8e154..13f6624bfb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## Unreleased + +### Fixes + +- Keep sending events from Android 7.1.0 and older devices after Sentry changes its TLS certificate authority ([#6227](https://github.com/getsentry/sentry-java/pull/6227)) + - In February 2027, Sentry moves its TLS certificates from DigiCert to Let's Encrypt and Google Trust Services ([announcement](https://sentry.io/changelog/were-changing-our-tls-certificate-authority-in-february-2027)). + - Android 7.1.0 (API 25) and older don't trust the Let's Encrypt root certificate (ISRG Root X1), so without this fix, the SDK on those devices can no longer send events to Sentry after the switch. + - The SDK now bundles ISRG Root X1 and trusts it in addition to the device's root certificates on API 25 and lower. This only applies to the SDK's own uploads to Sentry SaaS (DSNs on `sentry.io`), not to self-hosted Sentry, not to any other connection your app makes, and not if you set your own `SentryOptions.setSslSocketFactory`. + - Apps need to update to an SDK version with this fix to keep sending events from these devices. + +### Internal + +- Measure the hostname cache TTL on the monotonic ticker the options provide, so that a device time change no longer shortens or extends it ([#6100](https://github.com/getsentry/sentry-java/pull/6100)) +- Measure Session Replay's 1h recording cap and its touch-move debounce on a monotonic clock, so a device time change no longer stops a healthy recording early, keeps a finished one alive, or suppresses gesture events ([#6090](https://github.com/getsentry/sentry-java/pull/6090)) + ## 8.61.0 ### Fixes diff --git a/build-logic/build.gradle.kts b/build-logic/build.gradle.kts index bba758f9b79..fc90a792782 100644 --- a/build-logic/build.gradle.kts +++ b/build-logic/build.gradle.kts @@ -2,10 +2,6 @@ plugins { `kotlin-dsl` } -repositories { - gradlePluginPortal() -} - dependencies { implementation(libs.animalsniffer.gradle.plugin) implementation(libs.spotlessLib) diff --git a/build-logic/settings.gradle.kts b/build-logic/settings.gradle.kts index aa5e146f1c7..58fadd9f178 100644 --- a/build-logic/settings.gradle.kts +++ b/build-logic/settings.gradle.kts @@ -1,4 +1,16 @@ +pluginManagement { + repositories { + mavenCentral() + gradlePluginPortal() + } +} + dependencyResolutionManagement { + repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS) + repositories { + mavenCentral() + gradlePluginPortal() + } versionCatalogs { create("libs") { from(files("../gradle/libs.versions.toml")) diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts index 451e5827ed9..4459ee342fe 100644 --- a/buildSrc/build.gradle.kts +++ b/buildSrc/build.gradle.kts @@ -4,10 +4,6 @@ plugins { `kotlin-dsl` } -repositories { - mavenCentral() -} - tasks.withType().configureEach { compilerOptions.jvmTarget = org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_17 } diff --git a/buildSrc/settings.gradle.kts b/buildSrc/settings.gradle.kts index 12b905adc13..e457d888b84 100644 --- a/buildSrc/settings.gradle.kts +++ b/buildSrc/settings.gradle.kts @@ -1 +1,15 @@ +pluginManagement { + repositories { + mavenCentral() + gradlePluginPortal() + } +} + +dependencyResolutionManagement { + repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS) + repositories { + mavenCentral() + } +} + rootProject.name = "sentry-buildSrc" 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 index 12ffb90356c..7626acfb1ba 100644 --- 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 @@ -202,6 +202,14 @@ static void initializeIntegrationsAndProcessors( if (options.getTransportGate() instanceof NoOpTransportGate) { options.setTransportGate(new AndroidTransportGate(options)); } + // API 25 and lower may lack ISRG Root X1 (Let's Encrypt), so the SDK's own envelope uploads to + // Sentry SaaS trust the bundled root in addition to the system ones. Self-hosted DSNs keep the + // platform default, so Network Security Config domain-config trust anchors still apply there. + if (buildInfoProvider.getSdkInfoVersion() <= Build.VERSION_CODES.N_MR1 + && options.getSslSocketFactory() == null + && SentryRootCaSslSocketFactory.isSentrySaasDsn(options.getDsn())) { + options.setSslSocketFactory(new SentryRootCaSslSocketFactory(options.getLogger())); + } final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); options.setAppStartExtender(appStartMetrics.getAppStartExtension()); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryRootCaSslSocketFactory.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryRootCaSslSocketFactory.java new file mode 100644 index 00000000000..00e4ec17696 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryRootCaSslSocketFactory.java @@ -0,0 +1,211 @@ +package io.sentry.android.core; + +import io.sentry.DsnUtil; +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import io.sentry.util.LazyEvaluator; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.InetAddress; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.cert.Certificate; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; +import java.util.Locale; +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.X509TrustManager; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; +import org.jetbrains.annotations.VisibleForTesting; + +/** + * An {@link SSLSocketFactory} that trusts the system root CAs plus the root CAs bundled in {@link + * SentryRootCertificates}. Both are put into a single trust store which is handed to the platform's + * default {@link TrustManagerFactory}, so certificate validation itself is still done by the + * platform. + * + *

Android API 25 and lower may not ship ISRG Root X1, so TLS handshakes with Let's Encrypt + * certificates fail on those devices. This factory is only set as {@link + * io.sentry.SentryOptions#setSslSocketFactory(SSLSocketFactory)} on those API levels and only when + * the DSN points to Sentry SaaS ({@code sentry.io}), see {@link #isSentrySaasDsn(String)}. So it + * only applies to the SDK's own envelope uploads to Sentry and not to any other connection of the + * app. + * + *

On API 24 and 25, the system CAs are read from the Network Security Config's base config. Any + * {@code domain-config} trust anchors or pins do not apply to connections using this factory. This + * is why it's limited to Sentry SaaS: self-hosted Sentry with a private CA keeps using the platform + * default. + * + *

The underlying {@link SSLContext} is created lazily on first use, so the cost of parsing the + * certificates is paid on the transport thread instead of during {@code SentryAndroid.init}. If + * creating it fails for any reason, the platform default {@link SSLSocketFactory} is used and the + * failure is not retried. + */ +@ApiStatus.Internal +final class SentryRootCaSslSocketFactory extends SSLSocketFactory { + + static final @NotNull String SENTRY_ROOT_CA_ALIAS_PREFIX = "sentry-root-ca-"; + + private final @NotNull LazyEvaluator delegate; + + /** Provides the CAs trusted by the platform. */ + interface SystemTrustedIssuersProvider { + @NotNull + X509Certificate[] getSystemTrustedIssuers() throws GeneralSecurityException; + } + + SentryRootCaSslSocketFactory(final @NotNull ILogger logger) { + this(logger, SentryRootCaSslSocketFactory::getSystemTrustedIssuers); + } + + @VisibleForTesting + SentryRootCaSslSocketFactory( + final @NotNull ILogger logger, + final @NotNull SystemTrustedIssuersProvider systemTrustedIssuersProvider) { + this.delegate = new LazyEvaluator<>(() -> createDelegate(logger, systemTrustedIssuersProvider)); + } + + /** + * Returns {@code true} if the DSN's host is {@code sentry.io} or a subdomain of it, for example + * {@code o123.ingest.us.sentry.io}. Those are the hosts whose certificates chain to the bundled + * roots. The host is read with {@link DsnUtil#getHost(String)}, which parses it the same way as + * the DSN the SDK sends envelopes to. + */ + static boolean isSentrySaasDsn(final @Nullable String dsn) { + @Nullable String host = DsnUtil.getHost(dsn); + if (host == null) { + return false; + } + if (host.endsWith(".")) { + host = host.substring(0, host.length() - 1); + } + host = host.toLowerCase(Locale.ROOT); + return host.equals("sentry.io") || host.endsWith(".sentry.io"); + } + + private static @NotNull SSLSocketFactory createDelegate( + final @NotNull ILogger logger, + final @NotNull SystemTrustedIssuersProvider systemTrustedIssuersProvider) { + try { + final @NotNull TrustManagerFactory trustManagerFactory = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + trustManagerFactory.init( + createTrustStore(systemTrustedIssuersProvider.getSystemTrustedIssuers())); + + final @NotNull SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(null, trustManagerFactory.getTrustManagers(), null); + return sslContext.getSocketFactory(); + } catch (Exception e) { + // Catches Exception, not only GeneralSecurityException and IOException: some OEM builds + // throw RuntimeExceptions from their trust store or certificate code. An exception escaping + // here would make every envelope upload fail without being retried, and the LazyEvaluator + // would try to build the delegate again on every send. Errors such as OutOfMemoryError are + // not caught on purpose, the process can't recover from them. + logger.log( + SentryLevel.ERROR, + "Failed to create SSLSocketFactory with bundled Sentry root CAs, using the default one.", + e); + return HttpsURLConnection.getDefaultSSLSocketFactory(); + } + } + + /** Returns the CAs trusted by the platform's default trust manager. */ + private static @NotNull X509Certificate[] getSystemTrustedIssuers() + throws GeneralSecurityException { + final @NotNull TrustManagerFactory trustManagerFactory = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + trustManagerFactory.init((KeyStore) null); + for (final TrustManager trustManager : trustManagerFactory.getTrustManagers()) { + if (trustManager instanceof X509TrustManager) { + return ((X509TrustManager) trustManager).getAcceptedIssuers(); + } + } + throw new GeneralSecurityException("No default X509TrustManager available"); + } + + /** + * Creates a trust store containing the given system trusted CAs plus the bundled Sentry root CAs. + */ + @VisibleForTesting + static @NotNull KeyStore createTrustStore(final @NotNull X509Certificate[] systemTrustedIssuers) + throws GeneralSecurityException, IOException { + final @NotNull KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + keyStore.load(null, null); + for (int i = 0; i < systemTrustedIssuers.length; i++) { + keyStore.setCertificateEntry("system-ca-" + i, systemTrustedIssuers[i]); + } + final @NotNull CertificateFactory certificateFactory = CertificateFactory.getInstance("X.509"); + for (int i = 0; i < SentryRootCertificates.ALL.length; i++) { + final @NotNull Certificate certificate = + certificateFactory.generateCertificate( + new ByteArrayInputStream( + SentryRootCertificates.ALL[i].getBytes(StandardCharsets.UTF_8))); + keyStore.setCertificateEntry(SENTRY_ROOT_CA_ALIAS_PREFIX + i, certificate); + } + return keyStore; + } + + @TestOnly + @NotNull + SSLSocketFactory getDelegate() { + return delegate.getValue(); + } + + @Override + public String[] getDefaultCipherSuites() { + return delegate.getValue().getDefaultCipherSuites(); + } + + @Override + public String[] getSupportedCipherSuites() { + return delegate.getValue().getSupportedCipherSuites(); + } + + @Override + public Socket createSocket() throws IOException { + return delegate.getValue().createSocket(); + } + + @Override + public Socket createSocket( + final Socket socket, final String host, final int port, final boolean autoClose) + throws IOException { + return delegate.getValue().createSocket(socket, host, port, autoClose); + } + + @Override + public Socket createSocket(final String host, final int port) throws IOException { + return delegate.getValue().createSocket(host, port); + } + + @Override + public Socket createSocket( + final String host, final int port, final InetAddress localHost, final int localPort) + throws IOException { + return delegate.getValue().createSocket(host, port, localHost, localPort); + } + + @Override + public Socket createSocket(final InetAddress host, final int port) throws IOException { + return delegate.getValue().createSocket(host, port); + } + + @Override + public Socket createSocket( + final InetAddress address, + final int port, + final InetAddress localAddress, + final int localPort) + throws IOException { + return delegate.getValue().createSocket(address, port, localAddress, localPort); + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryRootCertificates.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryRootCertificates.java new file mode 100644 index 00000000000..f6a53189adb --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryRootCertificates.java @@ -0,0 +1,65 @@ +package io.sentry.android.core; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +/** + * Root CA certificates the SDK bundles for its own envelope uploads, see {@link + * SentryRootCaSslSocketFactory}. + * + *

Sentry SaaS uses Let's Encrypt, whose default certificate chains all end at ISRG Root X1 (see + * https://docs.sentry.io/security-legal-pii/security/ssl/ and + * https://letsencrypt.org/certificates/). ISRG Root X1 was only added to the Android CA store in + * Android 7.1 (API 25), so older devices can't validate those chains on their own. It's also + * applied on API 25 to cover vendor builds that lack it. The other Sentry roots (DigiCert, and + * Google Trust Services via its GlobalSign cross-sign) are already trusted on all supported API + * levels. + */ +@ApiStatus.Internal +final class SentryRootCertificates { + + private SentryRootCertificates() {} + + /** + * ISRG Root X1, SHA-256 + * 96:BC:EC:06:26:49:76:F3:74:60:77:9A:CF:28:C5:A7:CF:E8:A3:C0:AA:E1:1A:8F:FC:EE:05:C0:BD:DF:08:C6 + */ + static final @NotNull String ISRG_ROOT_X1 = + "-----BEGIN CERTIFICATE-----\n" + + "MIIFazCCA1OgAwIBAgIRAIIQz7DSQONZRGPgu2OCiwAwDQYJKoZIhvcNAQELBQAw\n" + + "TzELMAkGA1UEBhMCVVMxKTAnBgNVBAoTIEludGVybmV0IFNlY3VyaXR5IFJlc2Vh\n" + + "cmNoIEdyb3VwMRUwEwYDVQQDEwxJU1JHIFJvb3QgWDEwHhcNMTUwNjA0MTEwNDM4\n" + + "WhcNMzUwNjA0MTEwNDM4WjBPMQswCQYDVQQGEwJVUzEpMCcGA1UEChMgSW50ZXJu\n" + + "ZXQgU2VjdXJpdHkgUmVzZWFyY2ggR3JvdXAxFTATBgNVBAMTDElTUkcgUm9vdCBY\n" + + "MTCCAiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIBAK3oJHP0FDfzm54rVygc\n" + + "h77ct984kIxuPOZXoHj3dcKi/vVqbvYATyjb3miGbESTtrFj/RQSa78f0uoxmyF+\n" + + "0TM8ukj13Xnfs7j/EvEhmkvBioZxaUpmZmyPfjxwv60pIgbz5MDmgK7iS4+3mX6U\n" + + "A5/TR5d8mUgjU+g4rk8Kb4Mu0UlXjIB0ttov0DiNewNwIRt18jA8+o+u3dpjq+sW\n" + + "T8KOEUt+zwvo/7V3LvSye0rgTBIlDHCNAymg4VMk7BPZ7hm/ELNKjD+Jo2FR3qyH\n" + + "B5T0Y3HsLuJvW5iB4YlcNHlsdu87kGJ55tukmi8mxdAQ4Q7e2RCOFvu396j3x+UC\n" + + "B5iPNgiV5+I3lg02dZ77DnKxHZu8A/lJBdiB3QW0KtZB6awBdpUKD9jf1b0SHzUv\n" + + "KBds0pjBqAlkd25HN7rOrFleaJ1/ctaJxQZBKT5ZPt0m9STJEadao0xAH0ahmbWn\n" + + "OlFuhjuefXKnEgV4We0+UXgVCwOPjdAvBbI+e0ocS3MFEvzG6uBQE3xDk3SzynTn\n" + + "jh8BCNAw1FtxNrQHusEwMFxIt4I7mKZ9YIqioymCzLq9gwQbooMDQaHWBfEbwrbw\n" + + "qHyGO0aoSCqI3Haadr8faqU9GY/rOPNk3sgrDQoo//fb4hVC1CLQJ13hef4Y53CI\n" + + "rU7m2Ys6xt0nUW7/vGT1M0NPAgMBAAGjQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNV\n" + + "HRMBAf8EBTADAQH/MB0GA1UdDgQWBBR5tFnme7bl5AFzgAiIyBpY9umbbjANBgkq\n" + + "hkiG9w0BAQsFAAOCAgEAVR9YqbyyqFDQDLHYGmkgJykIrGF1XIpu+ILlaS/V9lZL\n" + + "ubhzEFnTIZd+50xx+7LSYK05qAvqFyFWhfFQDlnrzuBZ6brJFe+GnY+EgPbk6ZGQ\n" + + "3BebYhtF8GaV0nxvwuo77x/Py9auJ/GpsMiu/X1+mvoiBOv/2X/qkSsisRcOj/KK\n" + + "NFtY2PwByVS5uCbMiogziUwthDyC3+6WVwW6LLv3xLfHTjuCvjHIInNzktHCgKQ5\n" + + "ORAzI4JMPJ+GslWYHb4phowim57iaztXOoJwTdwJx4nLCgdNbOhdjsnvzqvHu7Ur\n" + + "TkXWStAmzOVyyghqpZXjFaH3pO3JLF+l+/+sKAIuvtd7u+Nxe5AW0wdeRlN8NwdC\n" + + "jNPElpzVmbUq4JUagEiuTDkHzsxHpFKVK7q4+63SM1N95R1NbdWhscdCb+ZAJzVc\n" + + "oyi3B43njTOQ5yOf+1CceWxG1bQVs5ZufpsMljq4Ui0/1lvh+wjChP4kqKOJ2qxq\n" + + "4RgqsahDYVvTH9w7jXbyLeiNdd8XM2w9U/t7y0Ff/9yi0GE44Za4rF2LN9d11TPA\n" + + "mRGunUHBcnWEvgJBQl9nJEiU0Zsnvgc/ubhPgXRR4Xq37Z0j4r7g1SgEEzwxA57d\n" + + "emyPxgcYxn/eR44/KJ4EBs+lVDR3veyJm+kXQ99b21/+jh5Xos1AnX5iItreGCc=\n" + + "-----END CERTIFICATE-----\n"; + + // TODO(2028-01-28): Google Trust Services certs are only trusted on API <= 28 via the GTS + // Root R1 cross-sign by GlobalSign Root CA, which expires on 2028-01-28. Before that date, + // bundle GTS Root R1-R4 here and raise the API gate in AndroidOptionsInitializer to <= 28. + // See https://github.com/getsentry/sentry-java/pull/6227 + static final @NotNull String[] ALL = new String[] {ISRG_ROOT_X1}; +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 4610e4bbcb8..99b99b81120 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -60,6 +60,8 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.annotation.Config +private const val SAAS_DSN = "https://key@o1.ingest.us.sentry.io/1" + @RunWith(AndroidJUnit4::class) class AndroidOptionsInitializerTest { class Fixture(val context: Context, private val file: File) { @@ -368,6 +370,44 @@ class AndroidOptionsInitializerTest { assertTrue(fixture.sentryOptions.transportGate is AndroidTransportGate) } + @Config(sdk = [Build.VERSION_CODES.N_MR1]) + @Test + fun `init on API 25 and lower sets SSLSocketFactory with bundled Sentry root CAs`() { + fixture.initSut(configureOptions = { dsn = SAAS_DSN }) + + assertIs(fixture.sentryOptions.sslSocketFactory) + } + + @Config(sdk = [Build.VERSION_CODES.N_MR1]) + @Test + fun `init on API 25 and lower does not set SSLSocketFactory for self-hosted DSN`() { + fixture.initSut(configureOptions = { dsn = "https://key@sentry.example.com/1" }) + + assertNull(fixture.sentryOptions.sslSocketFactory) + } + + @Config(sdk = [Build.VERSION_CODES.O]) + @Test + fun `init on API 26 and higher does not set SSLSocketFactory`() { + fixture.initSut(configureOptions = { dsn = SAAS_DSN }) + + assertNull(fixture.sentryOptions.sslSocketFactory) + } + + @Config(sdk = [Build.VERSION_CODES.N_MR1]) + @Test + fun `init on API 25 and lower keeps user provided SSLSocketFactory`() { + val sslSocketFactory = mock() + fixture.initSut( + configureOptions = { + dsn = SAAS_DSN + this.sslSocketFactory = sslSocketFactory + } + ) + + assertEquals(sslSocketFactory, fixture.sentryOptions.sslSocketFactory) + } + @Test fun `init should set Android continuous profiler`() { fixture.initSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryRootCaSslSocketFactoryTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryRootCaSslSocketFactoryTest.kt new file mode 100644 index 00000000000..e55eb5508fc --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryRootCaSslSocketFactoryTest.kt @@ -0,0 +1,191 @@ +package io.sentry.android.core + +import com.google.common.truth.Truth.assertThat +import io.sentry.ILogger +import io.sentry.SentryLevel +import java.security.KeyStore +import java.security.MessageDigest +import java.security.cert.CertPathValidator +import java.security.cert.CertPathValidatorException +import java.security.cert.Certificate +import java.security.cert.CertificateFactory +import java.security.cert.PKIXParameters +import java.security.cert.X509Certificate +import java.util.Date +import java.util.concurrent.atomic.AtomicInteger +import javax.net.ssl.HttpsURLConnection +import javax.net.ssl.TrustManagerFactory +import javax.net.ssl.X509TrustManager +import kotlin.test.Test +import kotlin.test.assertFailsWith +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify + +class SentryRootCaSslSocketFactoryTest { + + @Test + fun `bundles ISRG Root X1 matching the fingerprint published on docs_sentry_io`() { + val keyStore = SentryRootCaSslSocketFactory.createTrustStore(emptyArray()) + + val fingerprints = + keyStore.aliases().toList().map { alias -> keyStore.getCertificate(alias).sha256() } + + assertThat(fingerprints) + .containsExactly( + // ISRG Root X1 + ISRG_ROOT_X1_SHA256 + ) + } + + @Test + fun `trust store keeps the system trusted CAs and adds the bundled root`() { + // The JVM's cacerts already has the ISRG roots, so remove them to act like an old Android + // device. + val systemIssuers = systemIssuersWithoutIsrgRoots() + assertThat(systemIssuers).isNotEmpty() + + val trustManager = + defaultTrustManager(SentryRootCaSslSocketFactory.createTrustStore(systemIssuers)) + + val accepted = trustManager.acceptedIssuers.map { it.sha256() } + assertThat(accepted).containsAtLeastElementsIn(systemIssuers.map { it.sha256() }) + assertThat(accepted).contains(ISRG_ROOT_X1_SHA256) + } + + @Test + fun `Let's Encrypt chain only validates with the bundled root`() { + val chain = letsEncryptChain() + + // Without any ISRG root, like Android 7.1.0 and older. + val ex = + assertFailsWith { + validate(chain, plainKeyStore(systemIssuersWithoutIsrgRoots())) + } + assertThat(ex.message).contains("trust anchor") + + // The same system CAs plus the bundled root. + validate(chain, SentryRootCaSslSocketFactory.createTrustStore(systemIssuersWithoutIsrgRoots())) + // Only the bundled root. + validate(chain, SentryRootCaSslSocketFactory.createTrustStore(emptyArray())) + } + + @Test + fun `falls back to the default SSLSocketFactory once when building the delegate throws a RuntimeException`() { + val logger = mock() + val calls = AtomicInteger() + val sut = + SentryRootCaSslSocketFactory(logger) { + calls.incrementAndGet() + throw IllegalStateException("broken OEM trust store") + } + + assertThat(sut.delegate).isSameInstanceAs(HttpsURLConnection.getDefaultSSLSocketFactory()) + assertThat(sut.supportedCipherSuites).isNotEmpty() + assertThat(sut.delegate).isSameInstanceAs(HttpsURLConnection.getDefaultSSLSocketFactory()) + + assertThat(calls.get()).isEqualTo(1) + verify(logger).log(eq(SentryLevel.ERROR), any(), any()) + } + + @Test + fun `creates a custom delegate SSLSocketFactory lazily and only once`() { + val sut = SentryRootCaSslSocketFactory(mock()) + + val delegate = sut.delegate + + assertThat(delegate).isNotSameInstanceAs(HttpsURLConnection.getDefaultSSLSocketFactory()) + assertThat(sut.supportedCipherSuites).isNotEmpty() + assertThat(sut.delegate).isSameInstanceAs(delegate) + } + + @Test + fun `isSentrySaasDsn matches sentry_io and its subdomains only`() { + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("https://key@o1.ingest.us.sentry.io/1")) + .isTrue() + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("https://key@o1.ingest.sentry.io/1")) + .isTrue() + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("https://key:secret@sentry.io:443/1")) + .isTrue() + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("https://key@O1.Ingest.SENTRY.IO./1")) + .isTrue() + + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn(null)).isFalse() + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("")).isFalse() + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("https://key@sentry.example.com/1")) + .isFalse() + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("https://key@notsentry.io/1")).isFalse() + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("https://key@sentry.io.evil.com/1")) + .isFalse() + assertThat(SentryRootCaSslSocketFactory.isSentrySaasDsn("https://key@evil.com/sentry.io")) + .isFalse() + } + + /** The JVM's system CAs without ISRG Root X1 and any other ISRG root, such as Root YR. */ + private fun systemIssuersWithoutIsrgRoots(): Array = + defaultTrustManager(null) + .acceptedIssuers + .filter { + val subject = it.subjectX500Principal.name + it.sha256() != ISRG_ROOT_X1_SHA256 && + !subject.contains("O=ISRG") && + !subject.contains("O=Internet Security Research Group") + } + .toTypedArray() + + private fun plainKeyStore(certificates: Array): KeyStore = + KeyStore.getInstance(KeyStore.getDefaultType()).apply { + load(null, null) + certificates.forEachIndexed { i, certificate -> setCertificateEntry("ca-$i", certificate) } + } + + /** + * A real chain served by valid-isrgrootx1.letsencrypt.org: leaf, YR1 intermediate and Root YR + * cross-signed by ISRG Root X1. + */ + private fun letsEncryptChain(): List { + val factory = CertificateFactory.getInstance("X.509") + return listOf( + "letsencrypt-leaf.pem", + "letsencrypt-yr1.pem", + "letsencrypt-root-yr-cross-signed-by-isrg-root-x1.pem", + ) + .map { name -> + javaClass.classLoader!!.getResourceAsStream("root-ca/$name").use { + factory.generateCertificate(it) as X509Certificate + } + } + } + + /** + * Validates the chain with PKIX. Validates at a fixed date within the leaf's validity, so the + * test doesn't break when the leaf certificate expires. + */ + private fun validate(chain: List, trustStore: KeyStore) { + val params = + PKIXParameters(trustStore).apply { + isRevocationEnabled = false + date = Date(VALIDATION_DATE_MILLIS) + } + CertPathValidator.getInstance("PKIX") + .validate(CertificateFactory.getInstance("X.509").generateCertPath(chain), params) + } + + private fun defaultTrustManager(keyStore: KeyStore?): X509TrustManager { + val factory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()) + factory.init(keyStore) + return factory.trustManagers.filterIsInstance().first() + } + + private fun Certificate.sha256(): String = + MessageDigest.getInstance("SHA-256").digest(encoded).joinToString(":") { "%02X".format(it) } + + companion object { + private const val ISRG_ROOT_X1_SHA256 = + "96:BC:EC:06:26:49:76:F3:74:60:77:9A:CF:28:C5:A7:CF:E8:A3:C0:AA:E1:1A:8F:FC:EE:05:C0:BD:DF:08:C6" + + // 2026-10-14T00:00:00Z, within the validity of all certificates in the Let's Encrypt chain. + private const val VALIDATION_DATE_MILLIS = 1791936000000L + } +} diff --git a/sentry-android-core/src/test/resources/root-ca/letsencrypt-leaf.pem b/sentry-android-core/src/test/resources/root-ca/letsencrypt-leaf.pem new file mode 100644 index 00000000000..f8fff5c8970 --- /dev/null +++ b/sentry-android-core/src/test/resources/root-ca/letsencrypt-leaf.pem @@ -0,0 +1,30 @@ +-----BEGIN CERTIFICATE----- +MIIFGzCCBAOgAwIBAgISBq7WJGYYt4Q8Ccz1ozUhHgUtMA0GCSqGSIb3DQEBCwUA +MDMxCzAJBgNVBAYTAlVTMRYwFAYDVQQKEw1MZXQncyBFbmNyeXB0MQwwCgYDVQQD +EwNZUjEwHhcNMjYxMDA3MTczODM3WhcNMjcwMTA1MTczODM2WjArMSkwJwYDVQQD +EyB2YWxpZC1pc3Jncm9vdHgxLmxldHNlbmNyeXB0Lm9yZzCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAJegq3LAK4w4Je/obWixFZkCkxAJT1r1bfIQhyPj +Wn805cO7xuZ5RMfhGMZbKMlAHo+qzYZ2U1vrOFvnVWX7F3YpHP3KlIpFfRPLvOZp +Sliqs/PPtB2AUjQuiJ4HSySdtQfEoLwbiIqaxOJ39uofDlsOYDtFbN6IbQO8ThDQ +0d7pfUbRJ65+p7evlkHqCOSrOl8Z5ZGq3OdAIqapU5azR1TzWHDXiB8a4TPkat+L +U7jzG6QkEOy9I2XRQG446F2uTQNELUDkI+hihBu8zrHGYJqVegRbVM0CqeD7qMlB +GGGJj51HRbQ9+GjC7FmKO8Mrgko02vQKf1XqREANIc/HMIUCAwEAAaOCAi8wggIr +MA4GA1UdDwEB/wQEAwIFoDATBgNVHSUEDDAKBggrBgEFBQcDATAMBgNVHRMBAf8E +AjAAMB0GA1UdDgQWBBSshwnlgVMBW7xOwxUy7sA16PafjDAfBgNVHSMEGDAWgBQf +LzW+RhSCzUCxrnksVXj699Ro+zAzBggrBgEFBQcBAQQnMCUwIwYIKwYBBQUHMAKG +F2h0dHA6Ly95cjEuaS5sZW5jci5vcmcvMCsGA1UdEQQkMCKCIHZhbGlkLWlzcmdy +b290eDEubGV0c2VuY3J5cHQub3JnMBMGA1UdIAQMMAowCAYGZ4EMAQIBMC4GA1Ud +HwQnMCUwI6AhoB+GHWh0dHA6Ly95cjEuYy5sZW5jci5vcmcvNDYuY3JsMIIBDQYK +KwYBBAHWeQIEAgSB/gSB+wD5AHYARqI5Z8YNtkaHxm89+ZmUdpOmphEghFfVVefj +0KHZtkYAAAGhF6euqgAABAMARzBFAiBaxfd0LR2834Tq6CCmSa/g4ntvW2WPLxcT +y18xxUdJigIhANdm6yaOkq57UZrO5l4idTDkTTp/0D09lcADS7ponaLvAH8AjspH +C6zeavOiBrCkeoS3Rv4fxr+VPiXmm07kAkjzxugAAAGhF6evlQAIAAAFAB6q81IE +AwBIMEYCIQDIFPqkL/e1jgvJ1D/Gzn+NFfI3o3a7WHaUu/+c/h+ujgIhAIlx829E +YwDmgN7YNWDuUEeuqcoK/MlleU4K8Y/uYbfwMA0GCSqGSIb3DQEBCwUAA4IBAQBu +CYe3oorT9pbKsO/4Dy4nDxfVRM0r+eAbJcdIeTAZR4e6bIqdi28ynejflcdHTBip +hUkX1LUC2tTVpRMpo4CL+cLOUWNiDpy197ESv8suW0yF/P0/BnJWuyhaN6zvE7yi +kC9osdCretyxVBu17gZfYI6+RvJaUcmoOJDf6GZYx8ZNezsvo37K8toiOZcK+5Zd +jtjf5HvJWrtoptkNXpIMEgG9DNVGUzk/bjmDWOIYPeXNkMAkRczslg0iVqAjkZmV +N24GQ6xv5SRq8zTUDCld4d91izVOeLoq+p2cOprtCGvsrEa5LDzi6R0z/od7v4c2 +py2xTDhZOI36OVxyuzO4 +-----END CERTIFICATE----- diff --git a/sentry-android-core/src/test/resources/root-ca/letsencrypt-root-yr-cross-signed-by-isrg-root-x1.pem b/sentry-android-core/src/test/resources/root-ca/letsencrypt-root-yr-cross-signed-by-isrg-root-x1.pem new file mode 100644 index 00000000000..dd14377992b --- /dev/null +++ b/sentry-android-core/src/test/resources/root-ca/letsencrypt-root-yr-cross-signed-by-isrg-root-x1.pem @@ -0,0 +1,34 @@ +-----BEGIN CERTIFICATE----- +MIIF9DCCA9ygAwIBAgIRAPJLbRf52a18scn+p4eCaZ8wDQYJKoZIhvcNAQELBQAw +TzELMAkGA1UEBhMCVVMxKTAnBgNVBAoTIEludGVybmV0IFNlY3VyaXR5IFJlc2Vh +cmNoIEdyb3VwMRUwEwYDVQQDEwxJU1JHIFJvb3QgWDEwHhcNMjYwNTEzMDAwMDAw +WhcNMzIwOTAyMjM1OTU5WjAuMQswCQYDVQQGEwJVUzENMAsGA1UEChMESVNSRzEQ +MA4GA1UEAxMHUm9vdCBZUjCCAiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIB +ANvGJnN78CTJdWL3+eGfsLN5TrNBJs+VH9hRXqRbwxu9sGNiB0BD1fcOxbSUQCJI +M1xE13Db+5Cw1w0s0EBYsvuIP/6joF0w8cuImbgR1OGgYbSQ4OpzI+DG8SGuTlcE +873OCS+kh3srlo6vl43M5OJg4Aeo1sfHp6kTJDoIiFBNJAY+OKfX/FUvYKuhjT+n +o49lmqmupSBI5PkBQiqrEGtWU5uxU/cQWHGu8jSjFBznZqvbNPLMXMLFxCb3WTfr +JBXXjqvWG+v4bjzxjjeAtOlU7qarRDvNOyAuQYLln904M+faKx8hnLCpJ15ZqaEg +cNlY+9MMWcC5yvL2A2j3l9+2buggZX+dOE91zYmIdawTvSZuVvlbRrAlLxIB6pwM +BjneXCjYQ8+3BCCjssbSNpZU3hTcBDdhfAlEDlYr6pEatnMdmDT5BqnKC92bd0Eh +M1fbLHioLccLCuievT8ZkPhZrq7Mii7gNXAcUEAR8+lzYal+9zTg7C5DALyVOeG/ +CqfRAMn1KSHCR0NSA6P8tn/mGRlnCct5rtVCLnVySVpU6H1qGg3DgTOuskf8eahT +MiYbI5ezPJmO5ertalskQ1utp74+eDy92PI4ftHKTbq9IWhH4YZKh3WnJEIt+oQv +lYZbY8tpEroKrFB6PFGzrJIDRyts4HqvuH52RFj2zv/BAgMBAAGjgeswgegwDgYD +VR0PAQH/BAQDAgEGMBMGA1UdJQQMMAoGCCsGAQUFBwMBMA8GA1UdEwEB/wQFMAMB +Af8wHQYDVR0OBBYEFN7nW2DQIm1AKH0/DQH+pLVStFGUMB8GA1UdIwQYMBaAFHm0 +WeZ7tuXkAXOACIjIGlj26ZtuMDIGCCsGAQUFBwEBBCYwJDAiBggrBgEFBQcwAoYW +aHR0cDovL3gxLmkubGVuY3Iub3JnLzATBgNVHSAEDDAKMAgGBmeBDAECATAnBgNV +HR8EIDAeMBygGqAYhhZodHRwOi8veDEuYy5sZW5jci5vcmcvMA0GCSqGSIb3DQEB +CwUAA4ICAQA8spSI95KKfn2W6GMmDpHBJSPaLbsS3W93cijJCRCYAc1fsJgL1FIL +7C0C9ecPOdcwB2fi0Dk2p94j9iTJCxmt5CFSKLRWwnXT2MMSXexVxqoVB79BdWPx +VXETkVme/qYSAuKVHh5Ps+5BixgmwS1JkjSAc+MfrUbNssVEEnH0aEiAh+rotXAV +JSP/Ye7LJPEwD9DWG72vVWbhAcuOf5OLjz57Ctk7MgQHynZ7+PlHJtajroCaIbtC +r6tcZZaAwUQm+jQyeWdV+2hv9deOYFmKeQyjjcSrN5Nadrw+L9DZJLbA1HqeNvLh +BgqpP0fvJq2N6EtD574N6eMI7uMsJTnji2UDz9el5XLSv9fqJMuDQtYVb2oTNoKp +oUqhxPVC0aq4eG5MESaIdn8b5ZGSSeAJLMHXljEdlNza+ncfkviXk1POLnnFdvx8 +/gk6M374WbLWFXw8N141B/Rl/tINGfl1TxOIiqtiMYkL02RSGb1kq34BL9NPP27z +RGMuHGnzS3hFIrRTfKxrzUZ9RzQWzEG3K6fJ3r2nqSltkeytis9DIBoFY9VmVyjL +M71DMi+y1+TRSJVClEMwvA4yL++7q9XZx5r5wBRWB4kQTKH5qyoZnDw7iiuh1lID +yDFx8r7i9vIJU5HS3moZLkYWAOilMaV9N56A9Bgb6dNcHkvg3NoaYA== +-----END CERTIFICATE----- diff --git a/sentry-android-core/src/test/resources/root-ca/letsencrypt-yr1.pem b/sentry-android-core/src/test/resources/root-ca/letsencrypt-yr1.pem new file mode 100644 index 00000000000..eb3040fb203 --- /dev/null +++ b/sentry-android-core/src/test/resources/root-ca/letsencrypt-yr1.pem @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIIE2zCCAsOgAwIBAgIRAKICU/FfJpHAXcHOE7m8yk4wDQYJKoZIhvcNAQELBQAw +LjELMAkGA1UEBhMCVVMxDTALBgNVBAoTBElTUkcxEDAOBgNVBAMTB1Jvb3QgWVIw +HhcNMjUwOTAzMDAwMDAwWhcNMjgwOTAyMjM1OTU5WjAzMQswCQYDVQQGEwJVUzEW +MBQGA1UEChMNTGV0J3MgRW5jcnlwdDEMMAoGA1UEAxMDWVIxMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEAoVi8X2xCYgMXvJxNPKp/oF13UMgmPABB07VC +LNDtoXmt9luEZNJSBV10VyT1Pz6LD8Zq1d2gc43WNl1AdRrj4sEnazbOiz0nPpmG +Bp2hui49oZtDIY6wdKeZAi5BbNU20CH6RSBBMLSQ9cXrH8dxdv4PAJ45ssGML68U +SE3BsjC2a6cAN9L5CgXVIQi5tfNiTPoFZZ3S0OlXqLmmtdV95udWAb5b6e/F49Di +CsH0Y00Ag72BVIb1hzynmKe+X0mERBTtsb3BwmpV9ipeBjMLoR/D9cHxHQCWoi5l +TmXwY015J5rGelz1nZjJuxc2kioaX29XJBnhMkP531rSdG5uMwIDAQABo4HuMIHr +MA4GA1UdDwEB/wQEAwIBhjATBgNVHSUEDDAKBggrBgEFBQcDATASBgNVHRMBAf8E +CDAGAQH/AgEAMB0GA1UdDgQWBBQfLzW+RhSCzUCxrnksVXj699Ro+zAfBgNVHSME +GDAWgBTe51tg0CJtQCh9Pw0B/qS1UrRRlDAyBggrBgEFBQcBAQQmMCQwIgYIKwYB +BQUHMAKGFmh0dHA6Ly95ci5pLmxlbmNyLm9yZy8wEwYDVR0gBAwwCjAIBgZngQwB +AgEwJwYDVR0fBCAwHjAcoBqgGIYWaHR0cDovL3lyLmMubGVuY3Iub3JnLzANBgkq +hkiG9w0BAQsFAAOCAgEA0+zvMq3kHig1ddTmmm+RibTr9/RpX7k4buanMMRqbV/y +IvP82zAHN3mvaw+cASuVsdpd0ikjhr4hnhJQLQOzOp2ccKrsdGOAgo0vddeISFAq +EWEV4lmUM3vFF796up+bSgmJ1u6RupDCMxDgF8M3eLvGuj6L0lu3zkQ0KuQLnKxL +tB0oQqn1Idg5CuuGpMvQzk29Pa3D/qHurc0EIM9SxukQuJqq63lxsYyRQFU8yMBO +hq1w5LbfaWNRrz1uklOfI/pYkAb2E2MTZrAMQkBIE2S8Jt1F8gRc96o/xOsrgvSk +a84AisX6xq1lz1Z7jGvrnXc4TMcjxZTjiTaihcYI1JIXZiLtEMSCa5l3cu8YWd6z +dLRQlqRdclVjuQfNHawRJ6GWlkK0QJosivTKwdBw3KxEtzGo8yMHERbsy57gP1UX +HOMcmZYQC0gtyR3SxfenIM/MxC3Ia2Ypab/kQ/CTnlIn2KQ5JUC6NYrGCbhFN9bp +5lKJStEwCUnLpntcrXk5XVDCNv/5RyWpRThkGOV7GetKkQ0qAY8hCzWK6oqnAhDZ +cjlYVdWfqOw3DIOX6EDNBgAqHarRVxyF9QZdOaXSyPJ0ueD2BYJEBgaCGQ8rAaU/ +Qc123V5LTXDZW4CcsPBDyhy4v+c8hClAyw/IkJlfBqxB9D+/wvIMHgECZ4ptP6o= +-----END CERTIFICATE----- diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index 4b9d1eb89c4..015e162afc7 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -228,6 +228,7 @@ public class ReplayIntegration( options, scopes, dateProvider, + options.monotonicTicker, replayExecutor, persistingExecutor, replayCacheProvider, @@ -237,6 +238,7 @@ public class ReplayIntegration( options, scopes, dateProvider, + options.monotonicTicker, replayExecutor, persistingExecutor, replayCacheProvider, diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt index 3c8af759cb0..b6b8981f8ce 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt @@ -35,24 +35,30 @@ import io.sentry.android.replay.gestures.ReplayGestureConverter import io.sentry.android.replay.util.ReplayRunnable import io.sentry.protocol.SentryId import io.sentry.rrweb.RRWebEvent +import io.sentry.time.Deadline +import io.sentry.time.MonotonicTicker import io.sentry.transport.ICurrentDateProvider import java.io.File import java.util.Date import java.util.Deque import java.util.concurrent.ConcurrentLinkedDeque import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit.MILLISECONDS import java.util.concurrent.atomic.AtomicBoolean -import java.util.concurrent.atomic.AtomicLong import java.util.concurrent.atomic.AtomicReference import kotlin.properties.ReadWriteProperty import kotlin.reflect.KProperty @SuppressLint("UseRequiresApi") @TargetApi(26) -internal abstract class BaseCaptureStrategy( +internal abstract class BaseCaptureStrategy +@Suppress("LongParameterList") +constructor( private val options: SentryOptions, private val scopes: IScopes?, - private val dateProvider: ICurrentDateProvider, + // TODO [v9]: We should consider replacing this with AnchoredClock in V9. + dateProvider: ICurrentDateProvider, + private val ticker: MonotonicTicker, protected val replayExecutor: ScheduledExecutorService, protected val persistingExecutor: ScheduledExecutorService, private val replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null, @@ -63,7 +69,7 @@ internal abstract class BaseCaptureStrategy( private const val MAX_CONTEXT_VALUES = 100 } - private val gestureConverter = ReplayGestureConverter(dateProvider) + private val gestureConverter = ReplayGestureConverter(dateProvider, ticker) protected val isTerminating = AtomicBoolean(false) protected var cache: ReplayCache? = null @@ -85,7 +91,11 @@ internal abstract class BaseCaptureStrategy( if (newValue == null) null else DateUtils.getTimestamp(newValue), ) } - protected val replayStartTimestamp = AtomicLong() + /** + * When the recording reaches its `sessionDuration` cap, or `null` while nothing is being + * recorded. Only [SessionCaptureStrategy] enforces a cap; buffer mode records indefinitely. + */ + @Volatile protected var replayDeadline: Deadline? = null private var recordingMetadata: RecordingMetadata? by persistableAtomicNullable(propertyName = "") { _, _, newValue -> persistMetadataValue(SEGMENT_KEY_RELEASE, newValue?.release) @@ -123,7 +133,7 @@ internal abstract class BaseCaptureStrategy( this.replayType = replayType ?: (if (this is SessionCaptureStrategy) SESSION else BUFFER) segmentTimestamp = DateUtils.getCurrentDateTime() - replayStartTimestamp.set(dateProvider.currentTimeMillis) + replayDeadline = Deadline.after(ticker, options.sessionReplay.sessionDuration, MILLISECONDS) // the same values MainEventProcessor applies to current-run segments recordingMetadata = RecordingMetadata( @@ -152,7 +162,7 @@ internal abstract class BaseCaptureStrategy( replayExecutor.submit( ReplayRunnable("$TAG.stop") { cache?.close() - replayStartTimestamp.set(0) + replayDeadline = null segmentTimestamp = null currentReplayId = SentryId.EMPTY_ID } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt index c8210546ba6..071acc19cae 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt @@ -20,6 +20,7 @@ import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.util.ReplayRunnable import io.sentry.clientreport.DiscardReason.RATELIMIT_BACKOFF import io.sentry.protocol.SentryId +import io.sentry.time.MonotonicTicker import io.sentry.transport.ICurrentDateProvider import java.io.File import java.util.Date @@ -39,10 +40,13 @@ import java.util.concurrent.ScheduledExecutorService */ @SuppressLint("UseRequiresApi") @TargetApi(26) -internal class BufferCaptureStrategy( +internal class BufferCaptureStrategy +@Suppress("LongParameterList") +constructor( private val options: SentryOptions, private val scopes: IScopes?, private val dateProvider: ICurrentDateProvider, + private val ticker: MonotonicTicker, executor: ScheduledExecutorService, persistingExecutor: ScheduledExecutorService, replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null, @@ -51,6 +55,7 @@ internal class BufferCaptureStrategy( options, scopes, dateProvider, + ticker, executor, persistingExecutor, replayCacheProvider = replayCacheProvider, @@ -172,7 +177,14 @@ internal class BufferCaptureStrategy( // we hand over replayExecutor and persistingExecutor to the new strategy to preserve order of // execution val captureStrategy = - SessionCaptureStrategy(options, scopes, dateProvider, replayExecutor, persistingExecutor) + SessionCaptureStrategy( + options, + scopes, + dateProvider, + ticker, + replayExecutor, + persistingExecutor, + ) captureStrategy.recorderConfig = recorderConfig captureStrategy.start( segmentId = currentSegment, diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt index d30311549b5..8e6ae9fc4a4 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt @@ -11,6 +11,7 @@ import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.util.ReplayRunnable import io.sentry.protocol.SentryId +import io.sentry.time.MonotonicTicker import io.sentry.transport.ICurrentDateProvider import java.util.Date import java.util.concurrent.ScheduledExecutorService @@ -27,10 +28,13 @@ import java.util.concurrent.ScheduledExecutorService * * See [BufferCaptureStrategy] for the on-error counterpart. */ -internal class SessionCaptureStrategy( +internal class SessionCaptureStrategy +@Suppress("LongParameterList") +constructor( private val options: SentryOptions, private val scopes: IScopes?, private val dateProvider: ICurrentDateProvider, + ticker: MonotonicTicker, executor: ScheduledExecutorService, persistingExecutor: ScheduledExecutorService, replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null, @@ -39,6 +43,7 @@ internal class SessionCaptureStrategy( options, scopes, dateProvider, + ticker, executor, persistingExecutor, replayCacheProvider, @@ -110,6 +115,7 @@ internal class SessionCaptureStrategy( // reflecting the exact time of when it was captured val currentConfig = recorderConfig val frameTimestamp = dateProvider.currentTimeMillis + val deadlineExceeded = replayDeadline?.hasPassed() == true replayExecutor.submit( ReplayRunnable("$TAG.add_frame") { cache?.store(frameTimestamp) @@ -157,7 +163,7 @@ internal class SessionCaptureStrategy( } } - if (frameTimestamp - replayStartTimestamp.get() >= options.sessionReplay.sessionDuration) { + if (deadlineExceeded) { options.replayController.stop() options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording") } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt index cbedfc24cc2..c348178e9d2 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt @@ -7,18 +7,27 @@ import io.sentry.rrweb.RRWebInteractionEvent import io.sentry.rrweb.RRWebInteractionEvent.InteractionType import io.sentry.rrweb.RRWebInteractionMoveEvent import io.sentry.rrweb.RRWebInteractionMoveEvent.Position +import io.sentry.time.Deadline +import io.sentry.time.MonotonicTicker import io.sentry.transport.ICurrentDateProvider +import java.util.concurrent.TimeUnit.MILLISECONDS -internal class ReplayGestureConverter(private val dateProvider: ICurrentDateProvider) { +internal class ReplayGestureConverter( + // TODO [v9]: Replace this with AnchoredClock in V9 + private val dateProvider: ICurrentDateProvider, + private val ticker: MonotonicTicker, +) { internal companion object { // rrweb values - private const val TOUCH_MOVE_DEBOUNCE_THRESHOLD = 50 + private const val TOUCH_MOVE_DEBOUNCE_THRESHOLD = 50L private const val CAPTURE_MOVE_EVENT_THRESHOLD = 500 } private val currentPositions = LinkedHashMap>(10) private var touchMoveBaseline = 0L - private var lastCapturedMoveEvent = 0L + + /** When the next move event may be captured, or `null` before the first one. */ + private var moveDebounce: Deadline? = null fun convert( event: MotionEvent, @@ -28,12 +37,10 @@ internal class ReplayGestureConverter(private val dateProvider: ICurrentDateProv MotionEvent.ACTION_MOVE -> { // we only throttle move events as those can be overwhelming val now = dateProvider.currentTimeMillis - if ( - lastCapturedMoveEvent != 0L && lastCapturedMoveEvent + TOUCH_MOVE_DEBOUNCE_THRESHOLD > now - ) { + if (moveDebounce?.hasPassed() == false) { return null } - lastCapturedMoveEvent = now + moveDebounce = Deadline.after(ticker, TOUCH_MOVE_DEBOUNCE_THRESHOLD, MILLISECONDS) currentPositions.keys.forEach { pId -> val pIndex = event.findPointerIndex(pId) diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt index 14d500cb615..4f4f7ff3c96 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt @@ -1377,6 +1377,7 @@ class ReplayIntegrationTest { ICurrentDateProvider { System.currentTimeMillis() + fixture.options.sessionReplay.sessionSegmentDuration }, + fixture.options.monotonicTicker, // run tasks synchronously in tests mock { whenever(mock.submit(any())).doAnswer { @@ -1761,6 +1762,7 @@ class ReplayIntegrationTest { options, null, CurrentDateProvider.getInstance(), + options.monotonicTicker, executor = mock { whenever(mock.submit(any())).doAnswer { diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt index aae35a7d957..6fdf3ab210d 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt @@ -23,6 +23,7 @@ import io.sentry.android.replay.capture.BufferCaptureStrategyTest.Fixture.Compan import io.sentry.clientreport.DiscardReason import io.sentry.clientreport.DiscardedEvent import io.sentry.protocol.SentryId +import io.sentry.time.TestMonotonicTicker import io.sentry.transport.CurrentDateProvider import io.sentry.transport.ICurrentDateProvider import io.sentry.transport.RateLimiter @@ -108,6 +109,8 @@ class BufferCaptureStrategyTest { ?.discardedEvents .orEmpty() + val ticker = TestMonotonicTicker() + fun getSut( dateProvider: ICurrentDateProvider = CurrentDateProvider.getInstance(), replayCacheDir: File? = null, @@ -117,6 +120,7 @@ class BufferCaptureStrategyTest { options, scopes, dateProvider, + ticker, mock { whenever(it.submit(any())).doAnswer { invocation -> (invocation.arguments[0] as Runnable).run() diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt index d2e75507d3e..b0dbe3f82bb 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt @@ -36,11 +36,14 @@ import io.sentry.protocol.SentryId import io.sentry.rrweb.RRWebBreadcrumbEvent import io.sentry.rrweb.RRWebMetaEvent import io.sentry.rrweb.RRWebOptionsEvent +import io.sentry.time.TestMonotonicTicker import io.sentry.transport.CurrentDateProvider import io.sentry.transport.ICurrentDateProvider import java.io.File import java.util.Date import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit.MILLISECONDS +import java.util.concurrent.TimeUnit.SECONDS import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals @@ -62,6 +65,7 @@ import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +@Suppress("LargeClass") class SessionCaptureStrategyTest { @get:Rule val tmpDir = TemporaryFolder() @@ -114,6 +118,8 @@ class SessionCaptureStrategyTest { bitRate = 20_000, ) + val ticker = TestMonotonicTicker() + fun getSut( dateProvider: ICurrentDateProvider = CurrentDateProvider.getInstance(), replayCacheDir: File? = null, @@ -131,6 +137,7 @@ class SessionCaptureStrategyTest { options, scopes, dateProvider, + ticker, replayExecutor, mock { doAnswer { invocation -> @@ -441,6 +448,7 @@ class SessionCaptureStrategyTest { strategy.onScreenshotRecorded(mock()) {} now += fixture.options.sessionReplay.sessionDuration + fixture.ticker.advance(fixture.options.sessionReplay.sessionDuration, MILLISECONDS) tasks.forEach(Runnable::run) verify(fixture.scopes, never()).captureReplay(any(), any()) @@ -498,6 +506,37 @@ class SessionCaptureStrategyTest { strategy.start() strategy.onConfigurationChanged(mock()) now += fixture.options.sessionReplay.sessionDuration * 2 + fixture.ticker.advance(fixture.options.sessionReplay.sessionDuration * 2, MILLISECONDS) + + strategy.onScreenshotRecorded(mock()) {} + + verify(fixture.options.replayController).stop() + } + + @Test + fun `onScreenshotRecorded does not stop replay when the wall clock jumps past the deadline`() { + var now = System.currentTimeMillis() + val strategy = fixture.getSut(dateProvider = { now }) + strategy.start() + strategy.onConfigurationChanged(mock()) + // the device corrects its clock two hours forwards a second into the recording + now += fixture.options.sessionReplay.sessionDuration * 2 + fixture.ticker.advance(1, SECONDS) + + strategy.onScreenshotRecorded(mock()) {} + + verify(fixture.options.replayController, never()).stop() + } + + @Test + fun `onScreenshotRecorded stops replay when the wall clock steps backwards past the deadline`() { + var now = System.currentTimeMillis() + val strategy = fixture.getSut(dateProvider = { now }) + strategy.start() + strategy.onConfigurationChanged(mock()) + // an hour of recording, during which the device drags its clock back to before the start + fixture.ticker.advance(fixture.options.sessionReplay.sessionDuration, MILLISECONDS) + now -= 1000 strategy.onScreenshotRecorded(mock()) {} diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/ReplayGestureConverterTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/ReplayGestureConverterTest.kt index d72fd4c88d6..a396a62f93a 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/ReplayGestureConverterTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/ReplayGestureConverterTest.kt @@ -5,7 +5,9 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.rrweb.RRWebInteractionEvent import io.sentry.rrweb.RRWebInteractionMoveEvent +import io.sentry.time.TestMonotonicTicker import io.sentry.transport.ICurrentDateProvider +import java.util.concurrent.TimeUnit.MILLISECONDS import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -19,10 +21,17 @@ import org.robolectric.annotation.Config class ReplayGestureConverterTest { internal class Fixture { var now: Long = 1000L + val ticker = TestMonotonicTicker() + + /** Time passing normally: the wall clock and the ticker move together. */ + fun advance(millis: Long) { + now += millis + ticker.advance(millis, MILLISECONDS) + } fun getSut( dateProvider: ICurrentDateProvider = ICurrentDateProvider { now } - ): ReplayGestureConverter = ReplayGestureConverter(dateProvider) + ): ReplayGestureConverter = ReplayGestureConverter(dateProvider, ticker) } private val fixture = Fixture() @@ -61,13 +70,46 @@ class ReplayGestureConverterTest { assertNotNull(result) // Second call within debounce threshold should be null - fixture.now += 40 // Increase time by 40ms + fixture.advance(40) result = sut.convert(event, recorderConfig) assertNull(result) event.recycle() } + @Test + fun `wall clock stepping backwards does not suppress move events`() { + val sut = fixture.getSut() + val recorderConfig = ScreenshotRecorderConfig(scaleFactorX = 1f, scaleFactorY = 1f) + val event = MotionEvent.obtain(0, 0, MotionEvent.ACTION_MOVE, 100f, 200f, 0) + + assertNotNull(sut.convert(event, recorderConfig)) + + // an NTP correction drags the wall clock backwards while real time moves on past the threshold + fixture.now -= 400 + fixture.ticker.advance(60, MILLISECONDS) + + assertNotNull(sut.convert(event, recorderConfig)) + + event.recycle() + } + + @Test + fun `wall clock stepping forwards does not lift the debounce early`() { + val sut = fixture.getSut() + val recorderConfig = ScreenshotRecorderConfig(scaleFactorX = 1f, scaleFactorY = 1f) + val event = MotionEvent.obtain(0, 0, MotionEvent.ACTION_MOVE, 100f, 200f, 0) + + assertNotNull(sut.convert(event, recorderConfig)) + + fixture.now += 60_000 + fixture.ticker.advance(10, MILLISECONDS) + + assertNull(sut.convert(event, recorderConfig)) + + event.recycle() + } + @Test fun `convert ACTION_MOVE event with capture threshold`() { val sut = fixture.getSut() @@ -83,7 +125,7 @@ class ReplayGestureConverterTest { assertNull(result) // Second call should trigger capture - fixture.now += 600 // Increase time by 600ms + fixture.advance(600) result = sut.convert(moveEvent, recorderConfig) assertNotNull(result) with(result[0] as RRWebInteractionMoveEvent) { @@ -217,7 +259,7 @@ class ReplayGestureConverterTest { 0, 0, ) - fixture.now += 100 // Increase time by 100ms + fixture.advance(100) result = sut.convert(event, recorderConfig) assertNotNull(result) assertTrue(result[0] is RRWebInteractionEvent) @@ -254,7 +296,7 @@ class ReplayGestureConverterTest { result = sut.convert(event, recorderConfig) assertNull(result) - fixture.now += 600 // Increase time by 600ms to trigger move capture + fixture.advance(600) result = sut.convert(event, recorderConfig) assertNotNull(result) assertTrue((result[0] as RRWebInteractionMoveEvent).positions!!.size == 2) @@ -278,7 +320,7 @@ class ReplayGestureConverterTest { 0, 0, ) - fixture.now += 100 // Increase time by 100ms + fixture.advance(100) result = sut.convert(event, recorderConfig) assertNotNull(result) assertTrue(result[0] is RRWebInteractionEvent) @@ -291,7 +333,7 @@ class ReplayGestureConverterTest { // Simulate first finger up event = MotionEvent.obtain(0, 4, MotionEvent.ACTION_UP, 90f, 90f, 0) - fixture.now += 100 // Increase time by 100ms + fixture.advance(100) result = sut.convert(event, recorderConfig) assertNotNull(result) assertTrue(result[0] is RRWebInteractionEvent) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 813a4691e83..abf8fc28526 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -508,6 +508,7 @@ public final class io/sentry/DiagnosticLogger : io/sentry/ILogger { public final class io/sentry/DsnUtil { public fun ()V + public static fun getHost (Ljava/lang/String;)Ljava/lang/String; public static fun urlContainsDsnHost (Lio/sentry/SentryOptions;Ljava/lang/String;)Z } diff --git a/sentry/src/main/java/io/sentry/Dsn.java b/sentry/src/main/java/io/sentry/Dsn.java index 15aea2a064c..1e4b49aed82 100644 --- a/sentry/src/main/java/io/sentry/Dsn.java +++ b/sentry/src/main/java/io/sentry/Dsn.java @@ -87,7 +87,7 @@ URI getSentryUri() { final String hostPort = hostAndPath.substring(0, firstSlash); final int portColon = portSeparatorIndex(hostPort); - final String host = portColon < 0 ? hostPort : hostPort.substring(0, portColon); + final String host = hostOf(hostPort, portColon); final int port = portColon < 0 ? -1 : parsePort(hostPort.substring(portColon + 1)); final String rawPath = stripTrailingSlash(collapseSlashes(hostAndPath.substring(firstSlash))); @@ -105,6 +105,38 @@ URI getSentryUri() { } } + /** + * Returns the host of the given DSN, parsed the same way as the constructor does, or {@code null} + * if no host can be found. Only the parts needed to find the host are checked, so a non-null + * result doesn't mean the DSN is valid. Doesn't create a {@link URI}, so it's cheap to call. + */ + static @Nullable String parseHost(final @Nullable String dsn) { + if (dsn == null) { + return null; + } + final String dsnString = dsn.trim(); + final int schemeEnd = dsnString.indexOf("://"); + if (schemeEnd < 0) { + return null; + } + final int atIndex = dsnString.indexOf('@', schemeEnd + 3); + if (atIndex < 0) { + return null; + } + final String hostAndPath = stripQueryAndFragment(dsnString, atIndex + 1); + final int firstSlash = hostAndPath.indexOf('/'); + if (firstSlash < 0) { + return null; + } + final String hostPort = hostAndPath.substring(0, firstSlash); + final String host = hostOf(hostPort, portSeparatorIndex(hostPort)); + return host.isEmpty() ? null : host; + } + + private static @NotNull String hostOf(final @NotNull String hostPort, final int portColon) { + return portColon < 0 ? hostPort : hostPort.substring(0, portColon); + } + private static int parsePort(final @NotNull String portString) { try { return Integer.parseInt(portString); diff --git a/sentry/src/main/java/io/sentry/DsnUtil.java b/sentry/src/main/java/io/sentry/DsnUtil.java index f31d1c286af..28f12d8aaa1 100644 --- a/sentry/src/main/java/io/sentry/DsnUtil.java +++ b/sentry/src/main/java/io/sentry/DsnUtil.java @@ -9,6 +9,15 @@ @ApiStatus.Internal public final class DsnUtil { + /** + * Returns the host of the given DSN, parsed the same way as the DSN the SDK sends envelopes to, + * or {@code null} if no host can be found. Unlike {@link SentryOptions#retrieveParsedDsn()}, this + * doesn't create a {@link URI}, so it's cheap enough to call on the main thread. + */ + public static @Nullable String getHost(final @Nullable String dsn) { + return Dsn.parseHost(dsn); + } + public static boolean urlContainsDsnHost(@Nullable SentryOptions options, @Nullable String url) { if (options == null) { return false; diff --git a/sentry/src/main/java/io/sentry/HostnameCache.java b/sentry/src/main/java/io/sentry/HostnameCache.java index b4a19124b81..dd1a2157034 100644 --- a/sentry/src/main/java/io/sentry/HostnameCache.java +++ b/sentry/src/main/java/io/sentry/HostnameCache.java @@ -1,5 +1,7 @@ package io.sentry; +import io.sentry.time.Deadline; +import io.sentry.time.MonotonicTicker; import io.sentry.util.Objects; import java.net.InetAddress; import java.util.concurrent.Callable; @@ -16,14 +18,15 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; /** * Time sensitive cache in charge of keeping track of the hostname. The {@code * InetAddress.getLocalHost().getCanonicalHostName()} call can be quite expensive and could be * called for the creation of each {@link SentryEvent}. This system will prevent unnecessary costs - * by keeping track of the hostname for a period defined during the construction. For performance - * purposes, the operation of retrieving the hostname will automatically fail after a period of time - * defined by {@link #GET_HOSTNAME_TIMEOUT} without result. + * by keeping track of the hostname for a period defined by {@link #HOSTNAME_CACHE_DURATION}. For + * performance purposes, the operation of retrieving the hostname will automatically fail after a + * period of time defined by {@link #GET_HOSTNAME_TIMEOUT} without result. * *

One instance is held per {@link SentryOptions} and should be obtained through {@link * SentryOptions#getHostnameCache()}. @@ -38,14 +41,13 @@ public final class HostnameCache { /** How long the worker thread may stay idle before it self-terminates. */ private static final long THREAD_KEEP_ALIVE_SECONDS = 30; - /** Time for which the cache is kept. */ - private final long cacheDuration; + private final @NotNull MonotonicTicker ticker; /** Current value for hostname (might change over time). */ @Nullable private volatile String hostname; - /** Time at which the cache should expire. */ - private volatile long expirationTimestamp; + /** When the cached hostname goes stale. */ + private volatile @NotNull Deadline cacheFreshUntil; /** Whether a cache update thread is currently running or not. */ private final @NotNull AtomicBoolean updateRunning = new AtomicBoolean(false); @@ -59,27 +61,30 @@ public final class HostnameCache { * SentryOptions#getHostnameCache()}. Every instance resolves the hostname as soon as it is built * and caches it on its own, so another instance would repeat that blocking lookup instead of * sharing the cached value. + * + * @param ticker the ticker the cache lifetime is measured on */ - HostnameCache() { - this(HOSTNAME_CACHE_DURATION); - } - - HostnameCache(long cacheDuration) { + HostnameCache(final @NotNull MonotonicTicker ticker) { // avoid method refs on Android due to some issues with older AGP setups // noinspection Convert2MethodRef - this(cacheDuration, () -> InetAddress.getLocalHost()); + this(() -> InetAddress.getLocalHost(), ticker); } /** * Sets up a cache for the hostname. * - * @param cacheDuration cache duration in milliseconds. * @param getLocalhost a callback to obtain the localhost address - this is mostly here because of * testability + * @param ticker the ticker the cache lifetime is measured on */ - HostnameCache(long cacheDuration, final @NotNull Callable getLocalhost) { - this.cacheDuration = cacheDuration; + @VisibleForTesting + HostnameCache( + final @NotNull Callable getLocalhost, final @NotNull MonotonicTicker ticker) { this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required"); + this.ticker = Objects.requireNonNull(ticker, "ticker is required"); + // Nothing resolved yet, so the cache is stale rather than fresh until updateCache says + // otherwise. + this.cacheFreshUntil = Deadline.passed(ticker); // A single thread executor whose worker thread times out while idle, so no thread is kept // alive between the infrequent cache refreshes and nothing has to shut it down. final @NotNull ThreadPoolExecutor executor = @@ -104,8 +109,7 @@ public final class HostnameCache { */ @Nullable public String getHostname() { - if (expirationTimestamp < System.currentTimeMillis() - && updateRunning.compareAndSet(false, true)) { + if (cacheFreshUntil.hasPassed() && updateRunning.compareAndSet(false, true)) { updateCache(); } @@ -118,7 +122,8 @@ private void updateCache() { () -> { try { hostname = getLocalhost.call().getCanonicalHostName(); - expirationTimestamp = System.currentTimeMillis() + cacheDuration; + cacheFreshUntil = + Deadline.after(ticker, HOSTNAME_CACHE_DURATION, TimeUnit.MILLISECONDS); } finally { updateRunning.set(false); } @@ -151,7 +156,7 @@ private void updateCache() { } private void handleCacheUpdateFailure() { - expirationTimestamp = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(1); + cacheFreshUntil = Deadline.after(ticker, 1, TimeUnit.SECONDS); } private static final class HostnameCacheThreadFactory implements ThreadFactory { diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index f9b9fb13c93..3706a9f0a14 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -543,7 +543,7 @@ public class SentryOptions implements RateLimiterConfig { */ @ApiStatus.Internal private final @NotNull LazyEvaluator hostnameCache = - new LazyEvaluator<>(() -> new HostnameCache()); + new LazyEvaluator<>(() -> new HostnameCache(getMonotonicTicker())); private final @NotNull List performanceCollectors = new ArrayList<>(); diff --git a/sentry/src/test/java/io/sentry/DsnTest.kt b/sentry/src/test/java/io/sentry/DsnTest.kt index f8195d16af6..24cd35adc76 100644 --- a/sentry/src/test/java/io/sentry/DsnTest.kt +++ b/sentry/src/test/java/io/sentry/DsnTest.kt @@ -249,4 +249,29 @@ class DsnTest { assertThat(dsn.sentryUri.toURL().toString()) .isEqualTo("https://o123.ingest.sentry.io/api/1234567") } + + @Test + fun `parseHost returns the same host as the parsed DSN`() { + listOf( + "https://key@o1.ingest.us.sentry.io/1", + "https://key:secret@sentry.io:443/path/1", + " https://key@host/id ", + "http://key@localhost:9000/456", + "https://key@host:443/id?foo=bar&baz=1", + "https://key@host/id#fragment", + "http://key@[::1]:9000/1", + "https://key@evil.com/x@o1.ingest.sentry.io/1", + ) + .forEach { dsn -> assertThat(Dsn.parseHost(dsn)).isEqualTo(Dsn(dsn).sentryUri.host) } + } + + @Test + fun `parseHost returns null when there is no host`() { + assertThat(Dsn.parseHost(null)).isNull() + assertThat(Dsn.parseHost("")).isNull() + assertThat(Dsn.parseHost("host/id")).isNull() + assertThat(Dsn.parseHost("https://host/id")).isNull() + assertThat(Dsn.parseHost("https://key@host")).isNull() + assertThat(Dsn.parseHost("https://key@/id")).isNull() + } } diff --git a/sentry/src/test/java/io/sentry/HostnameCacheTest.kt b/sentry/src/test/java/io/sentry/HostnameCacheTest.kt index 6d0398858a3..b72b9d4051d 100644 --- a/sentry/src/test/java/io/sentry/HostnameCacheTest.kt +++ b/sentry/src/test/java/io/sentry/HostnameCacheTest.kt @@ -2,7 +2,7 @@ package io.sentry import com.google.common.truth.Truth.assertThat import io.sentry.test.getProperty -import io.sentry.test.injectForField +import io.sentry.time.TestMonotonicTicker import java.net.InetAddress import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit @@ -16,7 +16,7 @@ class HostnameCacheTest { private fun getSut(): HostnameCache { val address = mock() whenever(address.canonicalHostName).thenReturn("myhost") - return HostnameCache(TimeUnit.HOURS.toMillis(1)) { address } + return HostnameCache({ address }, TestMonotonicTicker()) } @Test @@ -25,13 +25,32 @@ class HostnameCacheTest { assertThat(cache.hostname).isEqualTo("myhost") } + @Test + fun `hostname is re-resolved only once the cache duration has elapsed`() { + val ticker = TestMonotonicTicker() + val address = mock() + whenever(address.canonicalHostName).thenReturn("first", "second") + val cache = HostnameCache({ address }, ticker) + + assertThat(cache.hostname).isEqualTo("first") + + ticker.advance(4, TimeUnit.HOURS) + assertThat(cache.hostname).isEqualTo("first") + + ticker.advance(1, TimeUnit.HOURS) + assertThat(cache.hostname).isEqualTo("second") + } + @Test fun `a refresh that cannot be queued does not stop later refreshes`() { - val cache = getSut() - // Reject the next submit the way an executor that could not start a thread would, and mark the - // cache stale so that reading the hostname attempts a refresh. + val ticker = TestMonotonicTicker() + val address = mock() + whenever(address.canonicalHostName).thenReturn("myhost") + val cache = HostnameCache({ address }, ticker) + // Reject the next submit the way an executor that could not start a thread would, and let the + // cache go stale so that reading the hostname attempts a refresh. cache.getProperty("executorService").shutdown() - cache.injectForField("expirationTimestamp", 0L) + ticker.advance(5, TimeUnit.HOURS) assertThat(cache.hostname).isEqualTo("myhost") diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index cded7e87755..7529d0a3e54 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -7,9 +7,11 @@ import io.sentry.protocol.DebugMeta import io.sentry.protocol.SdkVersion import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User +import io.sentry.time.TestMonotonicTicker import io.sentry.util.HintUtils import java.lang.RuntimeException import java.net.InetAddress +import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -17,7 +19,6 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue -import org.awaitility.kotlin.await import org.mockito.kotlin.mock import org.mockito.kotlin.reset import org.mockito.kotlin.times @@ -28,6 +29,7 @@ class MainEventProcessorTest { class Fixture { val scopes = mock() val getLocalhost = mock() + val hostnameCacheTicker = TestMonotonicTicker() // Built in getSut() rather than here: the constructor resolves the hostname straight away, // so it has to run after getLocalhost is stubbed. lateinit var hostnameCache: HostnameCache @@ -53,7 +55,6 @@ class MainEventProcessorTest { serverName: String? = "server", host: String? = null, resolveHostDelay: Long? = null, - hostnameCacheDuration: Long = 10, proguardUuid: String? = null, bundleIds: List? = null, modules: Map? = null, @@ -80,7 +81,7 @@ class MainEventProcessorTest { } whenever(scopes.options).thenReturn(sentryOptions) sentryTracer = SentryTracer(TransactionContext("", ""), scopes) - hostnameCache = HostnameCache(hostnameCacheDuration) { getLocalhost } + hostnameCache = HostnameCache({ getLocalhost }, hostnameCacheTicker) return MainEventProcessor(sentryOptions) } @@ -428,7 +429,7 @@ class MainEventProcessorTest { @Test fun `uses cache to retrieve servername for subsequent events`() { - val processor = fixture.getSut(serverName = null, host = "aHost", hostnameCacheDuration = 1000) + val processor = fixture.getSut(serverName = null, host = "aHost") val firstEvent = SentryEvent() processor.process(firstEvent, Hint()) assertEquals("aHost", firstEvent.serverName) @@ -447,12 +448,11 @@ class MainEventProcessorTest { reset(fixture.getLocalhost) whenever(fixture.getLocalhost.canonicalHostName).thenReturn("newHost") + fixture.hostnameCacheTicker.advance(6, TimeUnit.HOURS) - await.untilAsserted { - val secondEvent = SentryEvent() - processor.process(secondEvent, Hint()) - assertEquals("newHost", secondEvent.serverName) - } + val secondEvent = SentryEvent() + processor.process(secondEvent, Hint()) + assertEquals("newHost", secondEvent.serverName) } @Test diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 101a136f642..284c0a98cd5 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -4,6 +4,8 @@ import com.google.common.truth.Truth.assertThat import io.sentry.SentryOptions.RequestSize import io.sentry.logger.ILoggerBatchProcessorFactory import io.sentry.test.getProperty +import io.sentry.time.MonotonicTicker +import io.sentry.time.TestMonotonicTicker import io.sentry.util.LazyEvaluator import io.sentry.util.StringUtils import java.io.File @@ -1337,9 +1339,16 @@ class SentryOptionsTest { private fun SentryOptions.peekHostnameCache(): HostnameCache? = getProperty>("hostnameCache").getProperty("value") + /** Options whose hostname cache resolves against a ticker the test controls. */ + private fun optionsWithTicker(ticker: MonotonicTicker): SentryOptions = + object : SentryOptions() { + override fun getMonotonicTicker(): MonotonicTicker = ticker + } + @Test fun `hostname is resolved on first use, not when options are constructed`() { - val options = SentryOptions() + val ticker = TestMonotonicTicker() + val options = optionsWithTicker(ticker) assertThat(options.peekHostnameCache()).isNull() @@ -1348,5 +1357,6 @@ class SentryOptionsTest { // Also keeps the assertion above honest: peek does report a cache once one exists. assertThat(options.peekHostnameCache()).isSameInstanceAs(cache) assertThat(options.hostnameCache).isSameInstanceAs(cache) + assertThat(cache.getProperty("ticker")).isSameInstanceAs(ticker) } }