Skip to content

Commit

Permalink
Prevent NPE when using pathExtension predicate
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sdeleuze committed Mar 12, 2024
1 parent 6767f70 commit b1bf8c5
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ public PathExtensionPredicate(String extension) {
@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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.net.URI;
import java.util.Collections;
import java.util.List;
import java.util.function.Function;

import org.junit.jupiter.api.Test;
Expand All @@ -33,6 +34,7 @@

/**
* @author Arjen Poutsma
* @author Sebastien Deleuze
*/
class RequestPredicatesTests {

Expand Down Expand Up @@ -317,6 +319,32 @@ void pathExtension() {
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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ public PathExtensionPredicate(String extension) {
@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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -34,6 +35,7 @@

/**
* @author Arjen Poutsma
* @author Sebastien Deleuze
*/
class RequestPredicatesTests {

Expand Down Expand Up @@ -232,6 +234,18 @@ void pathExtension() {
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
Expand Down

0 comments on commit b1bf8c5

Please sign in to comment.