From 2f6325f651f4ef078f1e15843ae2527e303779b5 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Mon, 3 Dec 2012 15:07:18 -0600 Subject: [PATCH] SEC-2084: AntPathRequestMatcher and RegexpRequestMatcher support request.getMethod() Previously a NullPointerException would occur if an HttpServletRequest.getMethod() returned null. Now AntPathRequestMatcher and RegexpRequestMatcher will handle if the HttpServletRequest.getMethod() returns null. While under normal circumstances, it is unlikely for the method to be null this can occur when using DefaultWebInvocationPrivilegeEvaluator.isAllowed(String, Authentication). --- .../web/util/AntPathRequestMatcher.java | 15 ++++- .../web/util/RegexRequestMatcher.java | 15 ++++- .../web/util/AntPathRequestMatcherTests.java | 52 +++++++++++++++++- .../web/util/RegexRequestMatcherTests.java | 55 +++++++++++++++++++ 4 files changed, 133 insertions(+), 4 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java index 49a4c88fb8..1fdcde2b5d 100644 --- a/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java @@ -1,3 +1,15 @@ +/* + * Copyright 2002-2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ package org.springframework.security.web.util; import javax.servlet.http.HttpServletRequest; @@ -23,6 +35,7 @@ import org.springframework.util.StringUtils; * for this class for comprehensive information on the syntax used. * * @author Luke Taylor + * @author Rob Winch * @since 3.1 * * @see org.springframework.util.AntPathMatcher @@ -80,7 +93,7 @@ public final class AntPathRequestMatcher implements RequestMatcher { * {@code servletPath} + {@code pathInfo} of the request. */ public boolean matches(HttpServletRequest request) { - if (httpMethod != null && httpMethod != HttpMethod.valueOf(request.getMethod())) { + if (httpMethod != null && request.getMethod() != null && httpMethod != HttpMethod.valueOf(request.getMethod())) { if (logger.isDebugEnabled()) { logger.debug("Request '" + request.getMethod() + " " + getRequestPath(request) + "'" + " doesn't match '" + httpMethod + " " + pattern); diff --git a/web/src/main/java/org/springframework/security/web/util/RegexRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/RegexRequestMatcher.java index af5f289be8..3ed660b7a3 100644 --- a/web/src/main/java/org/springframework/security/web/util/RegexRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/RegexRequestMatcher.java @@ -1,3 +1,15 @@ +/* + * Copyright 2002-2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ package org.springframework.security.web.util; import java.util.regex.Pattern; @@ -19,6 +31,7 @@ import org.springframework.util.StringUtils; * argument. * * @author Luke Taylor + * @author Rob Winch * @since 3.1 */ public final class RegexRequestMatcher implements RequestMatcher { @@ -61,7 +74,7 @@ public final class RegexRequestMatcher implements RequestMatcher { * @return true if the pattern matches the URL, false otherwise. */ public boolean matches(HttpServletRequest request) { - if (httpMethod != null && httpMethod != HttpMethod.valueOf(request.getMethod())) { + if (httpMethod != null && request.getMethod() != null && httpMethod != HttpMethod.valueOf(request.getMethod())) { return false; } diff --git a/web/src/test/java/org/springframework/security/web/util/AntPathRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/AntPathRequestMatcherTests.java index 16588d3db8..4adcc7c5b6 100644 --- a/web/src/test/java/org/springframework/security/web/util/AntPathRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/util/AntPathRequestMatcherTests.java @@ -1,12 +1,27 @@ +/* + * Copyright 2002-2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ package org.springframework.security.web.util; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; -import org.junit.*; +import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; /** * @author Luke Taylor + * @author Rob Winch */ public class AntPathRequestMatcherTests { @@ -46,6 +61,39 @@ public class AntPathRequestMatcherTests { assertTrue(matcher.matches(createRequest("/blah/aaa/blah/bbb"))); } + @Test + public void requestHasNullMethodMatches() { + AntPathRequestMatcher matcher = new AntPathRequestMatcher("/something/*", "GET"); + MockHttpServletRequest request = createRequest("/something/here"); + request.setMethod(null); + assertTrue(matcher.matches(request)); + } + + // SEC-2084 + @Test + public void requestHasNullMethodNoMatch() { + AntPathRequestMatcher matcher = new AntPathRequestMatcher("/something/*", "GET"); + MockHttpServletRequest request = createRequest("/nomatch"); + request.setMethod(null); + assertFalse(matcher.matches(request)); + } + + @Test + public void requestHasNullMethodAndNullMatcherMatches() { + AntPathRequestMatcher matcher = new AntPathRequestMatcher("/something/*"); + MockHttpServletRequest request = createRequest("/something/here"); + request.setMethod(null); + assertTrue(matcher.matches(request)); + } + + @Test + public void requestHasNullMethodAndNullMatcherNoMatch() { + AntPathRequestMatcher matcher = new AntPathRequestMatcher("/something/*"); + MockHttpServletRequest request = createRequest("/nomatch"); + request.setMethod(null); + assertFalse(matcher.matches(request)); + } + @Test public void exactMatchOnlyMatchesIdenticalPath() throws Exception { AntPathRequestMatcher matcher = new AntPathRequestMatcher("/login.html"); diff --git a/web/src/test/java/org/springframework/security/web/util/RegexRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/RegexRequestMatcherTests.java index 0099d20bdc..9945f9c3de 100644 --- a/web/src/test/java/org/springframework/security/web/util/RegexRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/util/RegexRequestMatcherTests.java @@ -1,3 +1,15 @@ +/* + * Copyright 2002-2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ package org.springframework.security.web.util; import static org.junit.Assert.*; @@ -7,6 +19,7 @@ import org.springframework.mock.web.MockHttpServletRequest; /** * @author Luke Taylor + * @author Rob Winch */ public class RegexRequestMatcherTests { @@ -40,4 +53,46 @@ public class RegexRequestMatcherTests { assertTrue(matcher.matches(request)); } + + @Test + public void requestHasNullMethodMatches() { + RegexRequestMatcher matcher = new RegexRequestMatcher("/something/.*", "GET"); + MockHttpServletRequest request = createRequest("/something/here"); + request.setMethod(null); + assertTrue(matcher.matches(request)); + } + + // SEC-2084 + @Test + public void requestHasNullMethodNoMatch() { + RegexRequestMatcher matcher = new RegexRequestMatcher("/something/.*", "GET"); + MockHttpServletRequest request = createRequest("/nomatch"); + request.setMethod(null); + assertFalse(matcher.matches(request)); + } + + @Test + public void requestHasNullMethodAndNullMatcherMatches() { + RegexRequestMatcher matcher = new RegexRequestMatcher("/something/.*", null); + MockHttpServletRequest request = createRequest("/something/here"); + request.setMethod(null); + assertTrue(matcher.matches(request)); + } + + @Test + public void requestHasNullMethodAndNullMatcherNoMatch() { + RegexRequestMatcher matcher = new RegexRequestMatcher("/something/.*", null); + MockHttpServletRequest request = createRequest("/nomatch"); + request.setMethod(null); + assertFalse(matcher.matches(request)); + } + + private MockHttpServletRequest createRequest(String path) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setQueryString("doesntMatter"); + request.setServletPath(path); + request.setMethod("POST"); + + return request; + } }