From 84200f3141d396dd1d2fd0ef84d4f73bb21c83be Mon Sep 17 00:00:00 2001 From: "Scheidter,Ryan" Date: Tue, 16 Jul 2019 08:06:02 -0500 Subject: [PATCH] Treat null path as non-matching pattern in AntPathMatcher Prior to this commit, a null path supplied to the isPattern(), match(), and matchStart() methods in AntPathMatcher resulted in a NullPointerException. This commit addresses this by treating a `null` path as a non-matching pattern. Closes gh-23297 --- .../springframework/util/AntPathMatcher.java | 5 ++- .../util/AntPathMatcherTests.java | 6 ++++ .../result/MockMvcResultMatchersTests.java | 36 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java index be00ab8fc49..3c4c4c50f52 100644 --- a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java +++ b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java @@ -169,6 +169,9 @@ public class AntPathMatcher implements PathMatcher { @Override public boolean isPattern(String path) { + if(path == null) { + return false; + } boolean uriVar = false; for (int i = 0; i < path.length(); i++) { char c = path.charAt(i); @@ -207,7 +210,7 @@ public class AntPathMatcher implements PathMatcher { protected boolean doMatch(String pattern, String path, boolean fullMatch, @Nullable Map uriTemplateVariables) { - if (path.startsWith(this.pathSeparator) != pattern.startsWith(this.pathSeparator)) { + if (path == null || path.startsWith(this.pathSeparator) != pattern.startsWith(this.pathSeparator)) { return false; } diff --git a/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java b/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java index 73278dd0997..747de65690a 100644 --- a/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java +++ b/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java @@ -130,6 +130,11 @@ public class AntPathMatcherTests { assertThat(pathMatcher.match("", "")).isTrue(); assertThat(pathMatcher.match("/{bla}.*", "/testing.html")).isTrue(); + + assertThat(pathMatcher.match("/test", null)).isFalse(); + assertThat(pathMatcher.match("/", null)).isFalse(); + assertThat(pathMatcher.match("/", null)).isFalse(); + assertThat(pathMatcher.match(null, null)).isFalse(); } // SPR-14247 @@ -688,6 +693,7 @@ public class AntPathMatcherTests { assertThat(pathMatcher.isPattern("/test/{name}")).isTrue(); assertThat(pathMatcher.isPattern("/test/name")).isFalse(); assertThat(pathMatcher.isPattern("/test/foo{bar")).isFalse(); + assertThat(pathMatcher.isPattern(null)).isFalse(); } } diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/result/MockMvcResultMatchersTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/result/MockMvcResultMatchersTests.java index 688ad49f977..aabb6a864e5 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/result/MockMvcResultMatchersTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/result/MockMvcResultMatchersTests.java @@ -42,6 +42,18 @@ public class MockMvcResultMatchersTests { redirectedUrl("/resource/1").match(getRedirectedUrlStubMvcResult("/resource/1")); } + @Test + public void redirectNonMatching() { + assertThatExceptionOfType(AssertionError.class).isThrownBy(() -> + redirectedUrl("/resource/2").match(getRedirectedUrlStubMvcResult("/resource/1"))); + } + + @Test + public void redirectNonMatchingBecauseNotRedirect() { + assertThatExceptionOfType(AssertionError.class).isThrownBy(() -> + redirectedUrl("/resource/1").match(getForwardedUrlStubMvcResult("/resource/1"))); + } + @Test public void redirectWithUrlTemplate() throws Exception { redirectedUrlTemplate("/orders/{orderId}/items/{itemId}", 1, 2).match(getRedirectedUrlStubMvcResult("/orders/1/items/2")); @@ -58,11 +70,29 @@ public class MockMvcResultMatchersTests { redirectedUrlPattern("/resource/").match(getRedirectedUrlStubMvcResult("/resource/1"))); } + @Test + public void redirectWithNonMatchingPatternBecauseNotRedirect() { + assertThatExceptionOfType(AssertionError.class).isThrownBy(() -> + redirectedUrlPattern("/resource/*").match(getForwardedUrlStubMvcResult("/resource/1"))); + } + @Test public void forward() throws Exception { forwardedUrl("/api/resource/1").match(getForwardedUrlStubMvcResult("/api/resource/1")); } + @Test + public void forwardNonMatching() { + assertThatExceptionOfType(AssertionError.class).isThrownBy(() -> + forwardedUrlPattern("api/resource/2").match(getForwardedUrlStubMvcResult("api/resource/1"))); + } + + @Test + public void forwardNonMatchingBecauseNotForward() { + assertThatExceptionOfType(AssertionError.class).isThrownBy(() -> + forwardedUrlPattern("api/resource/1").match(getRedirectedUrlStubMvcResult("api/resource/1"))); + } + @Test public void forwardWithQueryString() throws Exception { forwardedUrl("/api/resource/1?arg=value").match(getForwardedUrlStubMvcResult("/api/resource/1?arg=value")); @@ -84,6 +114,12 @@ public class MockMvcResultMatchersTests { forwardedUrlPattern("/resource/").match(getForwardedUrlStubMvcResult("/resource/1"))); } + @Test + public void forwardWithNonMatchingPatternBecauseNotForward() { + assertThatExceptionOfType(AssertionError.class).isThrownBy(() -> + forwardedUrlPattern("/resource/*").match(getRedirectedUrlStubMvcResult("/resource/1"))); + } + private StubMvcResult getRedirectedUrlStubMvcResult(String redirectUrl) throws Exception { MockHttpServletResponse response = new MockHttpServletResponse(); response.sendRedirect(redirectUrl);