Respond to Feedback

Author: jzheaux

docs/modules/ROOT/pages/migration/web.adoc

@@ -15,8 +15,8 @@ Java::
 [source,java,role="primary"]
 ----
 @Bean
-MethodPatternRequestMatcherFactory requestMatcherFactory() {
-	return PathPatternRequestMatcher.path();
+MethodPatternRequestMatcherFactory requestMatcherFactory(@Qualifier("mvcPatternParser") PathPatternParser parser) {
+	return PathPatternRequestMatcher.withPathPatternParser(parser);
 }
 ----
 
@@ -25,8 +25,8 @@ Kotlin::
 [source,kotlin,role="secondary"]
 ----
 @Bean
-fun requestMatcherFactory(): MethodPatternRequestMatcherFactory {
-    return PathPatternRequestMatcher.path()
+fun requestMatcherFactory(@Qualifier("mvcPatternParser") parser: PathPatternParser): MethodPatternRequestMatcherFactory {
+	return PathPatternRequestMatcher.withPathPatternParser(parser)
 }
 ----
 ======

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

@@ -16,19 +16,15 @@
 
 package org.springframework.security.web.servlet.util.matcher;
 
-import java.nio.charset.StandardCharsets;
 import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicReference;
 
-import jakarta.servlet.RequestDispatcher;
 import jakarta.servlet.ServletContext;
 import jakarta.servlet.ServletRegistration;
-import jakarta.servlet.http.HttpServletMapping;
 import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.MappingMatch;
 
 import org.springframework.http.HttpMethod;
 import org.springframework.http.server.PathContainer;
@@ -39,10 +35,7 @@
 import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.Assert;
-import org.springframework.util.ObjectUtils;
 import org.springframework.web.util.ServletRequestPathUtils;
-import org.springframework.web.util.UriUtils;
-import org.springframework.web.util.WebUtils;
 import org.springframework.web.util.pattern.PathPattern;
 import org.springframework.web.util.pattern.PathPatternParser;
 
@@ -88,9 +81,7 @@ private PathPatternRequestMatcher(PathPattern pattern) {
 	 * prefix
 	 * <p>
 	 * When there is no context path, then these URIs are effectively absolute.
-	 * @return a {@link PathPatternRequestMatcher.Builder} that treats URIs as relative to
-	 * the context path, if any
-	 * @since 6.5
+	 * @return a {@link Builder} that treats URIs as relative to the context path, if any
 	 */
 	public static Builder path() {
 		return new Builder();
@@ -109,17 +100,25 @@ public static Builder path() {
 	 * {@link HttpServletRequest#getServletPath()} would return {@code /path} and so
 	 * {@code /path} is what is specified here.
 	 *
+	 * <p>
 	 * Specify the path here without the trailing {@code /*}.
-	 * @return a {@link PathPatternRequestMatcher.Builder} that treats URIs as relative to
-	 * the given {@code servletPath}
-	 * @since 6.5
+	 * @return a {@link Builder} that treats URIs as relative to the given
+	 * {@code servletPath}
 	 */
 	public static Builder servletPath(String servletPath) {
-		Assert.notNull(servletPath, "servletPath cannot be null");
-		Assert.isTrue(servletPath.startsWith("/"), "servletPath must start with '/'");
-		Assert.isTrue(!servletPath.endsWith("/"), "servletPath must not end with a slash");
-		Assert.isTrue(!servletPath.contains("*"), "servletPath must not contain a star");
-		return new Builder(new ServletPathRequestMatcher(servletPath));
+		return new Builder().servletPath(servletPath);
+	}
+
+	/**
+	 * Use this {@link PathPatternParser} to parse path patterns. Uses
+	 * {@link PathPatternParser#defaultInstance} by default.
+	 * @param parser the {@link PathPatternParser} to use
+	 * @return a {@link Builder} that treats URIs as relative to the given
+	 * {@code servletPath}
+	 */
+	public static Builder withPathPatternParser(PathPatternParser parser) {
+		Assert.notNull(parser, "pathPatternParser cannot be null");
+		return new Builder(parser);
 	}
 
 	/**
@@ -212,16 +211,27 @@ public String toString() {
 	 */
 	public static final class Builder implements MethodPatternRequestMatcherFactory {
 
-		private PathPatternParser parser = PathPatternParser.defaultInstance;
+		private final PathPatternParser parser;
 
-		private final RequestMatcher servletPath;
+		private RequestMatcher servletPath = AnyRequestMatcher.INSTANCE;
 
-		Builder() {
-			this(AnyRequestMatcher.INSTANCE);
+		private Builder() {
+			this.parser = PathPatternParser.defaultInstance;
 		}
 
-		Builder(RequestMatcher servletPath) {
-			this.servletPath = servletPath;
+		private Builder(PathPatternParser parser) {
+			this.parser = parser;
+		}
+
+		/**
+		 * Match requests starting with this {@code servletPath}.
+		 * @param servletPath the servlet path prefix
+		 * @see PathPatternRequestMatcher#servletPath
+		 * @return the {@link Builder} for more configuration
+		 */
+		public Builder servletPath(String servletPath) {
+			this.servletPath = new ServletPathRequestMatcher(servletPath);
+			return this;
 		}
 
 		/**
@@ -298,14 +308,18 @@ private static final class ServletPathRequestMatcher implements RequestMatcher {
 		private final AtomicReference<Boolean> servletExists = new AtomicReference<>();
 
 		ServletPathRequestMatcher(String servletPath) {
+			Assert.notNull(servletPath, "servletPath cannot be null");
+			Assert.isTrue(servletPath.startsWith("/"), "servletPath must start with '/'");
+			Assert.isTrue(!servletPath.endsWith("/"), "servletPath must not end with a slash");
+			Assert.isTrue(!servletPath.contains("*"), "servletPath must not contain a star");
 			this.path = servletPath;
 		}
 
 		@Override
 		public boolean matches(HttpServletRequest request) {
 			Assert.isTrue(servletExists(request), () -> this.path + "/* does not exist in your servlet registration "
 					+ registrationMappings(request));
-			return Objects.equals(this.path, getServletPathPrefix(request));
+			return Objects.equals(this.path, ServletRequestPathUtils.getServletPathPrefix(request));
 		}
 
 		private boolean servletExists(HttpServletRequest request) {
@@ -336,20 +350,6 @@ private Map<String, Collection<String>> registrationMappings(HttpServletRequest
 			return map;
 		}
 
-		@Nullable
-		private static String getServletPathPrefix(HttpServletRequest request) {
-			HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING);
-			mapping = (mapping != null) ? mapping : request.getHttpServletMapping();
-			if (ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) {
-				String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE);
-				servletPath = (servletPath != null) ? servletPath : request.getServletPath();
-				servletPath = servletPath.endsWith("/") ? servletPath.substring(0, servletPath.length() - 1)
-						: servletPath;
-				return UriUtils.encodePath(servletPath, StandardCharsets.UTF_8);
-			}
-			return null;
-		}
-
 		@Override
 		public String toString() {
 			return "ServletPath [" + this.path + "]";

web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java

@@ -17,6 +17,7 @@
 package org.springframework.security.web.util.matcher;
 
 import org.springframework.http.HttpMethod;
+import org.springframework.lang.Nullable;
 
 /**
  * A strategy for constructing request matchers that match method-path pairs
@@ -26,10 +27,26 @@
  */
 public interface MethodPatternRequestMatcherFactory {
 
+	/**
+	 * Request a method-pattern request matcher given the following {{@code method} and
+	 * {@code pattern}.
+	 * This method in this case is treated as a wildcard.
+	 *
+	 * @param pattern the path pattern to use
+	 * @return the {@link RequestMatcher}
+	 */
 	default RequestMatcher matcher(String pattern) {
 		return matcher(null, pattern);
 	}
 
-	RequestMatcher matcher(HttpMethod method, String pattern);
+	/**
+	 * Request a method-pattern request matcher given the following
+	 * {@code method} and {@code pattern}.
+	 *
+	 * @param method the method to use, may be null
+	 * @param pattern the path pattern to use
+	 * @return the {@link RequestMatcher}
+	 */
+	RequestMatcher matcher(@Nullable HttpMethod method, String pattern);
 
 }