Merge pull request quarkusio#53807 from brunobat/fix-null-pointers

Fix potential NPE and warn if trustall is being used in the exporter

Author: geoand

extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/OpenTelemetryUtil.java

@@ -48,6 +48,9 @@ public static Map<String, String> convertKeyValueListToMap(List<String> headers)
                 continue;
             }
             String[] parts = header.split("=", 2);
+            if (parts.length < 2) {
+                continue;
+            }
             String key = parts[0].trim();
             String value = parts[1].trim();
             result.put(key, value);

extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/exporter/otlp/OTelExporterRecorder.java

@@ -18,6 +18,8 @@
 import jakarta.enterprise.inject.Instance;
 import jakarta.enterprise.util.TypeLiteral;
 
+import org.jboss.logging.Logger;
+
 import io.opentelemetry.api.metrics.MeterProvider;
 import io.opentelemetry.exporter.internal.ExporterBuilderUtil;
 import io.opentelemetry.exporter.internal.grpc.GrpcExporter;
@@ -74,6 +76,7 @@
 @SuppressWarnings("deprecation")
 @Recorder
 public class OTelExporterRecorder {
+    private static final Logger LOG = Logger.getLogger(OTelExporterRecorder.class);
     public static final String BASE2EXPONENTIAL_AGGREGATION_NAME = AggregationUtil
             .aggregationName(Aggregation.base2ExponentialBucketHistogram());
 
@@ -404,6 +407,9 @@ private static Map<String, String> populateTracingExportHttpHeaders(OtlpExporter
                         continue;
                     }
                     String[] parts = header.split("=", 2);
+                    if (parts.length < 2) {
+                        continue;
+                    }
                     String key = parts[0].trim();
                     String value = parts[1].trim();
                     headersMap.put(key, value);
@@ -507,6 +513,9 @@ public Boolean get() {
                         }
                     });
             if (trustAll) {
+                LOG.warn("OpenTelemetry exporter TLS is configured with trustAll=true." +
+                        " This disables certificate and hostname verification, making the connection vulnerable to" +
+                        " MITM attacks. This should not be used in production.");
                 options.setTrustAll(true);
                 options.setVerifyHost(false);
             }

extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java

@@ -37,11 +37,14 @@ public SamplingResult shouldSample(Context parentContext, String traceId, String
 
         if (spanKind.equals(SpanKind.SERVER)) {
             // HTTP_TARGET was split into url.path and url.query
-            String query = attributes.get(URL_QUERY);
-            String target = attributes.get(URL_PATH) + (query == null ? "" : "?" + query);
+            String path = attributes.get(URL_PATH);
+            if (path != null) {
+                String query = attributes.get(URL_QUERY);
+                String target = path + (query == null ? "" : "?" + query);
 
-            if (shouldDrop(target)) {
-                return SamplingResult.drop();
+                if (shouldDrop(target)) {
+                    return SamplingResult.drop();
+                }
             }
         }
 

extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/cdi/WithSpanInterceptor.java

@@ -220,7 +220,7 @@ public String extract(final MethodRequest methodRequest) {
                     break;
                 }
             }
-            if (spanName.isEmpty()) {
+            if (spanName == null || spanName.isEmpty()) {
                 spanName = SpanNames.fromMethod(methodRequest.getMethod());
             }
             return spanName;

extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/exporter/otlp/HttpClientOptionsConsumerTest.java

@@ -7,11 +7,16 @@
 
 import java.net.URI;
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.OptionalInt;
+import java.util.logging.Handler;
+import java.util.logging.LogRecord;
+import java.util.logging.Logger;
 
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 
 import io.quarkus.opentelemetry.runtime.config.runtime.exporter.CompressionType;
 import io.quarkus.opentelemetry.runtime.config.runtime.exporter.OtlpExporterTracesConfig;
@@ -49,6 +54,46 @@ void testWithProxy() {
         assertThat(httpClientOptions.getProxyOptions().getPassword(), is("proxy-password"));
     }
 
+    @Test
+    void testTrustAllLogsWarning() {
+        Logger logger = Logger.getLogger(OTelExporterRecorder.class.getName());
+        List<LogRecord> records = new ArrayList<>();
+        Handler handler = new Handler() {
+            @Override
+            public void publish(LogRecord record) {
+                records.add(record);
+            }
+
+            @Override
+            public void flush() {
+            }
+
+            @Override
+            public void close() {
+            }
+        };
+        logger.addHandler(handler);
+        try {
+            OTelExporterRecorder.HttpClientOptionsConsumer consumer = new OTelExporterRecorder.HttpClientOptionsConsumer(
+                    createExporterConfig(false),
+                    URI.create("https://localhost:4317"),
+                    createTrustAllRegistry());
+
+            HttpClientOptions httpClientOptions = new HttpClientOptions();
+            consumer.accept(httpClientOptions);
+
+            assertThat(httpClientOptions.isTrustAll(), is(true));
+            assertThat(httpClientOptions.isVerifyHost(), is(false));
+            assertThat(records.stream()
+                    .anyMatch(r -> r.getMessage().contains("trustAll=true") &&
+                            r.getMessage().contains("This should not be used in production.") &&
+                            r.getLevel().equals(java.util.logging.Level.WARNING)),
+                    is(true));
+        } finally {
+            logger.removeHandler(handler);
+        }
+    }
+
     private OtlpExporterTracesConfig createExporterConfig(final boolean isEnabled) {
         return new OtlpExporterTracesConfig() {
             @Override
@@ -154,4 +199,14 @@ public void register(String name, TlsConfiguration configuration) {
 
         }
     }
+
+    private static TlsConfigurationRegistry createTrustAllRegistry() {
+        TlsConfiguration tlsConfig = Mockito.mock(TlsConfiguration.class);
+        Mockito.when(tlsConfig.isTrustAll()).thenReturn(true);
+
+        TlsConfigurationRegistry registry = Mockito.mock(TlsConfigurationRegistry.class);
+        Mockito.when(registry.getDefault()).thenReturn(Optional.of(tlsConfig));
+        Mockito.when(registry.get(Mockito.anyString())).thenReturn(Optional.empty());
+        return registry;
+    }
 }