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
This commit is contained in:
Sébastien Deleuze
2024-03-11 18:24:04 +01:00
parent 6767f7010c
commit b1bf8c5242
4 changed files with 44 additions and 2 deletions

View File

@@ -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

View File

@@ -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<String> 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")

View File

@@ -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

View File

@@ -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<String> 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