diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 12ad7b3f91..502568af2b 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -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 diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/CloudMetadataProvider.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/CloudMetadataProvider.java index 4d462cd935..21c9155c56 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/CloudMetadataProvider.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/CloudMetadataProvider.java @@ -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; @@ -415,7 +416,7 @@ private static Map deserialize(String input) throws IOException } private static String executeRequest(String url, String method, @Nullable Map 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)); diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java index 2679d269d1..268b72fad3 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java @@ -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; @@ -112,7 +112,11 @@ public void onChange(ConfigurationOption configurationOption, List oldVa } private void setServerUrls(List serverUrls) { - this.serverUrls = serverUrls; + List newServersUrls = new ArrayList<>(); + for (URL url : serverUrls) { + newServersUrls.add(HttpUtils.withDefaultHandler(url)); + } + this.serverUrls = newServersUrls; this.apmServerVersion = healthChecker.checkHealthAndGetMinVersion(); this.errorCount.set(0); } @@ -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 */ diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/HttpUtils.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/HttpUtils.java index b34aeb24c3..dfd581ff4d 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/HttpUtils.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/HttpUtils.java @@ -18,6 +18,8 @@ */ package co.elastic.apm.agent.report; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.stagemonitor.util.IOUtils; import javax.annotation.Nullable; @@ -25,10 +27,16 @@ 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() { } @@ -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; + } + } } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/HttpUtilsTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/HttpUtilsTest.java index d5fcb3c0aa..8dda5b3874 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/HttpUtilsTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/HttpUtilsTest.java @@ -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() { @@ -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 getTypeMatcher() { + return named("java.net.URL"); + } + + @Override + public ElementMatcher getMethodMatcher() { + return isStatic().and(named("setURLStreamHandlerFactory")); + } + + @Override + public Collection 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; + } + } + } + } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/util/CustomEnvVariables.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/util/CustomEnvVariables.java index 9930badee6..c25c41e06d 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/util/CustomEnvVariables.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/util/CustomEnvVariables.java @@ -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 customEnvVariables, Runnable runnable) {