Skip to content

Commit

Permalink
Resolve HTTP Proxy parameters from the external configuration. (#1035)
Browse files Browse the repository at this point in the history
  • Loading branch information
maciejwalkowiak authored Nov 16, 2020
1 parent bcbd5c5 commit 94691a2
Show file tree
Hide file tree
Showing 10 changed files with 317 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Fix broken NDK integration on 3.1.2 (release failed on packaging a .so file)
* Increase max cached events to 30 (#1029)
* Normalize DSN URI (#1030)
* Enhancement: Resolve HTTP Proxy parameters from the external configuration (#1028)

# 3.1.2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
}
}

Expand Down
74 changes: 73 additions & 1 deletion sentry/src/main/java/io/sentry/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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() {
Expand All @@ -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;
}
}
}
13 changes: 13 additions & 0 deletions sentry/src/main/java/io/sentry/config/PropertiesProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
19 changes: 19 additions & 0 deletions sentry/src/main/java/io/sentry/transport/AuthenticatorWrapper.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
47 changes: 44 additions & 3 deletions sentry/src/main/java/io/sentry/transport/HttpTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -120,7 +122,8 @@ public HttpTransport(
sslSocketFactory,
hostnameVerifier,
sentryUrl,
CurrentDateProvider.getInstance());
CurrentDateProvider.getInstance(),
AuthenticatorWrapper.getInstance());
}

HttpTransport(
Expand All @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
38 changes: 38 additions & 0 deletions sentry/src/main/java/io/sentry/transport/ProxyAuthenticator.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
51 changes: 51 additions & 0 deletions sentry/src/test/java/io/sentry/SentryOptionsTest.kt
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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()
}
}
}
Loading

0 comments on commit 94691a2

Please sign in to comment.