From 10d19b6a601d74e5f28fe11fb3551501abc12a2b Mon Sep 17 00:00:00 2001
From: brharrington <brharrington@gmail.com>
Date: Sun, 5 Jan 2025 15:02:17 -0600
Subject: [PATCH] aws: check for null values in custom tags (#1173)

Avoid null values in any custom tags that are passed into the
collector.
---
 .../aws/SpectatorRequestMetricCollector.java      | 15 +++++++++------
 .../aws/SpectatorRequestMetricCollectorTest.java  |  9 +++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/spectator-ext-aws/src/main/java/com/netflix/spectator/aws/SpectatorRequestMetricCollector.java b/spectator-ext-aws/src/main/java/com/netflix/spectator/aws/SpectatorRequestMetricCollector.java
index 6deeaa433..54ab844a8 100644
--- a/spectator-ext-aws/src/main/java/com/netflix/spectator/aws/SpectatorRequestMetricCollector.java
+++ b/spectator-ext-aws/src/main/java/com/netflix/spectator/aws/SpectatorRequestMetricCollector.java
@@ -143,18 +143,21 @@ public SpectatorRequestMetricCollector(Registry registry, Map<String, String> cu
     this.customTags = new HashMap<>();
     customTags.forEach((key, value) -> {
       if (ALL_DEFAULT_TAGS.contains(key)) {
-        registry.propagate(new IllegalArgumentException("Invalid custom tag " + key
-              + " - cannot override built-in tag"));
+        registry.propagate(new IllegalArgumentException(
+            "Invalid custom tag " + key + " - cannot override built-in tag"));
+      } else if (value == null) {
+        registry.propagate(new NullPointerException(
+            "Custom tag value for key " + key + " is null"));
       } else {
         this.customTags.put(key, value);
       }
     });
     Preconditions.checkNotNull(handlerContextKey, "handlerContextKey");
     this.handlerContextKey = handlerContextKey;
-    if (ALL_DEFAULT_TAGS.contains(this.handlerContextKey.getName())) {
-      registry.propagate(new IllegalArgumentException("Invalid handler context key "
-                                                      + this.handlerContextKey.getName()
-                                                      + " - cannot override built-in tag"));
+    final String keyName = this.handlerContextKey.getName();
+    if (ALL_DEFAULT_TAGS.contains(keyName)) {
+      registry.propagate(new IllegalArgumentException(
+          "Invalid handler context key " + keyName + " - cannot override built-in tag"));
     }
   }
 
diff --git a/spectator-ext-aws/src/test/java/com/netflix/spectator/aws/SpectatorRequestMetricCollectorTest.java b/spectator-ext-aws/src/test/java/com/netflix/spectator/aws/SpectatorRequestMetricCollectorTest.java
index dbba4205c..8f09f44ec 100644
--- a/spectator-ext-aws/src/test/java/com/netflix/spectator/aws/SpectatorRequestMetricCollectorTest.java
+++ b/spectator-ext-aws/src/test/java/com/netflix/spectator/aws/SpectatorRequestMetricCollectorTest.java
@@ -190,6 +190,15 @@ public void testCustomTags_overrideDefault() {
         new SpectatorRequestMetricCollector(new DefaultRegistry(Clock.SYSTEM, config), customTags));
   }
 
+  @Test
+  public void testCustomTagsNull() {
+    Map<String, String> customTags = new HashMap<>();
+    customTags.put("tagname", null);
+    collector = new SpectatorRequestMetricCollector(registry, customTags);
+    execRequest("http://monitoring", 503);
+    assertEquals(set(), valueSet("tagname"));
+  }
+
   @Test
   public void testHandlerContextKey() {
     String contextKeyName = "myContextKey";