From c111e1be6a0fd82d428dbe026d99917e0759d83c Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 9 Oct 2018 14:56:31 -0400 Subject: [PATCH] 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. In 5.0.x, this is enabled using `spring.integration.properties`. --- .../monitor/IntegrationMBeanExporter.java | 40 +++++++++++++++---- ...shingChannelAdapterParserTests-context.xml | 2 + ...onPublishingChannelAdapterParserTests.java | 10 +++-- ...sagingGatewaySupportRegistrationTests.java | 4 +- .../META-INF/spring.integration.properties | 1 + src/reference/asciidoc/jmx.adoc | 4 ++ 6 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 spring-integration-jmx/src/test/resources/META-INF/spring.integration.properties diff --git a/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java b/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java index dbc71961d2..7578a71466 100644 --- a/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java +++ b/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java @@ -28,6 +28,7 @@ import java.util.concurrent.atomic.AtomicBoolean; 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; @@ -157,6 +158,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati private final AtomicBoolean shuttingDown = new AtomicBoolean(); + private boolean quoteNames; + public IntegrationMBeanExporter() { super(); @@ -216,6 +219,14 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati @Override public void afterSingletonsInstantiated() { + if (this.applicationContext.containsBean(IntegrationContextUtils.INTEGRATION_GLOBAL_PROPERTIES_BEAN_NAME)) { + Properties props = this.applicationContext + .getBean(IntegrationContextUtils.INTEGRATION_GLOBAL_PROPERTIES_BEAN_NAME, Properties.class); + String prop = props.getProperty("spring.integration.jmx.quote.names"); + if (prop != null) { + this.quoteNames = Boolean.parseBoolean(prop); + } + } Map messageHandlers = this.applicationContext.getBeansOfType(MessageHandlerMetrics.class); for (Entry entry : messageHandlers.entrySet()) { @@ -744,28 +755,43 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati } 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 : this.quoteNames ? ObjectName.quote(name) : name; } private String getStaticNames() { diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/NotificationPublishingChannelAdapterParserTests-context.xml b/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/NotificationPublishingChannelAdapterParserTests-context.xml index bbf9a99c1e..fd148ede37 100644 --- a/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/NotificationPublishingChannelAdapterParserTests-context.xml +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/NotificationPublishingChannelAdapterParserTests-context.xml @@ -23,6 +23,8 @@ + + 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()); } diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MessagingGatewaySupportRegistrationTests.java b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MessagingGatewaySupportRegistrationTests.java index 870c931af3..83deedfb68 100644 --- a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MessagingGatewaySupportRegistrationTests.java +++ b/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 class MessagingGatewaySupportRegistrationTests { 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()); } diff --git a/spring-integration-jmx/src/test/resources/META-INF/spring.integration.properties b/spring-integration-jmx/src/test/resources/META-INF/spring.integration.properties new file mode 100644 index 0000000000..f6781128c6 --- /dev/null +++ b/spring-integration-jmx/src/test/resources/META-INF/spring.integration.properties @@ -0,0 +1 @@ +spring.integration.jmx.quote.names=true diff --git a/src/reference/asciidoc/jmx.adoc b/src/reference/asciidoc/jmx.adoc index 24e2467b79..ec293de8fc 100644 --- a/src/reference/asciidoc/jmx.adoc +++ b/src/reference/asciidoc/jmx.adoc @@ -301,6 +301,10 @@ NOTE: The default naming strategy is a http://docs.spring.io/spring/docs/current The exporter propagates the `default-domain` to that object to allow it to generate a fallback object name if parsing of the bean key fails. If your custom naming strategy is a `MetadataNamingStrategy` (or subclass), the exporter will *not* propagate the `default-domain`; you will need to configure it on your strategy bean. +Starting with version 5.0.9; any bean names (represented by the `name` key in the object name) can be quoted if they contain any characters that are not allowed in a Java identifier (or period `.`). +This requires setting `spring.integration.jmx.quote.names=true` in a `META-INF/spring.integration.properties` file on the class path. +In 5.1 it will not be configurable. + [[jmx-42-improvements]] ===== JMX Improvements