diff --git a/CHANGELOG.md b/CHANGELOG.md index 18d17e3013..6da128bea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * Increase max cached events to 30 (#1029) * Normalize DSN URI (#1030) +* Enhancement: Resolve HTTP Proxy parameters from the external configuration (#1028) # 3.1.2 diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 0e4d621d02..0c513f733e 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -92,7 +92,11 @@ class SentryAutoConfigurationTest { "sentry.dist=my-dist", "sentry.attach-threads=true", "sentry.attach-stacktrace=true", - "sentry.server-name=host-001" + "sentry.server-name=host-001", + "sentry.proxy.host=example.proxy.com", + "sentry.proxy.port=8090", + "sentry.proxy.user=proxy-user", + "sentry.proxy.pass=proxy-pass" ).run { val options = it.getBean(SentryOptions::class.java) assertThat(options.readTimeoutMillis).isEqualTo(10) @@ -110,6 +114,11 @@ class SentryAutoConfigurationTest { assertThat(options.isAttachThreads).isEqualTo(true) assertThat(options.isAttachStacktrace).isEqualTo(true) assertThat(options.serverName).isEqualTo("host-001") + assertThat(options.proxy).isNotNull() + assertThat(options.proxy!!.host).isEqualTo("example.proxy.com") + assertThat(options.proxy!!.port).isEqualTo("8090") + assertThat(options.proxy!!.user).isEqualTo("proxy-user") + assertThat(options.proxy!!.pass).isEqualTo("proxy-pass") } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index fcc9c1a179..91fbdbd056 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -10,7 +10,6 @@ import io.sentry.transport.NoOpTransport; import io.sentry.transport.NoOpTransportGate; import java.io.File; -import java.net.Proxy; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -27,6 +26,9 @@ public class SentryOptions { /** Default Log level if not specified Default is DEBUG */ static final SentryLevel DEFAULT_DIAGNOSTIC_LEVEL = SentryLevel.DEBUG; + /** The default HTTP proxy port to use if an HTTP Proxy hostname is set but port is not. */ + private static final String PROXY_PORT_DEFAULT = "80"; + /** * Are callbacks that run for every event. They can either return a new event which in most cases * means just adding data OR return null in case the event will be dropped and not sent. @@ -238,6 +240,15 @@ public class SentryOptions { options.setRelease(propertiesProvider.getProperty("release")); options.setDist(propertiesProvider.getProperty("dist")); options.setServerName(propertiesProvider.getProperty("servername")); + + final String proxyHost = propertiesProvider.getProperty("proxy.host"); + final String proxyUser = propertiesProvider.getProperty("proxy.user"); + final String proxyPass = propertiesProvider.getProperty("proxy.pass"); + final String proxyPort = propertiesProvider.getProperty("proxy.port", PROXY_PORT_DEFAULT); + + if (proxyHost != null) { + options.setProxy(new Proxy(proxyHost, proxyPort, proxyUser, proxyPass)); + } return options; } @@ -1129,6 +1140,9 @@ void merge(final @NotNull SentryOptions options) { if (options.getServerName() != null) { setServerName(options.getServerName()); } + if (options.getProxy() != null) { + setProxy(options.getProxy()); + } } private @NotNull SdkVersion createSdkVersion() { @@ -1141,4 +1155,62 @@ void merge(final @NotNull SentryOptions options) { return sdkVersion; } + + public static final class Proxy { + private @Nullable String host; + private @Nullable String port; + private @Nullable String user; + private @Nullable String pass; + + public Proxy( + final @Nullable String host, + final @Nullable String port, + final @Nullable String user, + final @Nullable String pass) { + this.host = host; + this.port = port; + this.user = user; + this.pass = pass; + } + + public Proxy() { + this(null, null, null, null); + } + + public Proxy(@Nullable String host, @Nullable String port) { + this(host, port, null, null); + } + + public @Nullable String getHost() { + return host; + } + + public void setHost(final @Nullable String host) { + this.host = host; + } + + public @Nullable String getPort() { + return port; + } + + public void setPort(final @Nullable String port) { + this.port = port; + } + + public @Nullable String getUser() { + return user; + } + + public void setUser(final @Nullable String user) { + this.user = user; + } + + public @Nullable String getPass() { + return pass; + } + + public void setPass(final @Nullable String pass) { + this.pass = pass; + } + } } diff --git a/sentry/src/main/java/io/sentry/config/PropertiesProvider.java b/sentry/src/main/java/io/sentry/config/PropertiesProvider.java index 5274792c01..080a6772be 100644 --- a/sentry/src/main/java/io/sentry/config/PropertiesProvider.java +++ b/sentry/src/main/java/io/sentry/config/PropertiesProvider.java @@ -12,4 +12,17 @@ public interface PropertiesProvider { */ @Nullable String getProperty(@NotNull String property); + + /** + * Resolves property given by it's name. + * + * @param property - the property name + * @param defaultValue - the default value if property is not set + * @return property value or the default value if not found. + */ + @NotNull + default String getProperty(@NotNull String property, @NotNull String defaultValue) { + final String result = getProperty(property); + return result != null ? result : defaultValue; + } } diff --git a/sentry/src/main/java/io/sentry/transport/AuthenticatorWrapper.java b/sentry/src/main/java/io/sentry/transport/AuthenticatorWrapper.java new file mode 100644 index 0000000000..b981fc74fe --- /dev/null +++ b/sentry/src/main/java/io/sentry/transport/AuthenticatorWrapper.java @@ -0,0 +1,19 @@ +package io.sentry.transport; + +import java.net.Authenticator; +import org.jetbrains.annotations.NotNull; + +/** Wraps {@link Authenticator} in order to make classes that use it testable. */ +final class AuthenticatorWrapper { + private static final AuthenticatorWrapper instance = new AuthenticatorWrapper(); + + public static AuthenticatorWrapper getInstance() { + return instance; + } + + private AuthenticatorWrapper() {} + + public void setDefault(final @NotNull Authenticator authenticator) { + Authenticator.setDefault(authenticator); + } +} diff --git a/sentry/src/main/java/io/sentry/transport/HttpTransport.java b/sentry/src/main/java/io/sentry/transport/HttpTransport.java index 64ec1d65eb..456b828493 100644 --- a/sentry/src/main/java/io/sentry/transport/HttpTransport.java +++ b/sentry/src/main/java/io/sentry/transport/HttpTransport.java @@ -21,6 +21,7 @@ import java.io.OutputStreamWriter; import java.io.Writer; import java.net.HttpURLConnection; +import java.net.InetSocketAddress; import java.net.MalformedURLException; import java.net.Proxy; import java.net.URI; @@ -37,6 +38,7 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * An implementation of the {@link ITransport} interface that sends the events to the Sentry server @@ -120,7 +122,8 @@ public HttpTransport( sslSocketFactory, hostnameVerifier, sentryUrl, - CurrentDateProvider.getInstance()); + CurrentDateProvider.getInstance(), + AuthenticatorWrapper.getInstance()); } HttpTransport( @@ -131,8 +134,8 @@ public HttpTransport( final @Nullable SSLSocketFactory sslSocketFactory, final @Nullable HostnameVerifier hostnameVerifier, final @NotNull URL sentryUrl, - final @NotNull ICurrentDateProvider currentDateProvider) { - this.proxy = options.getProxy(); + final @NotNull ICurrentDateProvider currentDateProvider, + final @NotNull AuthenticatorWrapper authenticatorWrapper) { this.connectionConfigurator = connectionConfigurator; this.serializer = options.getSerializer(); this.connectionTimeout = connectionTimeoutMillis; @@ -150,6 +153,39 @@ public HttpTransport( } catch (URISyntaxException | MalformedURLException e) { throw new IllegalArgumentException("Failed to compose the Sentry's server URL.", e); } + + this.proxy = resolveProxy(options.getProxy()); + + if (proxy != null && options.getProxy() != null) { + final String proxyUser = options.getProxy().getUser(); + final String proxyPassword = options.getProxy().getPass(); + + if (proxyUser != null && proxyPassword != null) { + authenticatorWrapper.setDefault(new ProxyAuthenticator(proxyUser, proxyPassword)); + } + } + } + + private @Nullable Proxy resolveProxy(final @Nullable SentryOptions.Proxy optionsProxy) { + Proxy proxy = null; + if (optionsProxy != null) { + final String port = optionsProxy.getPort(); + final String host = optionsProxy.getHost(); + if (port != null && host != null) { + try { + InetSocketAddress proxyAddr = new InetSocketAddress(host, Integer.parseInt(port)); + proxy = new Proxy(Proxy.Type.HTTP, proxyAddr); + } catch (NumberFormatException e) { + logger.log( + ERROR, + e, + "Failed to parse Sentry Proxy port: " + + optionsProxy.getPort() + + ". Proxy is ignored"); + } + } + } + return proxy; } protected @NotNull HttpURLConnection open() throws IOException { @@ -471,6 +507,11 @@ private boolean isSuccessfulResponseCode(final int responseCode) { return responseCode == HTTP_OK; } + @TestOnly + Proxy getProxy() { + return proxy; + } + @Override public void close() throws IOException { // a connection is opened and closed for each request, so this method is not used at all. diff --git a/sentry/src/main/java/io/sentry/transport/ProxyAuthenticator.java b/sentry/src/main/java/io/sentry/transport/ProxyAuthenticator.java new file mode 100644 index 0000000000..53d54e128e --- /dev/null +++ b/sentry/src/main/java/io/sentry/transport/ProxyAuthenticator.java @@ -0,0 +1,38 @@ +package io.sentry.transport; + +import io.sentry.util.Objects; +import java.net.Authenticator; +import java.net.PasswordAuthentication; +import org.jetbrains.annotations.NotNull; + +final class ProxyAuthenticator extends Authenticator { + private final @NotNull String user; + private final @NotNull String password; + + /** + * Proxy authenticator. + * + * @param user proxy username + * @param password proxy password + */ + ProxyAuthenticator(final @NotNull String user, final @NotNull String password) { + this.user = Objects.requireNonNull(user, "user is required"); + this.password = Objects.requireNonNull(password, "password is required"); + } + + @Override + protected PasswordAuthentication getPasswordAuthentication() { + if (getRequestorType() == RequestorType.PROXY) { + return new PasswordAuthentication(user, password.toCharArray()); + } + return null; + } + + String getUser() { + return user; + } + + String getPassword() { + return password; + } +} diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 6b23cde754..c67a460192 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -1,6 +1,7 @@ package io.sentry import com.nhaarman.mockitokotlin2.mock +import io.sentry.config.PropertiesProviderFactory import java.io.File import kotlin.test.Test import kotlin.test.assertEquals @@ -9,6 +10,7 @@ import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue +import org.junit.rules.TemporaryFolder class SentryOptionsTest { @Test @@ -170,6 +172,7 @@ class SentryOptionsTest { externalOptions.environment = "environment" externalOptions.release = "release" externalOptions.serverName = "serverName" + externalOptions.proxy = SentryOptions.Proxy("example.com", "8090") val options = SentryOptions() options.merge(externalOptions) @@ -179,5 +182,53 @@ class SentryOptionsTest { assertEquals("environment", options.environment) assertEquals("release", options.release) assertEquals("serverName", options.serverName) + assertNotNull(options.proxy) + assertEquals("example.com", options.proxy!!.host) + assertEquals("8090", options.proxy!!.port) + } + + @Test + fun `creates options with proxy using external properties`() { + // create a sentry.properties file in temporary folder + val temporaryFolder = TemporaryFolder() + temporaryFolder.create() + val file = temporaryFolder.newFile("sentry.properties") + file.appendText("proxy.host=proxy.example.com\n") + file.appendText("proxy.port=9090\n") + file.appendText("proxy.user=some-user\n") + file.appendText("proxy.pass=some-pass") + // set location of the sentry.properties file + System.setProperty("sentry.properties.file", file.absolutePath) + + try { + val options = SentryOptions.from(PropertiesProviderFactory.create()) + assertNotNull(options.proxy) + assertEquals("proxy.example.com", options.proxy!!.host) + assertEquals("9090", options.proxy!!.port) + assertEquals("some-user", options.proxy!!.user) + assertEquals("some-pass", options.proxy!!.pass) + } finally { + temporaryFolder.delete() + } + } + + @Test + fun `when proxy port is not set default proxy port is used`() { + // create a sentry.properties file in temporary folder + val temporaryFolder = TemporaryFolder() + temporaryFolder.create() + val file = temporaryFolder.newFile("sentry.properties") + file.appendText("proxy.host=proxy.example.com\n") + // set location of the sentry.properties file + System.setProperty("sentry.properties.file", file.absolutePath) + + try { + val options = SentryOptions.from(PropertiesProviderFactory.create()) + assertNotNull(options.proxy) + assertEquals("proxy.example.com", options.proxy!!.host) + assertEquals("80", options.proxy!!.port) + } finally { + temporaryFolder.delete() + } } } diff --git a/sentry/src/test/java/io/sentry/config/PropertiesProviderTest.kt b/sentry/src/test/java/io/sentry/config/PropertiesProviderTest.kt new file mode 100644 index 0000000000..9ca46d42ba --- /dev/null +++ b/sentry/src/test/java/io/sentry/config/PropertiesProviderTest.kt @@ -0,0 +1,26 @@ +package io.sentry.config + +import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.spy +import com.nhaarman.mockitokotlin2.whenever +import kotlin.test.Test +import kotlin.test.assertEquals + +class PropertiesProviderTest { + + private val propertiesProvider = spy() + + @Test + fun `when property is set returns the property value`() { + whenever(propertiesProvider.getProperty(any())).thenReturn("value") + val result = propertiesProvider.getProperty("prop", "defaultValue") + assertEquals("value", result) + } + + @Test + fun `when property not set returns the default value`() { + whenever(propertiesProvider.getProperty(any())).thenReturn(null) + val result = propertiesProvider.getProperty("prop", "defaultValue") + assertEquals("defaultValue", result) + } +} diff --git a/sentry/src/test/java/io/sentry/transport/HttpTransportTest.kt b/sentry/src/test/java/io/sentry/transport/HttpTransportTest.kt index 686ca6d39e..7c0b751ef6 100644 --- a/sentry/src/test/java/io/sentry/transport/HttpTransportTest.kt +++ b/sentry/src/test/java/io/sentry/transport/HttpTransportTest.kt @@ -1,19 +1,23 @@ package io.sentry.transport import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.check import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.verifyZeroInteractions import com.nhaarman.mockitokotlin2.whenever import io.sentry.ISerializer import io.sentry.SentryEnvelope import io.sentry.SentryEvent import io.sentry.SentryOptions +import io.sentry.SentryOptions.Proxy import io.sentry.Session import io.sentry.protocol.User import java.io.IOException -import java.net.Proxy +import java.net.InetSocketAddress +import java.net.Proxy.Type import java.net.URI import java.net.URL import javax.net.ssl.HostnameVerifier @@ -22,6 +26,7 @@ import javax.net.ssl.SSLSocketFactory import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertTrue class HttpTransportTest { @@ -35,6 +40,7 @@ class HttpTransportTest { var readTimeout = 500 val connection = mock() val currentDateProvider = mock() + val authenticatorWrapper = mock() var sslSocketFactory: SSLSocketFactory? = null var hostnameVerifier: HostnameVerifier? = null @@ -52,7 +58,7 @@ class HttpTransportTest { options.sslSocketFactory = sslSocketFactory options.hostnameVerifier = hostnameVerifier - return object : HttpTransport(options, requestUpdater, connectionTimeout, readTimeout, sslSocketFactory, hostnameVerifier, dsn, currentDateProvider) { + return object : HttpTransport(options, requestUpdater, connectionTimeout, readTimeout, sslSocketFactory, hostnameVerifier, dsn, currentDateProvider, authenticatorWrapper) { override fun open(): HttpsURLConnection { return connection } @@ -313,6 +319,40 @@ class HttpTransportTest { verify(fixture.connection, never()).hostnameVerifier = any() } + @Test + fun `When Proxy host and port are given, set to connection`() { + fixture.proxy = Proxy("proxy.example.com", "8090") + val transport = fixture.getSUT() + + transport.send(createEnvelope()) + + assertEquals(java.net.Proxy(Type.HTTP, InetSocketAddress("proxy.example.com", 8090)), transport.proxy) + } + + @Test + fun `When Proxy username and password are given, set to connection`() { + fixture.proxy = Proxy("proxy.example.com", "8090", "some-user", "some-password") + val transport = fixture.getSUT() + + transport.send(createEnvelope()) + + verify(fixture.authenticatorWrapper).setDefault(check { + assertEquals("some-user", it.user) + assertEquals("some-password", it.password) + }) + } + + @Test + fun `When Proxy port has invalid format, proxy is not set to connection`() { + fixture.proxy = Proxy("proxy.example.com", "xxx") + val transport = fixture.getSUT() + + transport.send(createEnvelope()) + + assertNull(transport.proxy) + verifyZeroInteractions(fixture.authenticatorWrapper) + } + private fun createSession(): Session { return Session("123", User(), "env", "release") }