Fail to register MBean with bean name containing invalid character

[Stiuil06]

Issue originally posted spring-projects/spring-integration#3051

Hi all.
I have similar(I think) issues to spring-cloud-stream/#1497 when I try run application to listening mqtt broker.
Simple project with that issue and workaround: https://github.com/Stiuil06/demo-mqtt-listener
Im trying to run this code:
https://docs.spring.io/spring-integration/docs/5.2.0.BUILD-SNAPSHOT/reference/html/mqtt.html#configuring-with-the-java-dsl
Configuring with the Java DSL

for Configuring with Java Configuration everything works.

My stacktrace

org.springframework.jmx.export.UnableToRegisterMBeanException: Unable to register MBean [mqttInbound.mqtt:inbound-channel-adapter#0] with key 'mqttInbound.mqtt:inbound-channel-adapter#0'; nested exception is javax.management.MalformedObjectNameException: Invalid character `:' in value
	at org.springframework.jmx.export.MBeanExporter.registerBeanNameOrInstance(MBeanExporter.java:625) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.jmx.export.MBeanExporter.lambda$registerBeans$2(MBeanExporter.java:551) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at java.base/java.util.HashMap.forEach(HashMap.java:1333) ~[na:na]
	at org.springframework.jmx.export.MBeanExporter.registerBeans(MBeanExporter.java:551) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.jmx.export.MBeanExporter.afterSingletonsInstantiated(MBeanExporter.java:434) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:862) ~[spring-beans-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:877) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:549) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:743) ~[spring-boot-2.1.7.RELEASE.jar:2.1.7.RELEASE]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:390) ~[spring-boot-2.1.7.RELEASE.jar:2.1.7.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:312) ~[spring-boot-2.1.7.RELEASE.jar:2.1.7.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1214) ~[spring-boot-2.1.7.RELEASE.jar:2.1.7.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1203) ~[spring-boot-2.1.7.RELEASE.jar:2.1.7.RELEASE]
	at com.firstnamelastname.grl.MiddlewareApplication.main(MiddlewareApplication.java:10) ~[classes/:na]
Caused by: javax.management.MalformedObjectNameException: Invalid character `:' in value
	at java.management/javax.management.ObjectName.parseValue(ObjectName.java:978) ~[na:na]
	at java.management/javax.management.ObjectName.checkValue(ObjectName.java:1010) ~[na:na]
	at java.management/javax.management.ObjectName.construct(ObjectName.java:729) ~[na:na]
	at java.management/javax.management.ObjectName.<init>(ObjectName.java:1450) ~[na:na]
	at java.management/javax.management.ObjectName.getInstance(ObjectName.java:1356) ~[na:na]
	at org.springframework.jmx.support.ObjectNameManager.getInstance(ObjectNameManager.java:99) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.jmx.export.naming.MetadataNamingStrategy.getObjectName(MetadataNamingStrategy.java:135) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.boot.autoconfigure.jmx.ParentAwareNamingStrategy.getObjectName(ParentAwareNamingStrategy.java:59) ~[spring-boot-autoconfigure-2.1.7.RELEASE.jar:2.1.7.RELEASE]
	at org.springframework.jmx.export.MBeanExporter.getObjectName(MBeanExporter.java:755) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.jmx.export.MBeanExporter.registerBeanInstance(MBeanExporter.java:654) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	at org.springframework.jmx.export.MBeanExporter.registerBeanNameOrInstance(MBeanExporter.java:615) ~[spring-context-5.1.9.RELEASE.jar:5.1.9.RELEASE]
	... 13 common frames omitted

My code

public class MeasurementListener {

    @Bean
    public IntegrationFlow mqttInbound() {
        return IntegrationFlows.from(
                new MqttPahoMessageDrivenChannelAdapter("tcp://localhost:1883", "/measurement/+/+"))
                .handle(m -> System.out.println(m.getPayload()))
                .get();
    }

}

My main pom

<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>com.firstnamelastname</groupId>
    <artifactId>grl</artifactId>
    <version>0.0.1-SNAPSHOT</version>
    <name>green-real-time</name>
    <description>Green Real Time</description>

    <packaging>pom</packaging>
    <modules>
        <module>grl-core</module>
        <module>grl-firmware-simulator</module>
        <module>grl-middleware</module>
    </modules>
    <parent>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-parent</artifactId>
        <version>2.1.7.RELEASE</version>
        <relativePath/> <!-- lookup parent from repository -->
    </parent>


    <properties>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
        <java.version>12</java.version>
        <maven.compiler.source>${java.version}</maven.compiler.source>
        <maven.compiler.target>${java.version}</maven.compiler.target>
    </properties>

    <dependencies>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-test</artifactId>
            <scope>test</scope>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>org.springframework.boot</groupId>
                <artifactId>spring-boot-maven-plugin</artifactId>
            </plugin>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-release-plugin</artifactId>
                <configuration>
                    <goals>install</goals>
                    <autoVersionSubmodules>true</autoVersionSubmodules>
                </configuration>
            </plugin>
        </plugins>
    </build>

</project>

pom for this module

<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <parent>
        <groupId>com.firstnamelastname</groupId>
        <artifactId>grl</artifactId>
        <version>0.0.1-SNAPSHOT</version>
    </parent>

    <artifactId>grl-middleware</artifactId>
    <version>0.0.1-SNAPSHOT</version>
    <name>green-real-time-middleware</name>
    <description>Green Real Time - Middleware</description>

    <properties>
        <spring-boot.repackage.skip>true</spring-boot.repackage.skip>
    </properties>

    <dependencies>
        <dependency>
            <groupId>com.firstnamelastname</groupId>
            <artifactId>grl-core</artifactId>
            <version>0.0.1-SNAPSHOT</version>
            <scope>compile</scope>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-data-cassandra-reactive</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.integration</groupId>
            <artifactId>spring-integration-mqtt</artifactId>
            <version>5.1.7.RELEASE</version>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>org.springframework.boot</groupId>
                <artifactId>spring-boot-maven-plugin</artifactId>
            </plugin>
        </plugins>
    </build>

</project>

[artembilan]

The point of this issue that we need somehow quote non-standard names for MBeans.
We wonder if that can happen on the core level.
See my comment over there: spring-projects/spring-integration#3051 (comment)
Thanks

[artembilan]

We have one more community request for the fix: spring-projects/spring-integration#3051 (comment).

Any chances that quoting will make it into the core?
It's really not only Spring Integration problem to provide non-standard JMX bean names to expose.

Thanks for understanding.

[artembilan]

See the same problem reported in other our project: spring-projects/spring-integration-aws#197

[chrylis]

A plus-one for this ticket: The out-of-the-box experience with Spring Integration in Boot is to blow up in a surprising manner (I didn't even realize that JMX MBeans were getting added in the first place, since I haven't included Actuator)! This is on a bare-bones Initializr project with tutorial inclusions of spring-boot-starter-integration and spring-integration-aws.

[snicoll]

@artembilan I don't understand (yet) why you think a fix in framework is required. I've tried to reproduce the problem to see if this was an automated export by bean name or something but that doesn't seem to be the case. As for Quoting, this is used for an element of the object name, not the object name itself.

mqttInbound.mqtt:inbound-channel-adapter#0 is trying to register an MBean on the mqttInbound.mqtt domain with a property named inbound-channel-adapter#0 with no value. I am sure that's not what's expected. At the very least, this channel adapter must be registered against a predicatable domain. Besides, the best practices states that an ObjectName should have a type parameter.

With all that said, even if we tried to clean the key, we'd change it behind the scenes which doesn't sound a good idea. Can we please get some clarification how we ended up in such a situation?

[artembilan]

Hi @snicoll !

Thank you for feedback.

So, in Spring Integration we have our own IntegrationMBeanExporter implementation and the logic is there like this:

	private String getHandlerBeanKey(IntegrationManagement monitor) {
		// This ordering of keys seems to work with default settings of JConsole
		return String.format(this.domain + ":type=MessageHandler,name=%s,bean=%s" + getStaticNames(),
				quoteIfNecessary(monitor.getManagedName()), quoteIfNecessary(monitor.getManagedType()));
	}

As you see, we do have all the required parts of an Object Name for MBean and we really quote some elements only.
However looking into MBeanExporter code, I see this:

this.beans.forEach((beanName, instance) -> registerBeanNameOrInstance(instance, beanName));
...
return this.namingStrategy.getObjectName(bean, beanKey);

Where that namingStrategy is a MetadataNamingStrategy (according to stacktrace) and that does this:

		else {
			Assert.state(beanKey != null, "No ManagedResource attribute and no bean key specified");
			try {
				return ObjectNameManager.getInstance(beanKey);
			}
			catch (MalformedObjectNameException ex) {
				String domain = this.defaultDomain;
				if (domain == null) {
					domain = ClassUtils.getPackageName(managedClass);
				}
				Hashtable<String, String> properties = new Hashtable<>();
				properties.put("type", ClassUtils.getShortName(managedClass));
				properties.put("name", beanKey);
				return ObjectNameManager.getInstance(domain, properties);
			}
		}

However it fails again because that name property is not quoted.
That's one side of the puzzle.

Another one from where that mqttInbound.mqtt:inbound-channel-adapter#0 appeared as a bean name is Spring Integration logic where we auto-generate it from component type if end-user doesn't provide as specific one.
In the MQTT case, mentioned in this issue, we have:

	public String getComponentType() {
		return "mqtt:inbound-channel-adapter";
	}

We probably can rework the logic in Spring Integration in that bean name generation part, but why just not follow quoting recommendations from JMX spec where we encounter invalid characters?
No one said that end-user may not end up with some bean name which would have those characters.

In the end, only what I'm asking as a fix here is this for the MetadataNamingStrategy:

properties.put("name", quoteIfNecessary(beanKey));
...
/*
 * 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);
}

Feel free to give more feedback!

Thanks

[snicoll]

Thanks @artembilan. Can I get something I can run myself that reproduces this? I think it will be the most efficient as I am not sure I understood all the moving parts. Thanks!

[artembilan]

Right. So, here is pretty simple unit test against just org.springframework.boot:spring-boot-starter dependency:

@SpringBootTest(properties = {"spring.jmx.enabled=true", "spring.jmx.default-domain=spr-23608"})
class Spr23608ApplicationTests {

	@Autowired
	MBeanServer mbeanServer;

	@Test
	void invalidMBeanNameCausesRegistrationFailure() throws MalformedObjectNameException {
		assertThat(this.mbeanServer.queryNames(ObjectName.getInstance("spr-23608:*"), null)).hasSize(1);
	}

	@SpringBootApplication
	static class Spr23608Application {

		@Bean(name = "some,jmx:invalid=bean\"name")
		MyManagedBean myManagedBean() {
			return new MyManagedBean();
		}

	}

	@ManagedResource
	public static class MyManagedBean {

	}

}

The stack trace is similar to what was reported before:

org.springframework.jmx.export.UnableToRegisterMBeanException: Unable to register MBean [org.springframework.spr23608.Spr23608ApplicationTests$MyManagedBean@20b9d5d5] with key 'some,jmx:invalid=bean"name'
	at org.springframework.jmx.export.MBeanExporter.registerBeanNameOrInstance(MBeanExporter.java:641) ~[spring-context-6.1.1.jar:6.1.1]
	at org.springframework.jmx.export.MBeanExporter.lambda$registerBeans$2(MBeanExporter.java:568) ~[spring-context-6.1.1.jar:6.1.1]
	at java.base/java.util.HashMap.forEach(HashMap.java:1421) ~[na:na]
	at org.springframework.jmx.export.MBeanExporter.registerBeans(MBeanExporter.java:568) ~[spring-context-6.1.1.jar:6.1.1]
	at org.springframework.jmx.export.MBeanExporter.afterSingletonsInstantiated(MBeanExporter.java:451) ~[spring-context-6.1.1.jar:6.1.1]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:984) ~[spring-beans-6.1.1.jar:6.1.1]
	...
Caused by: javax.management.MalformedObjectNameException: Invalid character in value: `,'
	at java.management/javax.management.ObjectName.checkValue(ObjectName.java:1014) ~[na:na]
	at java.management/javax.management.ObjectName.construct(ObjectName.java:730) ~[na:na]
	at java.management/javax.management.ObjectName.<init>(ObjectName.java:1451) ~[na:na]
	at java.management/javax.management.ObjectName.getInstance(ObjectName.java:1357) ~[na:na]
	at org.springframework.jmx.support.ObjectNameManager.getInstance(ObjectNameManager.java:99) ~[spring-context-6.1.1.jar:6.1.1]
	at org.springframework.jmx.export.naming.MetadataNamingStrategy.getObjectName(MetadataNamingStrategy.java:136) ~[spring-context-6.1.1.jar:6.1.1]
	at org.springframework.boot.autoconfigure.jmx.ParentAwareNamingStrategy.getObjectName(ParentAwareNamingStrategy.java:66) ~[spring-boot-autoconfigure-3.2.0.jar:3.2.0]
	at org.springframework.jmx.export.MBeanExporter.getObjectName(MBeanExporter.java:771) ~[spring-context-6.1.1.jar:6.1.1]
	at org.springframework.jmx.export.MBeanExporter.registerBeanInstance(MBeanExporter.java:670) ~[spring-context-6.1.1.jar:6.1.1]
	at org.springframework.jmx.export.MBeanExporter.registerBeanNameOrInstance(MBeanExporter.java:631) ~[spring-context-6.1.1.jar:6.1.1]

According to this doc from Oracle, they recommend to always quote name attribute: https://www.oracle.com/java/technologies/javase/management-extensions-best-practices.html#mozTocId434075

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.

[snicoll]

Thanks for the sample @artembilan!