Improve error message when accidentally specifying multiple hosts in spring.rabbitmq.host

[francis-a]

When using YAML configuration with the Spring Boot AMQP starter in version 3.0.x it was possible to omit the addresses configuration property and instead use host and port.

For example:

spring:
  rabbitmq:
    host: "my-rmq-host.net"
    username: "host_username"
    password: "host_password"
    port: 5672

After upgrading to Spring Boot 3.1.0 this configuration option no longer works. The service fails to start with a java.lang.ArrayIndexOutOfBoundsException.

Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 1
	at org.springframework.boot.autoconfigure.amqp.PropertiesRabbitConnectionDetails.getAddresses(PropertiesRabbitConnectionDetails.java:57) ~[spring-boot-autoconfigure-3.1.0.jar:3.1.0]
	at org.springframework.boot.autoconfigure.amqp.RabbitConnectionDetails.getFirstAddress(RabbitConnectionDetails.java:71) ~[spring-boot-autoconfigure-3.1.0.jar:3.1.0]
	at org.springframework.boot.autoconfigure.amqp.RabbitConnectionFactoryBeanConfigurer.configure(RabbitConnectionFactoryBeanConfigurer.java:98) ~[spring-boot-autoconfigure-3.1.0.jar:3.1.0]
	at org.springframework.boot.autoconfigure.amqp.RabbitAutoConfiguration$RabbitConnectionFactoryCreator.rabbitConnectionFactory(RabbitAutoConfiguration.java:126) ~[spring-boot-autoconfigure-3.1.0.jar:3.1.0]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[na:na]
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:139) ~[spring-beans-6.0.9.jar:6.0.9]
	... 62 common frames omitted

The root cause of this looks to be inside the RabbitConnectionFactoryBeanConfigurer, specifically Address address = this.connectionDetails.getFirstAddress();

In the case I described above the local RabbitConnectionDetails class already has a host, port, username and password configured. The values in these fields match the YAML configuration I described above. It looks like the public void configure(RabbitConnectionFactoryBean factory) is not attempting to use these values but instead requiring them to be set on an Address. With no address defined in my application yaml this change results in an ArrayIndexOutOfBoundsException.

Is this change intended? Is the rabbitmq.host property now deprecated and replaced by addresses? If so is the port property also deprecated?

[quaff]

The test passed.

	@Test
	void test(){
		RabbitProperties properties = new RabbitProperties();
		properties.setHost("test.com");
		properties.setPort(5672);
		PropertiesRabbitConnectionDetails details = new PropertiesRabbitConnectionDetails(properties);
		assertThat(details.getAddresses()).element(0).isEqualTo(new RabbitConnectionDetails.Address(properties.getHost(), properties.getPort()));
	}

[francis-a]

After looking a bit more it looks like the issue is actually in PropertiesRabbitConnectionDetails.

	@Override
	public List<Address> getAddresses() {
		List<Address> addresses = new ArrayList<>();
		for (String address : this.properties.determineAddresses().split(",")) {
			String[] components = address.split(":");
			addresses.add(new Address(components[0], Integer.parseInt(components[1])));
		}
		return addresses;
	}

the determineAddresses method returns the correct addresses, the values set in the host field. However since there is no port in the returned address Integer.parseInt(components[1])) fails.

It seems like the called method determineAddresses() is concatenating the port a single time at the end of the host.

For example, the result of this method is:
host-a,host-b,host-c:5672

The caller seems to be expecting the port to be concatenated to the end of each host

[mhalbritter]

I've added a test using your values from the properties:

	@Test
	void name() {
		RabbitProperties properties = new RabbitProperties();
		properties.setHost("my-rmq-host.net");
		properties.setUsername("host_username");
		properties.setPassword("host_password");
		properties.setPort(5672);
		PropertiesRabbitConnectionDetails details = new PropertiesRabbitConnectionDetails(properties);
		assertThat(details.getAddresses()).isEqualTo(List.of(new Address("my-rmq-host.net", 5672)));
		assertThat(details.getVirtualHost()).isNull();
		assertThat(details.getUsername()).isEqualTo("host_username");
		assertThat(details.getPassword()).isEqualTo("host_password");
	}

This passes.

It also works when putting those properties in an application.yaml.

Please provide the YAML which doesn't work, or a minimal sample which we can use to reproduce the problem. Thanks!

[francis-a]

Thanks,

If you change properties.setHost("my-rmq-host.net"); in your example to properties.setHost("my-rmq-host.net,my-rmq-host-2.net"); you should see the same issue

[mhalbritter]

I think the spring.rabbitmq.host property was never intended to have more than 1 host in it. In this case, addresses should be used.

To answer your question:

Is the rabbitmq.host property now deprecated and replaced by addresses? If so is the port property also deprecated?

No, we didn't deprecate anything. They're still supported. It's only your usage of them worked by accident in the past. You should change your config to use addresses if having more than 1 host.

[francis-a]

Makes sense, however from looking around it seems like the previous behavior did allow for this. I'm wondering if it would still be worthwhile to either provide a better error message here or maybe even revert to the old behavior.

Since both these fields essentially seem do the same thing and even share an implementation maybe in the future it would be worth deprecating one over the other.

[mhalbritter]

to either provide a better error message here

Yeah, that should be possible.

[francis-a]

Thanks for the support - this is clear to me now.

[mhalbritter]

We're going to add a check for a , in the host in org.springframework.boot.autoconfigure.amqp.RabbitProperties#determineAddresses and let it fail with a better error message if that's the case.

[rafaelrc7]

Hello, is this issue available for work?

[mhalbritter]

Yes, feel free to work on it. I'll assign the issue to you, please also make sure you have read our contributing document and here are some tips how to get started. If you encounter any problems, feel free to reach out. Have fun!

[rafaelrc7]

Hello, I believe I have a solution ready. However, before opening a PR, I would like to confirm some points, as this is my first contribution. I did as suggested and implemented a check for commas on org.springframework.boot.autoconfigure.amqp.RabbitProperties#determineAddresses. I used org.springframework.boot.actuate.autoconfigure.wavefront.WavefrontProperties#getApiTokenOrThrow as an example and used an InvalidConfigurationPropertyValueException to send the error message. Is that expected and okay? Furthermore, I would like to ask if the error message I wrote is good. The code can be found at

https://github.com/rafaelrc7/spring-boot/blob/fdfb17fc1f20d3090e61b647855f6e37aec86955/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java#L208-L211

I also implemented a test case to validate the solution:

https://github.com/rafaelrc7/spring-boot/blob/fdfb17fc1f20d3090e61b647855f6e37aec86955/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java#L345-L351

Thanks.

[mhalbritter]

Hey @rafaelrc7, this looks good, thank you very much!

InvalidConfigurationPropertyValueException is the correct exception to use, and the message looks good to me too. And Thanks for writing this test!

The changes look good, please create a PR.

[mhalbritter]

Superseded by #35684.