From 2210236f82ac0a90aca4613dac9834374afb6515 Mon Sep 17 00:00:00 2001 From: cbono Date: Sun, 24 Nov 2019 19:45:03 -0600 Subject: [PATCH 1/2] Use ssl.enabled flag when RabbitMQ address has no protocol See gh-19109 --- .../autoconfigure/amqp/RabbitProperties.java | 13 +++++---- .../amqp/RabbitPropertiesTests.java | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java index 349311597e25..f770907f5186 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java @@ -188,7 +188,7 @@ public void setAddresses(String addresses) { private List
parseAddresses(String addresses) { List
parsedAddresses = new ArrayList<>(); for (String address : StringUtils.commaDelimitedListToStringArray(addresses)) { - parsedAddresses.add(new Address(address)); + parsedAddresses.add(new Address(address, getSsl().isEnabled())); } return parsedAddresses; } @@ -968,21 +968,24 @@ private static final class Address { private boolean secureConnection; - private Address(String input) { + private Address(String input, boolean sslEnabled) { input = input.trim(); - input = trimPrefix(input); + input = trimPrefix(input, sslEnabled); input = parseUsernameAndPassword(input); input = parseVirtualHost(input); parseHostAndPort(input); } - private String trimPrefix(String input) { + private String trimPrefix(String input, boolean sslEnabled) { if (input.startsWith(PREFIX_AMQP_SECURE)) { this.secureConnection = true; return input.substring(PREFIX_AMQP_SECURE.length()); } if (input.startsWith(PREFIX_AMQP)) { - input = input.substring(PREFIX_AMQP.length()); + return input.substring(PREFIX_AMQP.length()); + } + if (sslEnabled) { + this.secureConnection = true; } return input; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java index 917c09847dfe..998a9488f3ac 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java @@ -101,6 +101,21 @@ 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); + this.properties.setPort(1234); + this.properties.setAddresses("rabbit1.example.com,rabbit2.example.com:2345"); + assertThat(this.properties.determinePort()).isEqualTo(5671); + } + @Test public void virtualHostDefaultsToNull() { assertThat(this.properties.getVirtualHost()).isNull(); @@ -240,6 +255,20 @@ public void determineSslUsingAmqpsReturnsStateOfFirstAddress() { assertThat(this.properties.getSsl().determineEnabled()).isTrue(); } + @Test + public void determineSslUsingAddressWithoutAmpqsDefersToSslFlagProperty() { + this.properties.getSsl().setEnabled(true); + this.properties.setAddresses("root:password@otherhost"); + assertThat(this.properties.getSsl().determineEnabled()).isTrue(); + } + + @Test + public void determineSslUsingAddressWithoutAmpqDefersToSslFlagProperty() { + this.properties.getSsl().setEnabled(false); + this.properties.setAddresses("root:password@otherhost"); + assertThat(this.properties.getSsl().determineEnabled()).isFalse(); + } + @Test public void determineSslUsingAmqpReturnsStateOfFirstAddress() { this.properties.setAddresses("amqp://root:password@otherhost,amqps://root:password2@otherhost2"); From 5d8fe860d744f1327499a1daf0f71ce6866798da Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 25 Nov 2019 11:17:33 +0100 Subject: [PATCH 2/2] Polish "Use ssl.enabled flag when RabbitMQ address has no protocol" 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 --- .../autoconfigure/amqp/RabbitProperties.java | 22 ++++++++++--------- .../amqp/RabbitPropertiesTests.java | 11 ++-------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java index f770907f5186..3c42f132a41b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java @@ -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) { @@ -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; } @@ -1016,11 +1014,11 @@ 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); @@ -1028,6 +1026,10 @@ private void parseHostAndPort(String input) { } } + private boolean determineSslEnabled(boolean sslEnabled) { + return (this.secureConnection != null) ? this.secureConnection : sslEnabled; + } + } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java index 998a9488f3ac..efe56f29f810 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java @@ -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); @@ -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();