From 551f03caa4c78bba5856171f0767a2adfea02819 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. --- .../monitor/IntegrationMBeanExporter.java | 30 ++++++++++++++----- ...shingChannelAdapterParserTests-context.xml | 2 ++ ...onPublishingChannelAdapterParserTests.java | 10 +++++-- ...sagingGatewaySupportRegistrationTests.java | 4 +-- src/reference/asciidoc/jmx.adoc | 2 ++ src/reference/asciidoc/whats-new.adoc | 8 +++++ 6 files changed, 44 insertions(+), 12 deletions(-) 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 d12420ad35..10b4e076d3 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; @@ -745,28 +746,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 : ObjectName.quote(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/src/reference/asciidoc/jmx.adoc b/src/reference/asciidoc/jmx.adoc index 8f6da23496..9c503a690b 100644 --- a/src/reference/asciidoc/jmx.adoc +++ b/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 diff --git a/src/reference/asciidoc/whats-new.adoc b/src/reference/asciidoc/whats-new.adoc index 07ac4d80e3..f6f5d17f49 100644 --- a/src/reference/asciidoc/whats-new.adoc +++ b/src/reference/asciidoc/whats-new.adoc @@ -182,3 +182,11 @@ See <> 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 <> and <> 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"`.