Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve HTTP Proxy parameters from the external configuration. #1035

Merged
merged 4 commits into from
Nov 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

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;
}
}
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
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