Skip to content
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.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ paths. The BCI warmup is on by default and may be disabled through the internal
** APM headers conversion issue within dubbo transaction
* Fix External plugins automatic setting of span outcome - {pull}2376[#2376]
* Avoid early initialization of JMX on Weblogic - {pull}2420[#2420]
* Ensure agent always uses JDK HTTP client - {pull}2429[#2429]

[[release-notes-1.x]]
=== Java Agent version 1.x
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import co.elastic.apm.agent.configuration.CoreConfiguration;
import co.elastic.apm.agent.configuration.ServerlessConfiguration;
import co.elastic.apm.agent.report.HttpUtils;
import co.elastic.apm.agent.util.ExecutorUtils;
import co.elastic.apm.agent.util.UrlConnectionUtils;
import com.dslplatform.json.DslJson;
Expand Down Expand Up @@ -415,7 +416,7 @@ private static Map<String, Object> deserialize(String input) throws IOException
}

private static String executeRequest(String url, String method, @Nullable Map<String, String> headers, int queryTimeoutMs) throws IOException {
HttpURLConnection urlConnection = (HttpURLConnection) UrlConnectionUtils.openUrlConnectionThreadSafely(new URL(url));
HttpURLConnection urlConnection = (HttpURLConnection) UrlConnectionUtils.openUrlConnectionThreadSafely(HttpUtils.withDefaultHandler(new URL(url)));
if (headers != null) {
for (String header : headers.keySet()) {
urlConnection.setRequestProperty(header, headers.get(header));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@

import co.elastic.apm.agent.configuration.CoreConfiguration;
import co.elastic.apm.agent.report.ssl.SslUtils;
import co.elastic.apm.agent.sdk.logging.Logger;
import co.elastic.apm.agent.sdk.logging.LoggerFactory;
import co.elastic.apm.agent.util.UrlConnectionUtils;
import co.elastic.apm.agent.util.Version;
import co.elastic.apm.agent.util.VersionUtils;
import co.elastic.apm.agent.sdk.logging.Logger;
import co.elastic.apm.agent.sdk.logging.LoggerFactory;
import org.stagemonitor.configuration.ConfigurationOption;

import javax.annotation.Nonnull;
Expand Down Expand Up @@ -112,7 +112,11 @@ public void onChange(ConfigurationOption<?> configurationOption, List<URL> oldVa
}

private void setServerUrls(List<URL> serverUrls) {
this.serverUrls = serverUrls;
List<URL> newServersUrls = new ArrayList<>();
for (URL url : serverUrls) {
newServersUrls.add(HttpUtils.withDefaultHandler(url));
}
this.serverUrls = newServersUrls;
this.apmServerVersion = healthChecker.checkHealthAndGetMinVersion();
this.errorCount.set(0);
}
Expand Down Expand Up @@ -410,6 +414,7 @@ private static String getUserAgent(CoreConfiguration coreConfiguration) {
/**
* Escapes the provided string from characters that are disallowed within HTTP header comments.
* See spec- https://httpwg.org/specs/rfc7230.html#field.components
*
* @param headerFieldComment HTTP header comment value to be escaped
* @return the escaped header comment
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,25 @@
*/
package co.elastic.apm.agent.report;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.stagemonitor.util.IOUtils;

import javax.annotation.Nullable;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.lang.reflect.InvocationTargetException;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLStreamHandler;

public class HttpUtils {

private static final Logger log = LoggerFactory.getLogger(HttpUtils.class);

private HttpUtils() {
}

Expand Down Expand Up @@ -65,4 +73,40 @@ public static void consumeAndClose(@Nullable HttpURLConnection connection) {
}
}
}

/**
* Rebuilds the provided URL with the default handler, as it may have been overriden by an application server at
* runtime and might prevent the agent from properly communicate through HTTP/HTTPS.
*
* @param url URL to rewrite
* @return equivalent URL with the default JDK handler
* @throws IllegalArgumentException if protocol is not supported or unable to access default handler
*/
public static URL withDefaultHandler(URL url) {
// the default handler for URLs might be overridden by another implementation than the one shipped with the JDK
// for example, this happens on Weblogic application server and triggers classloading issues as the agent
// is unable to properly use the Weblogic classes as they aren't visible to the agent classloaders.

String protocol = url.getProtocol();
try {
return new URL(protocol, url.getHost(), url.getPort(), url.getFile(), handlerForProtocol(protocol));
} catch (MalformedURLException e) {
return url;
}
}

@Nullable
private static URLStreamHandler handlerForProtocol(String protocol) {
try {
Class<?> handlerClass = Class.forName(String.format("sun.net.www.protocol.%s.Handler", protocol));
return (URLStreamHandler) handlerClass.getDeclaredConstructor().newInstance();
} catch (ClassNotFoundException | NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException e) {
if (log.isDebugEnabled()) {
log.debug("unable to create default HTTP stream handler for protocol '{}", protocol, e);
} else {
log.warn("unable to create default HTTP stream handler for protocol '{}", protocol);
}
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,40 @@
*/
package co.elastic.apm.agent.report;

import co.elastic.apm.agent.sdk.DynamicTransformer;
import co.elastic.apm.agent.sdk.ElasticApmInstrumentation;
import co.elastic.apm.agent.util.CustomEnvVariables;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import javax.annotation.Nullable;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;

import java.net.URL;
import java.net.URLConnection;
import java.net.URLStreamHandler;
import java.net.URLStreamHandlerFactory;
import java.net.UnknownHostException;
import java.util.Collection;
import java.util.Collections;
import java.util.Hashtable;
import java.util.concurrent.atomic.AtomicBoolean;

import static net.bytebuddy.matcher.ElementMatchers.isStatic;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

class HttpUtilsTest {
class HttpUtilsTest extends CustomEnvVariables {

@Test
void consumeAndCloseIgnoresNullConnection() {
Expand Down Expand Up @@ -82,4 +104,115 @@ private static InputStream mockEmptyInputStream() throws IOException {
return stream;
}

private static final String FACTORY_MSG = "fake handler can't open connections";
private static final AtomicBoolean factoryActive = new AtomicBoolean(false);

static {
// This method to set the global factory is designed to be called once. It is not possible to
// un-register the factory after registration
URL.setURLStreamHandlerFactory(new URLStreamHandlerFactory() {

@Nullable
@Override
public URLStreamHandler createURLStreamHandler(String p) {
if (!factoryActive.get()) {
// The factory will return null when it does not support a given protocol
return null;
}

// any non-null value returned here will be registered globally
// In order to reset the default behavior calling URL.setURLStreamHandlerFactory(null) is required.
return new URLStreamHandler() {
@Override
protected URLConnection openConnection(URL u) throws IOException {
throw new IllegalStateException(FACTORY_MSG);
}
};
}
});
}

@ParameterizedTest
@ValueSource(strings = {"http", "https"})
void nonDefaultUrlHandler(String protocol) throws IOException {

// the handler is set in the URL constructor, thus we have to rebuild it
// every time we expect a different handler.
String url = protocol + "://not.found:9999";

DynamicTransformer.ensureInstrumented(URL.class, Collections.singleton(TestUrlInstrumentation.class));

try {
// test default behavior
checkDefaultHandler(new URL(url));

factoryActive.set(true);

// required to remove registered handlers
URL.setURLStreamHandlerFactory(null);

// overridden default handler does not allow opening a connection
assertThatThrownBy(() -> new URL(url).openConnection())
.hasMessage(FACTORY_MSG);

checkDefaultHandler(HttpUtils.withDefaultHandler(new URL(url)));
} finally {
factoryActive.set(false);

// required to remove registered handlers
URL.setURLStreamHandlerFactory(null);

// should not impact non-fake URLs
checkDefaultHandler(new URL(url));
}
}


private void checkDefaultHandler(URL url) {
// unknown host exception is expected here
assertThatThrownBy(() -> url.openConnection().getInputStream())
.isInstanceOf(UnknownHostException.class);
}

/**
* Instrumentation of {@link URL#setURLStreamHandlerFactory(URLStreamHandlerFactory)} to allow resetting handlers
* by calling {@code URL.setURLStreamHandlerFactory(null);}.
*/
public static final class TestUrlInstrumentation extends ElasticApmInstrumentation {

@Override
public ElementMatcher<? super TypeDescription> getTypeMatcher() {
return named("java.net.URL");
}

@Override
public ElementMatcher<? super MethodDescription> getMethodMatcher() {
return isStatic().and(named("setURLStreamHandlerFactory"));
}

@Override
public Collection<String> getInstrumentationGroupNames() {
return Collections.singleton("test-url");
}

@Override
public String getAdviceClassName() {
return "co.elastic.apm.agent.report.HttpUtilsTest$TestUrlInstrumentation$AdviceClass";
}

public static final class AdviceClass {

@Advice.OnMethodEnter(suppress = Throwable.class, inline = false, skipOn = Advice.OnNonDefaultValue.class)
public static boolean onEnter(@Advice.Argument(0) @Nullable URLStreamHandlerFactory factory,
@Advice.FieldValue("handlers") Hashtable<?, ?> handlers) {

if (factory == null) {
handlers.clear();
return true;
}
return false;
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,10 @@

import co.elastic.apm.agent.AbstractInstrumentationTest;
import co.elastic.apm.agent.testinstr.SystemSingleEnvVariablesInstrumentation;
import org.junit.jupiter.api.Test;

import java.util.Map;
import java.util.concurrent.Callable;

import static org.assertj.core.api.Assertions.assertThat;

public abstract class CustomEnvVariables extends AbstractInstrumentationTest {

protected void runWithCustomEnvVariables(Map<String, String> customEnvVariables, Runnable runnable) {
Expand Down