diff --git a/CHANGELOG.md b/CHANGELOG.md index d80037414d..be4a5f7628 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Improvements + +- Improve SDK init performance by replacing `java.net.URI` with custom string parsing for DSN ([#5448](https://github.com/getsentry/sentry-java/pull/5448)) + ### Fixes - Session Replay: Fix `VerifyError` in Compose masking under DexGuard/R8 obfuscation ([#5507](https://github.com/getsentry/sentry-java/pull/5507)) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7ee39d75ed..e653069e2b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -238,6 +238,7 @@ camerax-camera2 = { module = "androidx.camera:camera-camera2", version.ref = "ca camerax-lifecycle = { module = "androidx.camera:camera-lifecycle", version.ref = "camerax" } camerax-view = { module = "androidx.camera:camera-view", version.ref = "camerax" } +google-truth = { module = "com.google.truth:truth", version = "1.4.5" } hsqldb = { module = "org.hsqldb:hsqldb", version = "2.6.1" } javafaker = { module = "com.github.javafaker:javafaker", version = "1.0.2" } kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "coroutines" } diff --git a/sentry/build.gradle.kts b/sentry/build.gradle.kts index 25e700995b..4c237803a5 100644 --- a/sentry/build.gradle.kts +++ b/sentry/build.gradle.kts @@ -25,6 +25,7 @@ dependencies { // tests testImplementation(kotlin(Config.kotlinStdLib)) testImplementation(libs.awaitility.kotlin) + testImplementation(libs.google.truth) testImplementation(libs.javafaker) testImplementation(libs.kotlin.test.junit) testImplementation(libs.mockito.kotlin) diff --git a/sentry/src/main/java/io/sentry/Dsn.java b/sentry/src/main/java/io/sentry/Dsn.java index 0d21499b5f..15aea2a064 100644 --- a/sentry/src/main/java/io/sentry/Dsn.java +++ b/sentry/src/main/java/io/sentry/Dsn.java @@ -2,6 +2,7 @@ import io.sentry.util.Objects; import java.net.URI; +import java.net.URISyntaxException; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.jetbrains.annotations.NotNull; @@ -17,98 +18,148 @@ final class Dsn { private final @NotNull URI sentryUri; private final @Nullable String orgId; - /* - / The project ID which the authenticated user is bound to. - */ + /** The project ID which the authenticated user is bound to. */ public @NotNull String getProjectId() { return projectId; } - /* - / An optional path of which Sentry is hosted - */ + /** An optional path of which Sentry is hosted. */ public @Nullable String getPath() { return path; } - /* - / The optional secret key to authenticate the SDK. - */ + /** The optional secret key to authenticate the SDK. */ public @Nullable String getSecretKey() { return secretKey; } - /* - / The required public key to authenticate the SDK. - */ + /** The required public key to authenticate the SDK. */ public @NotNull String getPublicKey() { return publicKey; } - /* - / The URI used to communicate with Sentry - */ + /** The org ID extracted from the host, or {@code null} when the host has no org prefix. */ + public @Nullable String getOrgId() { + return orgId; + } + + /** The URI used to communicate with Sentry. */ @NotNull URI getSentryUri() { return sentryUri; } + // Avoids java.net.URI for DSN parsing, which is slow on Android. Dsn(@Nullable String dsn) throws IllegalArgumentException { + final String dsnString = Objects.requireNonNull(dsn, "The DSN is required.").trim(); + if (dsnString.isEmpty()) { + throw new IllegalArgumentException("The DSN is empty."); + } + try { - final String dsnString = Objects.requireNonNull(dsn, "The DSN is required.").trim(); - if (dsnString.isEmpty()) { - throw new IllegalArgumentException("The DSN is empty."); + final int schemeEnd = dsnString.indexOf("://"); + if (schemeEnd < 0) { + throw new IllegalArgumentException("Invalid DSN: Missing scheme."); } - final URI uri = new URI(dsnString).normalize(); - final String scheme = uri.getScheme(); - if (!("http".equalsIgnoreCase(scheme) || "https".equalsIgnoreCase(scheme))) { - throw new IllegalArgumentException("Invalid DSN scheme: " + scheme); + final String scheme = dsnString.substring(0, schemeEnd); + if (!"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) { + throw new IllegalArgumentException("Invalid DSN: Invalid scheme '" + scheme + "'."); } - String userInfo = uri.getUserInfo(); - if (userInfo == null || userInfo.isEmpty()) { + final int authStart = schemeEnd + 3; + final int atIndex = dsnString.indexOf('@', authStart); + if (atIndex < 0) { throw new IllegalArgumentException("Invalid DSN: No public key provided."); } - String[] keys = userInfo.split(":", -1); - publicKey = keys[0]; - if (publicKey == null || publicKey.isEmpty()) { + final String userInfo = dsnString.substring(authStart, atIndex); + final int colonIndex = userInfo.indexOf(':'); + publicKey = colonIndex < 0 ? userInfo : userInfo.substring(0, colonIndex); + secretKey = colonIndex < 0 ? null : userInfo.substring(colonIndex + 1); + if (publicKey.isEmpty()) { throw new IllegalArgumentException("Invalid DSN: No public key provided."); } - secretKey = keys.length > 1 ? keys[1] : null; - String uriPath = uri.getPath(); - if (uriPath.endsWith("/")) { - uriPath = uriPath.substring(0, uriPath.length() - 1); - } - int projectIdStart = uriPath.lastIndexOf("/") + 1; - String path = uriPath.substring(0, projectIdStart); - if (!path.endsWith("/")) { - path += "/"; + + final String hostAndPath = stripQueryAndFragment(dsnString, atIndex + 1); + final int firstSlash = hostAndPath.indexOf('/'); + if (firstSlash < 0) { + throw new IllegalArgumentException("Invalid DSN: A Project Id is required."); } - this.path = path; - projectId = uriPath.substring(projectIdStart); + + final String hostPort = hostAndPath.substring(0, firstSlash); + final int portColon = portSeparatorIndex(hostPort); + final String host = portColon < 0 ? hostPort : hostPort.substring(0, portColon); + final int port = portColon < 0 ? -1 : parsePort(hostPort.substring(portColon + 1)); + + final String rawPath = stripTrailingSlash(collapseSlashes(hostAndPath.substring(firstSlash))); + final int projectIdStart = rawPath.lastIndexOf('/') + 1; + path = ensureTrailingSlash(rawPath.substring(0, projectIdStart)); + projectId = rawPath.substring(projectIdStart); if (projectId.isEmpty()) { throw new IllegalArgumentException("Invalid DSN: A Project Id is required."); } - sentryUri = - new URI( - scheme, null, uri.getHost(), uri.getPort(), path + "api/" + projectId, null, null); - - // Extract org ID from host (e.g., "o123.ingest.sentry.io" -> "123") - String extractedOrgId = null; - final String host = uri.getHost(); - if (host != null) { - final Matcher matcher = ORG_ID_PATTERN.matcher(host); - if (matcher.find()) { - extractedOrgId = matcher.group(1); - } + + sentryUri = new URI(scheme, null, host, port, path + "api/" + projectId, null, null); + orgId = extractOrgId(host); + } catch (URISyntaxException e) { + throw new IllegalArgumentException("Invalid DSN: " + e.getMessage(), e); + } + } + + private static int parsePort(final @NotNull String portString) { + try { + return Integer.parseInt(portString); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid DSN: Invalid port '" + portString + "'.", e); + } + } + + // Drops the query string and/or fragment, whichever appears first, from the host onwards. + private static @NotNull String stripQueryAndFragment( + final @NotNull String dsn, final int fromIndex) { + int cut = dsn.indexOf('?', fromIndex); + final int fragment = dsn.indexOf('#', fromIndex); + if (fragment >= 0 && (cut < 0 || fragment < cut)) { + cut = fragment; + } + return cut < 0 ? dsn.substring(fromIndex) : dsn.substring(fromIndex, cut); + } + + // IPv6 literals are bracketed and contain colons, so the port separator follows the ']'. + private static int portSeparatorIndex(final @NotNull String hostPort) { + return hostPort.startsWith("[") + ? hostPort.indexOf(':', hostPort.indexOf(']')) + : hostPort.indexOf(':'); + } + + // Collapses runs of slashes into a single slash, like URI.normalize(). + private static @NotNull String collapseSlashes(final @NotNull String path) { + if (!path.contains("//")) { + return path; + } + final StringBuilder sb = new StringBuilder(path.length()); + char previous = 0; + for (int i = 0; i < path.length(); i++) { + final char c = path.charAt(i); + if (c == '/' && previous == '/') { + continue; } - orgId = extractedOrgId; - } catch (Throwable e) { - throw new IllegalArgumentException(e); + sb.append(c); + previous = c; } + return sb.toString(); } - public @Nullable String getOrgId() { - return orgId; + private static @NotNull String stripTrailingSlash(final @NotNull String path) { + return path.endsWith("/") ? path.substring(0, path.length() - 1) : path; + } + + private static @NotNull String ensureTrailingSlash(final @NotNull String path) { + return path.endsWith("/") ? path : path + "/"; + } + + // Extracts the org ID from a host such as "o123.ingest.sentry.io" -> "123". + private static @Nullable String extractOrgId(final @NotNull String host) { + final Matcher matcher = ORG_ID_PATTERN.matcher(host); + return matcher.find() ? matcher.group(1) : null; } } diff --git a/sentry/src/test/java/io/sentry/DsnTest.kt b/sentry/src/test/java/io/sentry/DsnTest.kt index 7e2982073f..f8195d16af 100644 --- a/sentry/src/test/java/io/sentry/DsnTest.kt +++ b/sentry/src/test/java/io/sentry/DsnTest.kt @@ -1,21 +1,20 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import java.lang.IllegalArgumentException import kotlin.test.Test -import kotlin.test.assertEquals import kotlin.test.assertFailsWith -import kotlin.test.assertNull class DsnTest { @Test fun `dsn parsed with path, sets all properties`() { val dsn = Dsn("https://publicKey:secretKey@host/path/id") - assertEquals("https://host/path/api/id", dsn.sentryUri.toURL().toString()) - assertEquals("publicKey", dsn.publicKey) - assertEquals("secretKey", dsn.secretKey) - assertEquals("/path/", dsn.path) - assertEquals("id", dsn.projectId) + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://host/path/api/id") + assertThat(dsn.publicKey).isEqualTo("publicKey") + assertThat(dsn.secretKey).isEqualTo("secretKey") + assertThat(dsn.path).isEqualTo("/path/") + assertThat(dsn.projectId).isEqualTo("id") } @Test @@ -23,94 +22,90 @@ class DsnTest { // query strings were once a feature, but no more val dsn = Dsn("https://publicKey:secretKey@host/path/id?sample.rate=0.1") - assertEquals("https://host/path/api/id", dsn.sentryUri.toURL().toString()) - assertEquals("publicKey", dsn.publicKey) - assertEquals("secretKey", dsn.secretKey) - assertEquals("/path/", dsn.path) - assertEquals("id", dsn.projectId) + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://host/path/api/id") + assertThat(dsn.publicKey).isEqualTo("publicKey") + assertThat(dsn.secretKey).isEqualTo("secretKey") + assertThat(dsn.path).isEqualTo("/path/") + assertThat(dsn.projectId).isEqualTo("id") } @Test fun `dsn parsed without path`() { val dsn = Dsn("https://key@host/id") - assertEquals("https://host/api/id", dsn.sentryUri.toURL().toString()) + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://host/api/id") } @Test fun `dsn parsed with port number`() { val dsn = Dsn("http://key@host:69/id") - assertEquals("http://host:69/api/id", dsn.sentryUri.toURL().toString()) + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("http://host:69/api/id") } @Test fun `dsn parsed with trailing slash`() { val dsn = Dsn("http://key@host/id/") - assertEquals("http://host/api/id", dsn.sentryUri.toURL().toString()) + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("http://host/api/id") } @Test fun `dsn parsed with no delimiter for key`() { val dsn = Dsn("https://publicKey@host/id") - assertEquals("publicKey", dsn.publicKey) - assertNull(dsn.secretKey) + assertThat(dsn.publicKey).isEqualTo("publicKey") + assertThat(dsn.secretKey).isNull() } @Test fun `when no project id exists, throws exception`() { val ex = assertFailsWith { Dsn("http://key@host/") } - assertEquals( - "java.lang.IllegalArgumentException: Invalid DSN: A Project Id is required.", - ex.message, - ) + assertThat(ex).hasMessageThat().isEqualTo("Invalid DSN: A Project Id is required.") } @Test fun `when no key exists, throws exception`() { val ex = assertFailsWith { Dsn("http://host/id") } - assertEquals( - "java.lang.IllegalArgumentException: Invalid DSN: No public key provided.", - ex.message, - ) + assertThat(ex).hasMessageThat().isEqualTo("Invalid DSN: No public key provided.") } @Test fun `when only passing secret key, throws exception`() { val ex = assertFailsWith { Dsn("https://:secret@host/path/id") } - assertEquals( - "java.lang.IllegalArgumentException: Invalid DSN: No public key provided.", - ex.message, - ) + assertThat(ex).hasMessageThat().isEqualTo("Invalid DSN: No public key provided.") } @Test fun `dsn is normalized`() { val dsn = Dsn("http://key@host//id") - assertEquals("http://host/api/id", dsn.sentryUri.toURL().toString()) + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("http://host/api/id") } @Test fun `dsn parsed with leading and trailing whitespace`() { val dsn = Dsn(" https://key@host/id ") - assertEquals("https://host/api/id", dsn.sentryUri.toURL().toString()) + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://host/api/id") } @Test fun `when dsn is empty, throws exception`() { val ex = assertFailsWith { Dsn("") } - assertEquals("java.lang.IllegalArgumentException: The DSN is empty.", ex.message) + assertThat(ex).hasMessageThat().isEqualTo("The DSN is empty.") } @Test fun `when dsn is only whitespace, throws exception`() { val ex = assertFailsWith { Dsn(" ") } - assertEquals("java.lang.IllegalArgumentException: The DSN is empty.", ex.message) + assertThat(ex).hasMessageThat().isEqualTo("The DSN is empty.") } @Test fun `non http protocols are not accepted`() { - assertFailsWith { Dsn("ftp://publicKey:secretKey@host/path/id") } - assertFailsWith { Dsn("jar://publicKey:secretKey@host/path/id") } + val ftp = + assertFailsWith { Dsn("ftp://publicKey:secretKey@host/path/id") } + assertThat(ftp).hasMessageThat().isEqualTo("Invalid DSN: Invalid scheme 'ftp'.") + + val jar = + assertFailsWith { Dsn("jar://publicKey:secretKey@host/path/id") } + assertThat(jar).hasMessageThat().isEqualTo("Invalid DSN: Invalid scheme 'jar'.") } @Test @@ -125,24 +120,133 @@ class DsnTest { @Test fun `extracts org id from host`() { val dsn = Dsn("https://key@o123.ingest.sentry.io/456") - assertEquals("123", dsn.orgId) + assertThat(dsn.orgId).isEqualTo("123") } @Test fun `extracts single digit org id from host`() { val dsn = Dsn("https://key@o1.ingest.us.sentry.io/456") - assertEquals("1", dsn.orgId) + assertThat(dsn.orgId).isEqualTo("1") } @Test fun `returns null org id when host has no org prefix`() { val dsn = Dsn("https://key@sentry.io/456") - assertNull(dsn.orgId) + assertThat(dsn.orgId).isNull() } @Test fun `returns null org id for non-standard host`() { val dsn = Dsn("http://key@localhost:9000/456") - assertNull(dsn.orgId) + assertThat(dsn.orgId).isNull() + } + + @Test + fun `when dsn is null, throws exception`() { + val ex = assertFailsWith { Dsn(null) } + assertThat(ex).hasMessageThat().isEqualTo("The DSN is required.") + } + + @Test + fun `when dsn has no scheme separator, throws exception`() { + val ex = assertFailsWith { Dsn("httpspublicKey@host/id") } + assertThat(ex).hasMessageThat().isEqualTo("Invalid DSN: Missing scheme.") + } + + @Test + fun `when dsn has no slash after host, throws exception`() { + val ex = assertFailsWith { Dsn("https://key@host") } + assertThat(ex).hasMessageThat().isEqualTo("Invalid DSN: A Project Id is required.") + } + + @Test + fun `when port is not a number, throws exception`() { + val ex = assertFailsWith { Dsn("http://key@host:abc/1") } + assertThat(ex).hasMessageThat().isEqualTo("Invalid DSN: Invalid port 'abc'.") + } + + @Test + fun `dsn parsed with multiple path segments`() { + val dsn = Dsn("https://key@host/path/to/sentry/id") + + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://host/path/to/sentry/api/id") + assertThat(dsn.publicKey).isEqualTo("key") + assertThat(dsn.path).isEqualTo("/path/to/sentry/") + assertThat(dsn.projectId).isEqualTo("id") + } + + @Test + fun `dsn parsed with port and path`() { + val dsn = Dsn("http://key:secret@host:8080/path/id") + + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("http://host:8080/path/api/id") + assertThat(dsn.publicKey).isEqualTo("key") + assertThat(dsn.secretKey).isEqualTo("secret") + assertThat(dsn.path).isEqualTo("/path/") + assertThat(dsn.projectId).isEqualTo("id") + } + + @Test + fun `dsn with multiple double slashes in path is normalized`() { + val dsn = Dsn("http://key@host//path//id") + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("http://host/path/api/id") + } + + @Test + fun `dsn with query string and port`() { + val dsn = Dsn("https://key@host:443/id?foo=bar&baz=1") + + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://host:443/api/id") + assertThat(dsn.projectId).isEqualTo("id") + } + + @Test + fun `dsn with fragment is stripped from project id`() { + val dsn = Dsn("https://key@host/123#frag") + + assertThat(dsn.projectId).isEqualTo("123") + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://host/api/123") + } + + @Test + fun `dsn with both query string and fragment is stripped from project id`() { + val dsn = Dsn("https://key@host/123?foo=bar#frag") + + assertThat(dsn.projectId).isEqualTo("123") + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://host/api/123") + } + + @Test + fun `dsn with ipv6 host and port`() { + val dsn = Dsn("https://key@[2001:db8::1]:9000/1") + + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://[2001:db8::1]:9000/api/1") + assertThat(dsn.projectId).isEqualTo("1") + } + + @Test + fun `dsn with ipv6 host and no port`() { + val dsn = Dsn("https://key@[::1]/1") + + assertThat(dsn.sentryUri.toURL().toString()).isEqualTo("https://[::1]/api/1") + assertThat(dsn.projectId).isEqualTo("1") + } + + @Test + fun `dsn with empty secret key after colon`() { + val dsn = Dsn("https://publicKey:@host/id") + + assertThat(dsn.publicKey).isEqualTo("publicKey") + assertThat(dsn.secretKey).isEqualTo("") + } + + @Test + fun `dsn with numeric project id`() { + val dsn = Dsn("https://key@o123.ingest.sentry.io/1234567") + + assertThat(dsn.projectId).isEqualTo("1234567") + assertThat(dsn.orgId).isEqualTo("123") + assertThat(dsn.sentryUri.toURL().toString()) + .isEqualTo("https://o123.ingest.sentry.io/api/1234567") } }