Skip to content

Commit

Permalink
Polish "Use ssl.enabled flag when RabbitMQ address has no protocol"
Browse files Browse the repository at this point in the history
There is a direct link between the sslEnabled flag and the default port
that should be used by an address. The checks are currently set in two
places:

* Determine which port should be used
* Determine if SSL should be enabled

This commit polishes the initial proposal so that secureConnection is
only set if a protocol is available.

See gh-19109
  • Loading branch information
snicoll committed Nov 25, 2019
1 parent 2210236 commit 5d8fe86
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public boolean determineEnabled() {
return isEnabled();
}
Address address = RabbitProperties.this.parsedAddresses.get(0);
return address.secureConnection;
return address.determineSslEnabled(isEnabled());
}

public void setEnabled(boolean enabled) {
Expand Down Expand Up @@ -966,27 +966,25 @@ private static final class Address {

private String virtualHost;

private boolean secureConnection;
private Boolean secureConnection;

private Address(String input, boolean sslEnabled) {
input = input.trim();
input = trimPrefix(input, sslEnabled);
input = trimPrefix(input);
input = parseUsernameAndPassword(input);
input = parseVirtualHost(input);
parseHostAndPort(input);
parseHostAndPort(input, sslEnabled);
}

private String trimPrefix(String input, boolean sslEnabled) {
private String trimPrefix(String input) {
if (input.startsWith(PREFIX_AMQP_SECURE)) {
this.secureConnection = true;
return input.substring(PREFIX_AMQP_SECURE.length());
}
if (input.startsWith(PREFIX_AMQP)) {
this.secureConnection = false;
return input.substring(PREFIX_AMQP.length());
}
if (sslEnabled) {
this.secureConnection = true;
}
return input;
}

Expand Down Expand Up @@ -1016,18 +1014,22 @@ private String parseVirtualHost(String input) {
return input;
}

private void parseHostAndPort(String input) {
private void parseHostAndPort(String input, boolean sslEnabled) {
int portIndex = input.indexOf(':');
if (portIndex == -1) {
this.host = input;
this.port = (this.secureConnection) ? DEFAULT_PORT_SECURE : DEFAULT_PORT;
this.port = (determineSslEnabled(sslEnabled)) ? DEFAULT_PORT_SECURE : DEFAULT_PORT;
}
else {
this.host = input.substring(0, portIndex);
this.port = Integer.valueOf(input.substring(portIndex + 1));
}
}

private boolean determineSslEnabled(boolean sslEnabled) {
return (this.secureConnection != null) ? this.secureConnection : sslEnabled;
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,6 @@ public void determinePortUsingAmqpsReturnsPortOfFirstAddress() {
assertThat(this.properties.determinePort()).isEqualTo(5671);
}

@Test
public void determinePortReturnsDefaultAmqpsPortWhenFirstAddressHasNoExplicitPort() {
this.properties.setPort(1234);
this.properties.setAddresses("rabbit1.example.com,rabbit2.example.com:2345");
assertThat(this.properties.determinePort()).isEqualTo(5672);
}

@Test
public void determinePortReturnsDefaultAmqpsPortWhenFirstAddressHasNoExplicitPortButSslEnabled() {
this.properties.getSsl().setEnabled(true);
Expand Down Expand Up @@ -256,14 +249,14 @@ public void determineSslUsingAmqpsReturnsStateOfFirstAddress() {
}

@Test
public void determineSslUsingAddressWithoutAmpqsDefersToSslFlagProperty() {
public void sslDetermineEnabledIsTrueWhenAddressHasNoProtocolAndSslIsEnabled() {
this.properties.getSsl().setEnabled(true);
this.properties.setAddresses("root:password@otherhost");
assertThat(this.properties.getSsl().determineEnabled()).isTrue();
}

@Test
public void determineSslUsingAddressWithoutAmpqDefersToSslFlagProperty() {
public void sslDetermineEnabledIsFalseWhenAddressHasNoProtocolAndSslIsDisabled() {
this.properties.getSsl().setEnabled(false);
this.properties.setAddresses("root:password@otherhost");
assertThat(this.properties.getSsl().determineEnabled()).isFalse();
Expand Down

0 comments on commit 5d8fe86

Please sign in to comment.