From b1bf8c524255a7a01105de2ccae5e7ddd7ddcf03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Mon, 11 Mar 2024 18:24:04 +0100 Subject: [PATCH] Prevent NPE when using pathExtension predicate This commit ensures pathExtension predicate is skipped when the value is null, in order to provide a more predictable behavior, and allow a better compatibility with collections not supporting null elements like the ones created by List#of. Closes gh-32404 --- .../function/server/RequestPredicates.java | 2 +- .../server/RequestPredicatesTests.java | 28 +++++++++++++++++++ .../servlet/function/RequestPredicates.java | 2 +- .../function/RequestPredicatesTests.java | 14 ++++++++++ 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java index 4599215700..5af764dfe8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java @@ -843,7 +843,7 @@ public abstract class RequestPredicates { @Override public boolean test(ServerRequest request) { String pathExtension = UriUtils.extractFileExtension(request.path()); - return this.extensionPredicate.test(pathExtension); + return (pathExtension != null && this.extensionPredicate.test(pathExtension)); } @Override diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicatesTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicatesTests.java index 68140d02a0..d97e7a2a16 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicatesTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicatesTests.java @@ -18,6 +18,7 @@ package org.springframework.web.reactive.function.server; import java.net.URI; import java.util.Collections; +import java.util.List; import java.util.function.Function; import org.junit.jupiter.api.Test; @@ -33,6 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Arjen Poutsma + * @author Sebastien Deleuze */ class RequestPredicatesTests { @@ -317,6 +319,32 @@ class RequestPredicatesTests { assertThat(predicate.test(request)).isFalse(); } + @Test + void pathExtensionPredicate() { + List extensions = List.of("foo", "bar"); + RequestPredicate predicate = RequestPredicates.pathExtension(extensions::contains); + + URI uri = URI.create("https://localhost/file.foo"); + MockServerHttpRequest mockRequest = MockServerHttpRequest.method(HttpMethod.GET, uri).build(); + ServerRequest request = new DefaultServerRequest(MockServerWebExchange.from(mockRequest), Collections.emptyList()); + assertThat(predicate.test(request)).isTrue(); + + uri = URI.create("https://localhost/file.bar"); + mockRequest = MockServerHttpRequest.method(HttpMethod.GET, uri).build(); + request = new DefaultServerRequest(MockServerWebExchange.from(mockRequest), Collections.emptyList()); + assertThat(predicate.test(request)).isTrue(); + + uri = URI.create("https://localhost/file"); + mockRequest = MockServerHttpRequest.method(HttpMethod.GET, uri).build(); + request = new DefaultServerRequest(MockServerWebExchange.from(mockRequest), Collections.emptyList()); + assertThat(predicate.test(request)).isFalse(); + + uri = URI.create("https://localhost/file.baz"); + mockRequest = MockServerHttpRequest.method(HttpMethod.GET, uri).build(); + request = new DefaultServerRequest(MockServerWebExchange.from(mockRequest), Collections.emptyList()); + assertThat(predicate.test(request)).isFalse(); + } + @Test void queryParam() { MockServerHttpRequest mockRequest = MockServerHttpRequest.get("https://example.com") diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java index ecd097495c..5ab2b1ed8c 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java @@ -841,7 +841,7 @@ public abstract class RequestPredicates { @Override public boolean test(ServerRequest request) { String pathExtension = UriUtils.extractFileExtension(request.path()); - return this.extensionPredicate.test(pathExtension); + return (pathExtension != null && this.extensionPredicate.test(pathExtension)); } @Override diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RequestPredicatesTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RequestPredicatesTests.java index 4b56e61ed5..a14630a9a5 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RequestPredicatesTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RequestPredicatesTests.java @@ -17,6 +17,7 @@ package org.springframework.web.servlet.function; import java.util.Collections; +import java.util.List; import java.util.function.Consumer; import java.util.function.Function; @@ -34,6 +35,7 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Arjen Poutsma + * @author Sebastien Deleuze */ class RequestPredicatesTests { @@ -232,6 +234,18 @@ class RequestPredicatesTests { assertThat(predicate.test(initRequest("GET", "/FILE.TXT"))).isFalse(); assertThat(predicate.test(initRequest("GET", "/file.foo"))).isFalse(); + assertThat(predicate.test(initRequest("GET", "/file"))).isFalse(); + } + + @Test + void pathExtensionPredicate() { + List extensions = List.of("foo", "bar"); + RequestPredicate predicate = RequestPredicates.pathExtension(extensions::contains); + + assertThat(predicate.test(initRequest("GET", "/file.foo"))).isTrue(); + assertThat(predicate.test(initRequest("GET", "/file.bar"))).isTrue(); + assertThat(predicate.test(initRequest("GET", "/file"))).isFalse(); + assertThat(predicate.test(initRequest("GET", "/file.baz"))).isFalse(); } @Test