JMX: Quote object name values if not identifiers

https://www.oracle.com/technetwork/java/javase/tech/best-practices-jsp-136021.html

The set of characters in a value is also limited. If special characters may
occur, it is recommended that the value be quoted, using ObjectName.quote. If
the value for a given key is sometimes quoted, then it should always be quoted.
By default, if a value is a string (rather than a number, say), then it should
be quoted unless you are sure that it will never contain special characters.

Practically, some special characters are allowed, but we will standardize on
allowed characters in java identifiers.

Author: garyrussell

spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java

@@ -28,6 +28,7 @@
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
+import javax.lang.model.SourceVersion;
 import javax.management.DynamicMBean;
 import javax.management.JMException;
 import javax.management.ObjectName;
@@ -745,28 +746,43 @@ private Object extractTarget(Object bean) {
 	}
 
 	private String getChannelBeanKey(String channel) {
-		String name = "" + channel;
-		if (name.startsWith("org.springframework.integration")) {
-			name = name + ",source=anonymous";
+		String extra = "";
+		if (channel.startsWith("org.springframework.integration")) {
+			extra = ",source=anonymous";
 		}
-		return String.format(this.domain + ":type=MessageChannel,name=%s" + getStaticNames(), name);
+		return String.format(this.domain + ":type=MessageChannel,name=%s%s" + getStaticNames(),
+				quoteIfNecessary(channel), extra);
 	}
 
 	private String getHandlerBeanKey(MessageHandlerMetrics handler) {
 		// This ordering of keys seems to work with default settings of JConsole
 		return String.format(this.domain + ":type=MessageHandler,name=%s,bean=%s" + getStaticNames(),
-				handler.getManagedName(), handler.getManagedType());
+				quoteIfNecessary(handler.getManagedName()), quoteIfNecessary(handler.getManagedType()));
 	}
 
 	private String getSourceBeanKey(MessageSourceMetrics source) {
 		// This ordering of keys seems to work with default settings of JConsole
 		return String.format(this.domain + ":type=MessageSource,name=%s,bean=%s" + getStaticNames(),
-				source.getManagedName(), source.getManagedType());
+				quoteIfNecessary(source.getManagedName()), quoteIfNecessary(source.getManagedType()));
 	}
 
 	private String getEndpointBeanKey(AbstractEndpoint endpoint, String name, String source) {
 		// This ordering of keys seems to work with default settings of JConsole
-		return String.format(this.domain + ":type=ManagedEndpoint,name=%s,bean=%s" + getStaticNames(), name, source);
+		return String.format(this.domain + ":type=ManagedEndpoint,name=%s,bean=%s" + getStaticNames(),
+				quoteIfNecessary(name), source);
+	}
+
+	/*
+	 * https://www.oracle.com/technetwork/java/javase/tech/best-practices-jsp-136021.html
+	 *
+	 * The set of characters in a value is also limited. If special characters may
+	 * occur, it is recommended that the value be quoted, using ObjectName.quote. If
+	 * the value for a given key is sometimes quoted, then it should always be quoted.
+	 * By default, if a value is a string (rather than a number, say), then it should
+	 * be quoted unless you are sure that it will never contain special characters.
+	 */
+	private String quoteIfNecessary(String name) {
+		return SourceVersion.isName(name) ? name : ObjectName.quote(name);
 	}
 
 	private String getStaticNames() {

spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/NotificationPublishingChannelAdapterParserTests-context.xml

@@ -23,6 +23,8 @@
 
 	<si:channel id="channel"/>
 
+	<si:channel id="org.springframework.integration.test.anon"/>
+
 	<jmx:notification-publishing-channel-adapter
 			id="adapter" channel="channel"
 			object-name="test.publisher:name=publisher"

spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/NotificationPublishingChannelAdapterParserTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -139,8 +139,12 @@ public void publishStringMessageWithinChain() throws Exception {
 		assertEquals("test.type", notification.getType());
 		assertNull(notification.getUserData());
 		Set<ObjectName> names = server
-				.queryNames(new ObjectName("*:type=MessageHandler," + "name=chainWithJmxNotificationPublishing$child."
-						+ "jmx-notification-publishing-channel-adapter-within-chain,*"), null);
+				.queryNames(new ObjectName("*:type=MessageHandler," + "name=\"chainWithJmxNotificationPublishing$child."
+						+ "jmx-notification-publishing-channel-adapter-within-chain\",*"), null);
+		assertEquals(1, names.size());
+		names = server
+				.queryNames(new ObjectName("*:type=MessageChannel,"
+						+ "name=org.springframework.integration.test.anon,source=anonymous,*"), null);
 		assertEquals(1, names.size());
 	}
 

spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MessagingGatewaySupportRegistrationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2016 the original author or authors.
+ * Copyright 2015-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -60,7 +60,7 @@ public void testHandlerMBeanRegistration() throws Exception {
 		names = this.server.queryNames(new ObjectName("org.springframework.integration:*,type=MessageSource,name=foo"),
 				null);
 		assertEquals(1, names.size());
-		names = this.server.queryNames(new ObjectName("org.springframework.integration:*,name=foo#2"), null);
+		names = this.server.queryNames(new ObjectName("org.springframework.integration:*,name=\"foo#2\""), null);
 		assertEquals(1, names.size());
 	}
 

src/reference/asciidoc/jmx.adoc

@@ -332,6 +332,8 @@ The exporter propagates the `default-domain` to that object to let it generate a
 If your custom naming strategy is a `MetadataNamingStrategy` (or a subclass of it), the exporter does not propagate the `default-domain`.
 You must configure it on your strategy bean.
 
+Starting with version 5.1; any bean names (represented by the `name` key in the object name) will be quoted if they contain any characters that are not allowed in a Java identifier (or period `.`).
+
 [[jmx-42-improvements]]
 ===== JMX Improvements
 

src/reference/asciidoc/whats-new.adoc

@@ -182,3 +182,11 @@ See <<jms>> for more information.
 The `statusCodeExpression` (and `Function`) is now supplied with the `RequestEntity<?>` as a root object for evaluation context, so request headers, method, URI and body are available for target status code calculation.
 
 See <<http>> and <<webflux>> for more information.
+
+[[x51.-jmx]]
+=== JMX Changes
+
+Object name key values are now quoted if they contain any characters other than those allowed in a Java identifier (or period `.`).
+e.g. `org.springframework.integration:type=MessageChannel,name="input:foo.myGroup.errors"`.
+This has the side effect that previously "allowed" names, with such characters, will now be quoted.
+e.g. `org.springframework.integration:type=MessageChannel,name="input#foo.myGroup.errors"`.