From 93a24eabb1bbb89b7a5fcdbd9fd6b1feb668174f Mon Sep 17 00:00:00 2001 From: kdomo Date: Sun, 30 Aug 2026 13:12:35 +0900 Subject: [PATCH 1/2] Fix null customizer checks in HTTP client builders AbstractClientHttpRequestFactoryBuilder.mergedCustomizers and its reactive counterpart asserted on the customizers field rather than the customizer parameter. The field is never null since the constructor defaults it to an empty list, so the assertion always passed and a null customizer was not rejected. See gh-51509 Signed-off-by: kdomo --- .../AbstractClientHttpRequestFactoryBuilder.java | 2 +- .../reactive/AbstractClientHttpConnectorBuilder.java | 2 +- .../client/reactive/ClientHttpConnectorBuilder.java | 2 +- .../HttpComponentsClientHttpConnectorBuilder.java | 2 +- .../SimpleClientHttpRequestFactoryBuilderTests.java | 10 ++++++++++ .../HttpComponentsClientHttpConnectorBuilderTests.java | 9 +++++++++ .../reactive/JdkClientHttpConnectorBuilderTests.java | 8 ++++++++ 7 files changed, 31 insertions(+), 4 deletions(-) diff --git a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilder.java b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilder.java index 07c1c4c8fc1..fb34423aed2 100644 --- a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilder.java +++ b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilder.java @@ -48,7 +48,7 @@ abstract class AbstractClientHttpRequestFactoryBuilder> mergedCustomizers(Consumer customizer) { - Assert.notNull(this.customizers, "'customizer' must not be null"); + Assert.notNull(customizer, "'customizer' must not be null"); return merge(this.customizers, List.of(customizer)); } diff --git a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/AbstractClientHttpConnectorBuilder.java b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/AbstractClientHttpConnectorBuilder.java index a9e8d0f9e04..573becc5b2d 100644 --- a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/AbstractClientHttpConnectorBuilder.java +++ b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/AbstractClientHttpConnectorBuilder.java @@ -49,7 +49,7 @@ abstract class AbstractClientHttpConnectorBuilder } protected final List> mergedCustomizers(Consumer customizer) { - Assert.notNull(this.customizers, "'customizer' must not be null"); + Assert.notNull(customizer, "'customizer' must not be null"); return merge(this.customizers, List.of(customizer)); } diff --git a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/ClientHttpConnectorBuilder.java b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/ClientHttpConnectorBuilder.java index 5ea306737f5..c892754be59 100644 --- a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/ClientHttpConnectorBuilder.java +++ b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/ClientHttpConnectorBuilder.java @@ -139,7 +139,7 @@ public interface ClientHttpConnectorBuilder { */ @SuppressWarnings("unchecked") static ClientHttpConnectorBuilder of(Class clientHttpConnectorType) { - Assert.notNull(clientHttpConnectorType, "'requestFactoryType' must not be null"); + Assert.notNull(clientHttpConnectorType, "'clientHttpConnectorType' must not be null"); Assert.isTrue(clientHttpConnectorType != ClientHttpConnector.class, "'clientHttpConnectorType' must be an implementation of ClientHttpConnector"); if (clientHttpConnectorType == ReactorClientHttpConnector.class) { diff --git a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilder.java b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilder.java index d47f8c424ed..6eff3a3c598 100644 --- a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilder.java +++ b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilder.java @@ -66,7 +66,7 @@ public final class HttpComponentsClientHttpConnectorBuilder */ public HttpComponentsClientHttpConnectorBuilder withHttpClientCustomizer( Consumer httpClientCustomizer) { - Assert.notNull(httpClientCustomizer, "'customizer' must not be null"); + Assert.notNull(httpClientCustomizer, "'httpClientCustomizer' must not be null"); return new HttpComponentsClientHttpConnectorBuilder(getCustomizers(), this.httpClientBuilder.withCustomizer(httpClientCustomizer)); } diff --git a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java index 1b134981adf..191d5f5b237 100644 --- a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java +++ b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.http.client; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -25,6 +26,7 @@ import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** @@ -81,6 +83,14 @@ class SimpleClientHttpRequestFactoryBuilderTests super.redirectDontFollow(httpMethod); } + @Test + @SuppressWarnings("NullAway") // Test null check + void withCustomizerWhenCustomizerIsNullThrowsException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> ClientHttpRequestFactoryBuilder.simple().withCustomizer(null)) + .withMessage("'customizer' must not be null"); + } + @Override protected HttpStatus getExpectedRedirect(HttpMethod httpMethod) { return (httpMethod != HttpMethod.GET) ? HttpStatus.FOUND : HttpStatus.OK; diff --git a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilderTests.java b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilderTests.java index 0b5a7857478..e6b250fe56b 100644 --- a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilderTests.java +++ b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilderTests.java @@ -39,6 +39,7 @@ import org.springframework.http.client.reactive.HttpComponentsClientHttpConnecto import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Tests for {@link HttpComponentsClientHttpConnectorBuilder} and @@ -80,6 +81,14 @@ class HttpComponentsClientHttpConnectorBuilderTests defaultRequestConfigCustomizer1.assertCalled(); } + @Test + @SuppressWarnings("NullAway") // Test null check + void withHttpClientCustomizerWhenCustomizerIsNullThrowsException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> ClientHttpConnectorBuilder.httpComponents().withHttpClientCustomizer(null)) + .withMessage("'httpClientCustomizer' must not be null"); + } + @Test @WithPackageResources("test.jks") void withTlsSocketStrategyFactory() { diff --git a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/JdkClientHttpConnectorBuilderTests.java b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/JdkClientHttpConnectorBuilderTests.java index 4af086153a5..23c802a6d71 100644 --- a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/JdkClientHttpConnectorBuilderTests.java +++ b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/JdkClientHttpConnectorBuilderTests.java @@ -28,6 +28,7 @@ import org.springframework.http.client.reactive.JdkClientHttpConnector; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Tests for {@link JdkClientHttpConnectorBuilder} and {@link JdkHttpClientBuilder}. @@ -52,6 +53,13 @@ class JdkClientHttpConnectorBuilderTests extends AbstractClientHttpConnectorBuil httpClientCustomizer2.assertCalled(); } + @Test + @SuppressWarnings("NullAway") // Test null check + void withCustomizerWhenCustomizerIsNullThrowsException() { + assertThatIllegalArgumentException().isThrownBy(() -> ClientHttpConnectorBuilder.jdk().withCustomizer(null)) + .withMessage("'customizer' must not be null"); + } + @Test void withExecutor() { Executor executor = new SimpleAsyncTaskExecutor(); From 60445a50693e6de9c6b8d0ba6232ff6559b7af53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Sun, 30 Aug 2026 20:16:36 +0200 Subject: [PATCH 2/2] Polish "Fix null customizer checks in HTTP client builders" See gh-51509 --- .../http/client/ClientHttpRequestFactoryBuilder.java | 1 + .../client/reactive/ClientHttpConnectorBuilder.java | 3 ++- .../HttpComponentsClientHttpConnectorBuilder.java | 2 +- .../AbstractClientHttpRequestFactoryBuilderTests.java | 8 ++++++++ .../SimpleClientHttpRequestFactoryBuilderTests.java | 10 ---------- .../AbstractClientHttpConnectorBuilderTests.java | 8 ++++++++ .../HttpComponentsClientHttpConnectorBuilderTests.java | 9 --------- .../reactive/JdkClientHttpConnectorBuilderTests.java | 8 -------- 8 files changed, 20 insertions(+), 29 deletions(-) diff --git a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/ClientHttpRequestFactoryBuilder.java b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/ClientHttpRequestFactoryBuilder.java index d0cd096fdd0..fdef32cf11b 100644 --- a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/ClientHttpRequestFactoryBuilder.java +++ b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/ClientHttpRequestFactoryBuilder.java @@ -70,6 +70,7 @@ public interface ClientHttpRequestFactoryBuilder withCustomizer(Consumer customizer) { + Assert.notNull(customizer, "'customizer' must not be null"); return withCustomizers(List.of(customizer)); } diff --git a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/ClientHttpConnectorBuilder.java b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/ClientHttpConnectorBuilder.java index c892754be59..89cbfcbb27c 100644 --- a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/ClientHttpConnectorBuilder.java +++ b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/ClientHttpConnectorBuilder.java @@ -68,6 +68,7 @@ public interface ClientHttpConnectorBuilder { * @return a new {@link ClientHttpConnectorBuilder} instance */ default ClientHttpConnectorBuilder withCustomizer(Consumer customizer) { + Assert.notNull(customizer, "'customizer' must not be null"); return withCustomizers(List.of(customizer)); } @@ -139,7 +140,7 @@ public interface ClientHttpConnectorBuilder { */ @SuppressWarnings("unchecked") static ClientHttpConnectorBuilder of(Class clientHttpConnectorType) { - Assert.notNull(clientHttpConnectorType, "'clientHttpConnectorType' must not be null"); + Assert.notNull(clientHttpConnectorType, "'requestFactoryType' must not be null"); Assert.isTrue(clientHttpConnectorType != ClientHttpConnector.class, "'clientHttpConnectorType' must be an implementation of ClientHttpConnector"); if (clientHttpConnectorType == ReactorClientHttpConnector.class) { diff --git a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilder.java b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilder.java index 6eff3a3c598..d47f8c424ed 100644 --- a/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilder.java +++ b/module/spring-boot-http-client/src/main/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilder.java @@ -66,7 +66,7 @@ public final class HttpComponentsClientHttpConnectorBuilder */ public HttpComponentsClientHttpConnectorBuilder withHttpClientCustomizer( Consumer httpClientCustomizer) { - Assert.notNull(httpClientCustomizer, "'httpClientCustomizer' must not be null"); + Assert.notNull(httpClientCustomizer, "'customizer' must not be null"); return new HttpComponentsClientHttpConnectorBuilder(getCustomizers(), this.httpClientBuilder.withCustomizer(httpClientCustomizer)); } diff --git a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java index 2b13595e11e..fab586a415a 100644 --- a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java +++ b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java @@ -54,6 +54,7 @@ import org.springframework.util.StreamUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Base class for {@link ClientHttpRequestFactoryBuilder} tests. @@ -76,6 +77,13 @@ abstract class AbstractClientHttpRequestFactoryBuilderTests this.builder.withCustomizer(null)) + .withMessage("'customizer' must not be null"); + } + @Test void buildReturnsRequestFactoryOfExpectedType() { T requestFactory = this.builder.build(); diff --git a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java index 191d5f5b237..1b134981adf 100644 --- a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java +++ b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java @@ -16,7 +16,6 @@ package org.springframework.boot.http.client; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -26,7 +25,6 @@ import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** @@ -83,14 +81,6 @@ class SimpleClientHttpRequestFactoryBuilderTests super.redirectDontFollow(httpMethod); } - @Test - @SuppressWarnings("NullAway") // Test null check - void withCustomizerWhenCustomizerIsNullThrowsException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> ClientHttpRequestFactoryBuilder.simple().withCustomizer(null)) - .withMessage("'customizer' must not be null"); - } - @Override protected HttpStatus getExpectedRedirect(HttpMethod httpMethod) { return (httpMethod != HttpMethod.GET) ? HttpStatus.FOUND : HttpStatus.OK; diff --git a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/AbstractClientHttpConnectorBuilderTests.java b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/AbstractClientHttpConnectorBuilderTests.java index 0ebdfb443e1..8d8998f84e8 100644 --- a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/AbstractClientHttpConnectorBuilderTests.java +++ b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/AbstractClientHttpConnectorBuilderTests.java @@ -56,6 +56,7 @@ import org.springframework.web.reactive.function.client.WebClientRequestExceptio import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Base class for {@link ClientHttpConnectorBuilder} tests. @@ -77,6 +78,13 @@ abstract class AbstractClientHttpConnectorBuilderTests this.builder.withCustomizer(null)) + .withMessage("'customizer' must not be null"); + } + @Test void buildReturnsConnectorOfExpectedType() { T connector = this.builder.build(); diff --git a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilderTests.java b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilderTests.java index e6b250fe56b..0b5a7857478 100644 --- a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilderTests.java +++ b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/HttpComponentsClientHttpConnectorBuilderTests.java @@ -39,7 +39,6 @@ import org.springframework.http.client.reactive.HttpComponentsClientHttpConnecto import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Tests for {@link HttpComponentsClientHttpConnectorBuilder} and @@ -81,14 +80,6 @@ class HttpComponentsClientHttpConnectorBuilderTests defaultRequestConfigCustomizer1.assertCalled(); } - @Test - @SuppressWarnings("NullAway") // Test null check - void withHttpClientCustomizerWhenCustomizerIsNullThrowsException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> ClientHttpConnectorBuilder.httpComponents().withHttpClientCustomizer(null)) - .withMessage("'httpClientCustomizer' must not be null"); - } - @Test @WithPackageResources("test.jks") void withTlsSocketStrategyFactory() { diff --git a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/JdkClientHttpConnectorBuilderTests.java b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/JdkClientHttpConnectorBuilderTests.java index 23c802a6d71..4af086153a5 100644 --- a/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/JdkClientHttpConnectorBuilderTests.java +++ b/module/spring-boot-http-client/src/test/java/org/springframework/boot/http/client/reactive/JdkClientHttpConnectorBuilderTests.java @@ -28,7 +28,6 @@ import org.springframework.http.client.reactive.JdkClientHttpConnector; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Tests for {@link JdkClientHttpConnectorBuilder} and {@link JdkHttpClientBuilder}. @@ -53,13 +52,6 @@ class JdkClientHttpConnectorBuilderTests extends AbstractClientHttpConnectorBuil httpClientCustomizer2.assertCalled(); } - @Test - @SuppressWarnings("NullAway") // Test null check - void withCustomizerWhenCustomizerIsNullThrowsException() { - assertThatIllegalArgumentException().isThrownBy(() -> ClientHttpConnectorBuilder.jdk().withCustomizer(null)) - .withMessage("'customizer' must not be null"); - } - @Test void withExecutor() { Executor executor = new SimpleAsyncTaskExecutor();