Consider Aligning MvcRequestMatcher's matching methods

Closes gh-9284

Author: evgeniycheban

config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -314,6 +314,18 @@ public void getWhenServletPathRoleAdminConfiguredAndRoleIsUserThenRespondsWithFo
 		this.mvc.perform(requestWithUser).andExpect(status().isForbidden());
 	}
 
+	@Test
+	public void getWhenServletPathRoleAdminConfiguredAndRoleIsUserAndWithoutServletPathThenRespondsWithOk()
+			throws Exception {
+		this.spring.register(ServletPathConfig.class, BasicController.class).autowire();
+		// @formatter:off
+		MockHttpServletRequestBuilder requestWithUser = get("/")
+				.with(user("user")
+				.roles("USER"));
+		// @formatter:on
+		this.mvc.perform(requestWithUser).andExpect(status().isOk());
+	}
+
 	@Test
 	public void getWhenServletPathRoleAdminConfiguredAndRoleIsAdminThenRespondsWithOk() throws Exception {
 		this.spring.register(ServletPathConfig.class, BasicController.class).autowire();

web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * Copyright 2012-2021 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.
@@ -43,6 +43,7 @@
  *
  * @author Rob Winch
  * @author Eddú Meléndez
+ * @author Evgeniy Cheban
  * @since 4.1.1
  */
 public class MvcRequestMatcher implements RequestMatcher, RequestVariablesExtractor {
@@ -64,10 +65,7 @@ public MvcRequestMatcher(HandlerMappingIntrospector introspector, String pattern
 
 	@Override
 	public boolean matches(HttpServletRequest request) {
-		if (this.method != null && !this.method.name().equals(request.getMethod())) {
-			return false;
-		}
-		if (this.servletPath != null && !this.servletPath.equals(request.getServletPath())) {
+		if (notMatchMethodOrServletPath(request)) {
 			return false;
 		}
 		MatchableHandlerMapping mapping = getMapping(request);
@@ -95,6 +93,9 @@ public Map<String, String> extractUriTemplateVariables(HttpServletRequest reques
 
 	@Override
 	public MatchResult matcher(HttpServletRequest request) {
+		if (notMatchMethodOrServletPath(request)) {
+			return MatchResult.notMatch();
+		}
 		MatchableHandlerMapping mapping = getMapping(request);
 		if (mapping == null) {
 			return this.defaultMatcher.matcher(request);
@@ -103,6 +104,11 @@ public MatchResult matcher(HttpServletRequest request) {
 		return (result != null) ? MatchResult.match(result.extractUriTemplateVariables()) : MatchResult.notMatch();
 	}
 
+	private boolean notMatchMethodOrServletPath(HttpServletRequest request) {
+		return this.method != null && !this.method.name().equals(request.getMethod())
+				|| this.servletPath != null && !this.servletPath.equals(request.getServletPath());
+	}
+
 	/**
 	 * @param method the method to set
 	 */

web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * Copyright 2012-2021 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.
@@ -41,6 +41,7 @@
 /**
  * @author Rob Winch
  * @author Eddú Meléndez
+ * @author Evgeniy Cheban
  */
 @RunWith(MockitoJUnitRunner.class)
 public class MvcRequestMatcherTests {
@@ -220,4 +221,28 @@ public void toStringWhenOnlyPattern() {
 		assertThat(this.matcher.toString()).isEqualTo("Mvc [pattern='/path']");
 	}
 
+	@Test
+	public void matcherWhenMethodNotMatchesThenNotMatchResult() {
+		this.matcher.setMethod(HttpMethod.POST);
+		assertThat(this.matcher.matcher(this.request).isMatch()).isFalse();
+	}
+
+	@Test
+	public void matcherWhenMethodMatchesThenMatchResult() {
+		this.matcher.setMethod(HttpMethod.GET);
+		assertThat(this.matcher.matcher(this.request).isMatch()).isTrue();
+	}
+
+	@Test
+	public void matcherWhenServletPathNotMatchesThenNotMatchResult() {
+		this.matcher.setServletPath("/spring");
+		assertThat(this.matcher.matcher(this.request).isMatch()).isFalse();
+	}
+
+	@Test
+	public void matcherWhenServletPathMatchesThenMatchResult() {
+		this.matcher.setServletPath("/path");
+		assertThat(this.matcher.matcher(this.request).isMatch()).isTrue();
+	}
+
 }