JMXReporter may create invalid ObjectNames (#2031)

JMX managed ObjectNames cannot contain properties with values that are unquoted if the aforementioned values contain any of comma, equals, colon or double-quote characters. This fixes the default object naming strategy used by JMX reporter. Note that construction of the `ObjectName` does not fail, but JMX clients break on deserialization. Fixes #2030 (cherry picked from commit 3ac20d3)
dropwizard · Jun 16, 2021 · a993f39 · a993f39
1 parent df27b03
commit a993f39
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 2 deletions.
diff --git a/metrics-jmx/src/main/java/com/codahale/metrics/jmx/DefaultObjectNameFactory.java b/metrics-jmx/src/main/java/com/codahale/metrics/jmx/DefaultObjectNameFactory.java
@@ -10,6 +10,7 @@
 
 public class DefaultObjectNameFactory implements ObjectNameFactory {
 
+    private static final char[] QUOTABLE_CHARS = new char[] {',', '=', ':', '"'};
     private static final Logger LOGGER = LoggerFactory.getLogger(JmxReporter.class);
 
     @Override
@@ -29,10 +30,10 @@ public ObjectName createName(String type, String domain, String name) {
             if (objectName.isDomainPattern()) {
                 domain = ObjectName.quote(domain);
             }
-            if (objectName.isPropertyValuePattern("name")) {
+            if (objectName.isPropertyValuePattern("name") || shouldQuote(objectName.getKeyProperty("name"))) {
                 properties.put("name", ObjectName.quote(name));
             }
-            if (objectName.isPropertyValuePattern("type")) {
+            if (objectName.isPropertyValuePattern("type") || shouldQuote(objectName.getKeyProperty("type"))) {
                 properties.put("type", ObjectName.quote(type));
             }
             objectName = new ObjectName(domain, properties);
@@ -48,4 +49,21 @@ public ObjectName createName(String type, String domain, String name) {
         }
     }
 
+    /**
+     * Determines whether the value requires quoting.
+     * According to the {@link ObjectName} documentation, values can be quoted or unquoted. Unquoted
+     * values may not contain any of the characters comma, equals, colon, or quote.
+     *
+     * @param value a value to test
+     * @return true when it requires quoting, false otherwise
+     */
+    private boolean shouldQuote(final String value) {
+        for (char quotableChar : QUOTABLE_CHARS) {
+            if (value.indexOf(quotableChar) != -1) {
+                return true;
+            }
+        }
+        return false;
+    }
+
 }
diff --git a/metrics-jmx/src/test/java/com/codahale/metrics/jmx/DefaultObjectNameFactoryTest.java b/metrics-jmx/src/test/java/com/codahale/metrics/jmx/DefaultObjectNameFactoryTest.java
@@ -5,6 +5,7 @@
 import javax.management.ObjectName;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 public class DefaultObjectNameFactoryTest {
 
@@ -21,4 +22,12 @@ public void createsObjectNameWithNameAsKeyPropertyName() {
         ObjectName on = f.createName("type", "com.domain", "something.with.dots");
         assertThat(on.getKeyProperty("name")).isEqualTo("something.with.dots");
     }
+
+    @Test
+    public void createsObjectNameWithNameWithDisallowedUnquotedCharacters() {
+        DefaultObjectNameFactory f = new DefaultObjectNameFactory();
+        ObjectName on = f.createName("type", "com.domain", "something.with.quotes(\"ABcd\")");
+        assertThatCode(() -> new ObjectName(on.toString())).doesNotThrowAnyException();
+        assertThat(on.getKeyProperty("name")).isEqualTo("\"something.with.quotes(\\\"ABcd\\\")\"");
+    }
 }