From e6b5322f3e7da3c973189c1dd65848e587742d85 Mon Sep 17 00:00:00 2001 From: Fernando Cappi <3818944+fcappi@users.noreply.github.com> Date: Thu, 15 Jun 2023 17:20:02 +0100 Subject: [PATCH 1/2] Apply SslConfigurer in addition to configured mappers Update `ReactorClientHttpConnectorFactory` to that SSL configuration is applied in addition to any configured mappers. Prior to this commit, SSL configuration would prevent configured mappers from being applied. See gh-35914 --- .../client/ReactorClientHttpConnectorFactory.java | 14 +++++++------- ...ientHttpConnectorFactoryConfigurationTests.java | 9 ++++++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorClientHttpConnectorFactory.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorClientHttpConnectorFactory.java index 472614ee99..256a91d3e5 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorClientHttpConnectorFactory.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorClientHttpConnectorFactory.java @@ -55,14 +55,14 @@ class ReactorClientHttpConnectorFactory implements ClientHttpConnectorFactory (client) -> after.configure(before.configure(client))) - .orElse((client) -> client); - if (sslBundle != null) { - mapper = new SslConfigurer(sslBundle)::configure; - } - return new ReactorClientHttpConnector(this.reactorResourceFactory, mapper::configure); + final ReactorNettyHttpClientMapper mapper = + Stream.concat( + this.mappers.get(), + Stream.ofNullable(sslBundle != null ? (ReactorNettyHttpClientMapper) new SslConfigurer(sslBundle)::configure : null)) + .reduce((before, after) -> (client) -> after.configure(before.configure(client))) + .orElse((client) -> client); + return new ReactorClientHttpConnector(this.reactorResourceFactory, mapper::configure); } /** diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ClientHttpConnectorFactoryConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ClientHttpConnectorFactoryConfigurationTests.java index 234b9d7df0..4e743f637c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ClientHttpConnectorFactoryConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ClientHttpConnectorFactoryConfigurationTests.java @@ -24,6 +24,10 @@ import org.eclipse.jetty.util.thread.Scheduler; import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.ssl.SslBundle; +import org.springframework.boot.ssl.SslBundleKey; +import org.springframework.boot.ssl.jks.JksSslStoreBundle; +import org.springframework.boot.ssl.jks.JksSslStoreDetails; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner; import org.springframework.context.annotation.Bean; @@ -80,11 +84,14 @@ class ClientHttpConnectorFactoryConfigurationTests { @Test void shouldApplyHttpClientMapper() { + JksSslStoreDetails storeDetails = JksSslStoreDetails.forLocation("classpath:test.jks"); + JksSslStoreBundle stores = new JksSslStoreBundle(storeDetails, storeDetails); + SslBundle sslBundle = SslBundle.of(stores, SslBundleKey.of("password")); new ReactiveWebApplicationContextRunner() .withConfiguration(AutoConfigurations.of(ClientHttpConnectorFactoryConfiguration.ReactorNetty.class)) .withUserConfiguration(CustomHttpClientMapper.class) .run((context) -> { - context.getBean(ReactorClientHttpConnectorFactory.class).createClientHttpConnector(); + context.getBean(ReactorClientHttpConnectorFactory.class).createClientHttpConnector(sslBundle); assertThat(CustomHttpClientMapper.called).isTrue(); }); } From 3c7fbf34230ab360785d85a9e02d9c2f5f510fc5 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 21 Jun 2023 11:44:30 -0700 Subject: [PATCH 2/2] Polish 'Apply SslConfigurer in addition to configured mappers' See gh-35914 --- .../ReactorClientHttpConnectorFactory.java | 24 +++-- .../client/ReactorNettyHttpClientMapper.java | 33 ++++++- ...ttpConnectorFactoryConfigurationTests.java | 5 +- .../ReactorNettyHttpClientMapperTests.java | 99 +++++++++++++++++++ 4 files changed, 149 insertions(+), 12 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorNettyHttpClientMapperTests.java diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorClientHttpConnectorFactory.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorClientHttpConnectorFactory.java index 256a91d3e5..b5dcd6b136 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorClientHttpConnectorFactory.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorClientHttpConnectorFactory.java @@ -16,7 +16,10 @@ package org.springframework.boot.autoconfigure.web.reactive.function.client; +import java.util.ArrayList; +import java.util.List; import java.util.function.Supplier; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.net.ssl.SSLException; @@ -36,6 +39,7 @@ import org.springframework.util.function.ThrowingConsumer; * {@link ClientHttpConnectorFactory} for {@link ReactorClientHttpConnector}. * * @author Phillip Webb + * @author Fernando Cappi */ class ReactorClientHttpConnectorFactory implements ClientHttpConnectorFactory { @@ -55,20 +59,19 @@ class ReactorClientHttpConnectorFactory implements ClientHttpConnectorFactory (client) -> after.configure(before.configure(client))) - .orElse((client) -> client); - - return new ReactorClientHttpConnector(this.reactorResourceFactory, mapper::configure); + List mappers = this.mappers.get() + .collect(Collectors.toCollection(ArrayList::new)); + if (sslBundle != null) { + mappers.add(new SslConfigurer(sslBundle)); + } + return new ReactorClientHttpConnector(this.reactorResourceFactory, + ReactorNettyHttpClientMapper.of(mappers)::configure); } /** * Configures the Netty {@link HttpClient} with SSL. */ - private static class SslConfigurer { + private static class SslConfigurer implements ReactorNettyHttpClientMapper { private final SslBundle sslBundle; @@ -76,7 +79,8 @@ class ReactorClientHttpConnectorFactory implements ClientHttpConnectorFactory mappers) { + Assert.notNull(mappers, "Mappers must not be null"); + return of(mappers.toArray(ReactorNettyHttpClientMapper[]::new)); + } + + /** + * Return a new {@link ReactorNettyHttpClientMapper} composed of the given mappers. + * @param mappers the mappers to compose + * @return a composed {@link ReactorNettyHttpClientMapper} instance + * @since 3.1.1 + */ + static ReactorNettyHttpClientMapper of(ReactorNettyHttpClientMapper... mappers) { + Assert.notNull(mappers, "Mappers must not be null"); + return (httpClient) -> { + for (ReactorNettyHttpClientMapper mapper : mappers) { + httpClient = mapper.configure(httpClient); + } + return httpClient; + }; + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ClientHttpConnectorFactoryConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ClientHttpConnectorFactoryConfigurationTests.java index 4e743f637c..99f99ce617 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ClientHttpConnectorFactoryConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ClientHttpConnectorFactoryConfigurationTests.java @@ -38,6 +38,8 @@ import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; /** * Tests for {@link ClientHttpConnectorFactoryConfiguration}. @@ -86,13 +88,14 @@ class ClientHttpConnectorFactoryConfigurationTests { void shouldApplyHttpClientMapper() { JksSslStoreDetails storeDetails = JksSslStoreDetails.forLocation("classpath:test.jks"); JksSslStoreBundle stores = new JksSslStoreBundle(storeDetails, storeDetails); - SslBundle sslBundle = SslBundle.of(stores, SslBundleKey.of("password")); + SslBundle sslBundle = spy(SslBundle.of(stores, SslBundleKey.of("password"))); new ReactiveWebApplicationContextRunner() .withConfiguration(AutoConfigurations.of(ClientHttpConnectorFactoryConfiguration.ReactorNetty.class)) .withUserConfiguration(CustomHttpClientMapper.class) .run((context) -> { context.getBean(ReactorClientHttpConnectorFactory.class).createClientHttpConnector(sslBundle); assertThat(CustomHttpClientMapper.called).isTrue(); + verify(sslBundle).getManagers(); }); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorNettyHttpClientMapperTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorNettyHttpClientMapperTests.java new file mode 100644 index 0000000000..a49c95a6f4 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/function/client/ReactorNettyHttpClientMapperTests.java @@ -0,0 +1,99 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.autoconfigure.web.reactive.function.client; + +import java.util.Collection; +import java.util.List; + +import org.junit.jupiter.api.Test; +import reactor.netty.http.client.HttpClient; +import reactor.netty.http.client.HttpClientConfig; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +/** + * Tests for {@link ReactorNettyHttpClientMapper}. + * + * @author Phillip Webb + */ +class ReactorNettyHttpClientMapperTests { + + @Test + void ofWithCollectionCreatesComposite() { + ReactorNettyHttpClientMapper one = (httpClient) -> new TestHttpClient(httpClient, "1"); + ReactorNettyHttpClientMapper two = (httpClient) -> new TestHttpClient(httpClient, "2"); + ReactorNettyHttpClientMapper three = (httpClient) -> new TestHttpClient(httpClient, "3"); + ReactorNettyHttpClientMapper compose = ReactorNettyHttpClientMapper.of(List.of(one, two, three)); + TestHttpClient httpClient = (TestHttpClient) compose.configure(new TestHttpClient()); + assertThat(httpClient.getContent()).isEqualTo("123"); + } + + @Test + void ofWhenCollectionIsNullThrowsException() { + Collection mappers = null; + assertThatIllegalArgumentException().isThrownBy(() -> ReactorNettyHttpClientMapper.of(mappers)) + .withMessage("Mappers must not be null"); + } + + @Test + void ofWithArrayCreatesComposite() { + ReactorNettyHttpClientMapper one = (httpClient) -> new TestHttpClient(httpClient, "1"); + ReactorNettyHttpClientMapper two = (httpClient) -> new TestHttpClient(httpClient, "2"); + ReactorNettyHttpClientMapper three = (httpClient) -> new TestHttpClient(httpClient, "3"); + ReactorNettyHttpClientMapper compose = ReactorNettyHttpClientMapper.of(one, two, three); + TestHttpClient httpClient = (TestHttpClient) compose.configure(new TestHttpClient()); + assertThat(httpClient.getContent()).isEqualTo("123"); + } + + @Test + void ofWhenArrayIsNullThrowsException() { + ReactorNettyHttpClientMapper[] mappers = null; + assertThatIllegalArgumentException().isThrownBy(() -> ReactorNettyHttpClientMapper.of(mappers)) + .withMessage("Mappers must not be null"); + } + + private static class TestHttpClient extends HttpClient { + + private final String content; + + TestHttpClient() { + this.content = ""; + } + + TestHttpClient(HttpClient httpClient, String content) { + this.content = (httpClient instanceof TestHttpClient testHttpClient) ? testHttpClient.content + content + : content; + } + + @Override + public HttpClientConfig configuration() { + throw new UnsupportedOperationException("Auto-generated method stub"); + } + + @Override + protected HttpClient duplicate() { + throw new UnsupportedOperationException("Auto-generated method stub"); + } + + String getContent() { + return this.content; + } + + } + +}