rabbitmq/rabbitmq-java-client

IndexOutOfBoundsException when calling ConnectionFactory.newConnection()

Logic-32 opened this issue · 4 comments

Logic-32 commented

Describe the bug

In v5.16.0 a behavior change was introduced in ConnectionFactory.createAddressResolver().

Note the different if-conditions in the body. In the case where addresses.size() == 0 (isEmpty()) then:

  • v5.15.0 will create a ListAddressResolver
  • v5.16.0 will try to pull the first item from addresses and cause an IndexOutOfBoundsException

Example stack trace while using v5.18.0, where the bug is still present:

Index 0 out of bounds for length 0
java.lang.IndexOutOfBoundsException
	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
	at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248)
	at java.base/java.util.Objects.checkIndex(Objects.java:373)
	at java.base/java.util.ArrayList.get(ArrayList.java:426)
	at com.rabbitmq.client.ConnectionFactory.createAddressResolver(ConnectionFactory.java:1361)
	at com.rabbitmq.client.ConnectionFactory.newConnection(ConnectionFactory.java:1184)
	at com.rabbitmq.client.ConnectionFactory.newConnection(ConnectionFactory.java:1061)
...

Reproduction steps

  1. Create a ConnectionFactory with an empty List of addresses (e.g. use List.of())
  2. Call newConnection()
  3. Fail.

Expected behavior

While I realize it is absurd to try and connect to RMQ with no addresses provided we had unit tests that did exactly that for some reason. Interestingly, none of the usages of AddressResolver actually seem to care if an address is returned or not. So, the tests ran fine.

With that said, my expectations here would be either:

  1. An isEmpty() (or equivalent) check is added and ListAddressResolver continues to be used since it seems to work fine, or
  2. A check is still added and a useful Exception is thrown indicating that this is an invalid state

Additional context

No response

michaelklishin commented

Please submit a PR. You seem to understand the problem well.

Logic-32 commented

Is there a preference for which expectation to follow through with?

acogoluegnes commented

I'd add a check in createAddressResolver: throw an IllegalArgumentException if the List<Address> is empty.

Logic-32 commented

Thank you for beating me to the punch!

  • 👍1