Fix NPE in ApacheHttpClientApiAdapter#getHostName

CHANGELOG.asciidoc

@@ -31,6 +31,10 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
 
 === Unreleased
 
+[float]
+===== Bug fixes
+* Fixed NPE in ApacheHttpClientApiAdapter#getHostName - {pull}3479[#3479]
+
 [[release-notes-1.x]]
 === Java Agent version 1.x
 

apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAdvice.java

@@ -27,6 +27,7 @@
 import co.elastic.apm.agent.tracer.dispatch.TextHeaderGetter;
 import co.elastic.apm.agent.tracer.dispatch.TextHeaderSetter;
 
+import javax.annotation.Nullable;
 import java.net.URISyntaxException;
 
 public abstract class AbstractApacheHttpClientAdvice {
@@ -36,12 +37,12 @@ public abstract class AbstractApacheHttpClientAdvice {
             TextHeaderGetter<REQUEST>> Object startSpan(final Tracer tracer,
                                                         final ApacheHttpClientApiAdapter<REQUEST, WRAPPER, HTTPHOST, RESPONSE> adapter,
                                                         final WRAPPER request,
-                                                        final HTTPHOST httpHost,
+                                                        @Nullable final HTTPHOST httpHost,
                                                         final HeaderAccessor headerAccessor) throws URISyntaxException {
         ElasticContext<?> elasticContext = tracer.currentContext();
         Span<?> span = null;
         if (elasticContext.getSpan() != null) {
-            span = HttpClientHelper.startHttpClientSpan(elasticContext, adapter.getMethod(request), adapter.getUri(request), adapter.getHostName(httpHost));
+            span = HttpClientHelper.startHttpClientSpan(elasticContext, adapter.getMethod(request), adapter.getUri(request), adapter.getHostName(httpHost, request));
             if (span != null) {
                 span.activate();
             }

apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/ApacheHttpClientApiAdapter.java

@@ -19,6 +19,7 @@
 package co.elastic.apm.agent.httpclient.common;
 
 
+import javax.annotation.Nullable;
 import java.net.URI;
 import java.net.URISyntaxException;
 
@@ -27,7 +28,8 @@ public interface ApacheHttpClientApiAdapter<REQUEST, WRAPPER extends REQUEST, HT
 
     URI getUri(WRAPPER request) throws URISyntaxException;
 
-    CharSequence getHostName(HTTPHOST httpHost);
+    @Nullable
+    CharSequence getHostName(@Nullable HTTPHOST httpHost, WRAPPER request);
 
     int getResponseCode(RESPONSE response);
 

apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java

@@ -50,7 +50,7 @@ public URI getUri(HttpRequestWrapper request) {
     }
 
     @Override
-    public CharSequence getHostName(HttpHost httpHost) {
+    public CharSequence getHostName(HttpHost httpHost, HttpRequestWrapper request) {
         return httpHost.getHostName();
     }
 

apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/ApacheHttpClient5Instrumentation.java

@@ -48,7 +48,7 @@ public static class ApacheHttpClient5Advice extends AbstractApacheHttpClientAdvi
 
         @Nullable
         @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
-        public static Object onBeforeExecute(@Advice.Argument(0) HttpHost httpHost,
+        public static Object onBeforeExecute(@Advice.Argument(0) @Nullable HttpHost httpHost,
                                              @Advice.Argument(1) ClassicHttpRequest request) throws URISyntaxException {
             return startSpan(tracer, adapter, request, httpHost, RequestHeaderAccessor.INSTANCE);
         }

apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java

@@ -19,18 +19,25 @@
 package co.elastic.apm.agent.httpclient.v5.helper;
 
 import co.elastic.apm.agent.httpclient.common.ApacheHttpClientApiAdapter;
+import co.elastic.apm.agent.sdk.logging.Logger;
+import co.elastic.apm.agent.sdk.logging.LoggerFactory;
 import org.apache.hc.client5.http.CircularRedirectException;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
+import org.apache.hc.client5.http.routing.RoutingSupport;
 import org.apache.hc.core5.http.ClassicHttpRequest;
+import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequest;
 
+import javax.annotation.Nullable;
 import java.net.URI;
 import java.net.URISyntaxException;
 
 public class ApacheHttpClient5ApiAdapter implements ApacheHttpClientApiAdapter<HttpRequest, ClassicHttpRequest, HttpHost, CloseableHttpResponse> {
     private static final ApacheHttpClient5ApiAdapter INSTANCE = new ApacheHttpClient5ApiAdapter();
 
+    private static final Logger logger = LoggerFactory.getLogger(ApacheHttpClient5ApiAdapter.class);
+
     private ApacheHttpClient5ApiAdapter() {
     }
 
@@ -49,8 +56,19 @@ public URI getUri(ClassicHttpRequest request) throws URISyntaxException {
     }
 
     @Override
-    public CharSequence getHostName(HttpHost httpHost) {
-        return httpHost.getHostName();
+    public CharSequence getHostName(@Nullable HttpHost httpHost, ClassicHttpRequest request) {
+        if (httpHost != null) {
+            return httpHost.getHostName();
+        }
+
+        try {
+            return RoutingSupport.determineHost(request)
+                .getHostName();
+        } catch (HttpException e) {
+            logger.error("Exception while determining HostName", e);
+
+            return null;
+        }
     }
 
     @Override

apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/test/java/co/elastic/apm/agent/httpclient/v5/ApacheHttpClientExecuteOpenInstrumentationTest.java

@@ -0,0 +1,78 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package co.elastic.apm.agent.httpclient.v5;
+
+
+import co.elastic.apm.agent.httpclient.AbstractHttpClientInstrumentationTest;
+import org.apache.hc.client5.http.ClientProtocolException;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hc.client5.http.impl.classic.HttpClients;
+import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpEntity;
+import org.apache.hc.core5.http.io.HttpClientResponseHandler;
+import org.apache.hc.core5.http.io.entity.EntityUtils;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+import java.io.IOException;
+
+public class ApacheHttpClientExecuteOpenInstrumentationTest extends AbstractHttpClientInstrumentationTest {
+
+    private static CloseableHttpClient client;
+
+    @BeforeClass
+    public static void setUp() {
+        client = HttpClients.createDefault();
+    }
+
+    @AfterClass
+    public static void close() throws IOException {
+        client.close();
+    }
+
+    /**
+     * RFC 7230: treat presence of userinfo in authority component in request URI as an HTTP protocol violation.
+     *
+     * Uses {@link org.apache.hc.core5.http.message.BasicHttpRequest#setUri} to fill {@link org.apache.hc.core5.net.URIAuthority}
+     *
+     * Assertions on authority in {@link org.apache.hc.client5.http.impl.classic.ProtocolExec#execute}
+     */
+    @Override
+    public boolean isTestHttpCallWithUserInfoEnabled() {
+        return false;
+    }
+
+    @Override
+    protected void performGet(String path) throws Exception {
+        HttpClientResponseHandler<String> responseHandler = response -> {
+            int status = response.getCode();
+            if (status >= 200 && status < 300) {
+                HttpEntity entity = response.getEntity();
+                String res = entity != null ? EntityUtils.toString(entity) : null;
+                return res;
+            } else {
+                throw new ClientProtocolException("Unexpected response status: " + status);
+            }
+        };
+
+        ClassicHttpResponse response = client.executeOpen(null, new HttpGet(path), null);
+        responseHandler.handleResponse(response);
+    }
+}