Removal of ParsingPathMatcher

Changes needed for the removal of ParsingPathMatcher in Spring Framework

b1440b6816 (diff-972650c759c249004b9725f94b570db3R156)
This commit is contained in:
Rob Winch 2017-08-02 11:11:11 -05:00
parent 0f0563cd6f
commit bfaead6f68
5 changed files with 58 additions and 59 deletions

View File

@ -17,6 +17,7 @@ package org.springframework.security.config.web.server;
import org.springframework.http.HttpMethod; import org.springframework.http.HttpMethod;
import org.springframework.security.web.server.util.matcher.OrServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.OrServerWebExchangeMatcher;
import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher;
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatchers; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatchers;
@ -40,7 +41,7 @@ abstract class AbstractServerWebExchangeMatcherRegistry<T> {
/** /**
* Maps a {@link List} of * Maps a {@link List} of
* {@link org.springframework.security.web.server.util.matcher.PathMatcherServerWebExchangeMatcher} * {@link PathPatternParserServerWebExchangeMatcher}
* instances. * instances.
* *
* @param method the {@link HttpMethod} to use for any * @param method the {@link HttpMethod} to use for any
@ -54,13 +55,13 @@ abstract class AbstractServerWebExchangeMatcherRegistry<T> {
/** /**
* Maps a {@link List} of * Maps a {@link List} of
* {@link org.springframework.security.web.server.util.matcher.PathMatcherServerWebExchangeMatcher} * {@link PathPatternParserServerWebExchangeMatcher}
* instances. * instances.
* *
* @param method the {@link HttpMethod} to use or {@code null} for any * @param method the {@link HttpMethod} to use or {@code null} for any
* {@link HttpMethod}. * {@link HttpMethod}.
* @param antPatterns the ant patterns to create. If {@code null} or empty, then matches on nothing. * @param antPatterns the ant patterns to create. If {@code null} or empty, then matches on nothing.
* {@link org.springframework.security.web.server.util.matcher.PathMatcherServerWebExchangeMatcher} from * {@link PathPatternParserServerWebExchangeMatcher} from
* *
* @return the object that is chained after creating the {@link ServerWebExchangeMatcher} * @return the object that is chained after creating the {@link ServerWebExchangeMatcher}
*/ */
@ -70,11 +71,11 @@ abstract class AbstractServerWebExchangeMatcherRegistry<T> {
/** /**
* Maps a {@link List} of * Maps a {@link List} of
* {@link org.springframework.security.web.server.util.matcher.PathMatcherServerWebExchangeMatcher} * {@link PathPatternParserServerWebExchangeMatcher}
* instances that do not care which {@link HttpMethod} is used. * instances that do not care which {@link HttpMethod} is used.
* *
* @param antPatterns the ant patterns to create * @param antPatterns the ant patterns to create
* {@link org.springframework.security.web.server.util.matcher.PathMatcherServerWebExchangeMatcher} from * {@link PathPatternParserServerWebExchangeMatcher} from
* *
* @return the object that is chained after creating the {@link ServerWebExchangeMatcher} * @return the object that is chained after creating the {@link ServerWebExchangeMatcher}
*/ */

View File

@ -32,8 +32,7 @@ import org.springframework.security.core.userdetails.UserDetailsRepository;
import org.springframework.security.test.web.reactive.server.WebTestClientBuilder; import org.springframework.security.test.web.reactive.server.WebTestClientBuilder;
import org.springframework.security.web.server.SecurityWebFilterChain; import org.springframework.security.web.server.SecurityWebFilterChain;
import org.springframework.security.web.server.WebFilterChainFilter; import org.springframework.security.web.server.WebFilterChainFilter;
import org.springframework.security.web.server.util.matcher.PathMatcherServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher;
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.context.junit4.SpringRunner;
import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.test.web.reactive.server.WebTestClient;
@ -101,7 +100,7 @@ public class EnableWebFluxSecurityTests {
@Bean @Bean
public SecurityWebFilterChain apiHttpSecurity(HttpSecurity http) { public SecurityWebFilterChain apiHttpSecurity(HttpSecurity http) {
http http
.securityMatcher(new PathMatcherServerWebExchangeMatcher("/api/**")) .securityMatcher(new PathPatternParserServerWebExchangeMatcher("/api/**"))
.authorizeExchange() .authorizeExchange()
.anyExchange().denyAll(); .anyExchange().denyAll();
return http.build(); return http.build();

View File

@ -17,60 +17,64 @@
*/ */
package org.springframework.security.web.server.util.matcher; package org.springframework.security.web.server.util.matcher;
import java.util.HashMap;
import java.util.Map;
import org.springframework.http.HttpMethod; import org.springframework.http.HttpMethod;
import org.springframework.http.server.PathContainer;
import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.PathMatcher;
import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.util.pattern.ParsingPathMatcher; import org.springframework.web.util.pattern.PathPattern;
import org.springframework.web.util.pattern.PathPatternParser;
import reactor.core.publisher.Mono; import reactor.core.publisher.Mono;
import java.util.HashMap;
import java.util.Map;
/** /**
* @author Rob Winch * @author Rob Winch
* @since 5.0 * @since 5.0
*/ */
public final class PathMatcherServerWebExchangeMatcher implements ServerWebExchangeMatcher { public final class PathPatternParserServerWebExchangeMatcher implements ServerWebExchangeMatcher {
private static final PathMatcher DEFAULT_PATH_MATCHER = new ParsingPathMatcher(); private static final PathPatternParser DEFAULT_PATTERN_PARSER = new PathPatternParser();
private PathMatcher pathMatcher = DEFAULT_PATH_MATCHER; private final PathPattern pattern;
private final String pattern;
private final HttpMethod method; private final HttpMethod method;
public PathMatcherServerWebExchangeMatcher(String pattern) { public PathPatternParserServerWebExchangeMatcher(PathPattern pattern) {
this(pattern, null); this(pattern, null);
} }
public PathMatcherServerWebExchangeMatcher(String pattern, HttpMethod method) { public PathPatternParserServerWebExchangeMatcher(PathPattern pattern, HttpMethod method) {
Assert.notNull(pattern, "pattern cannot be null"); Assert.notNull(pattern, "pattern cannot be null");
this.pattern = pattern; this.pattern = pattern;
this.method = method; this.method = method;
} }
public PathPatternParserServerWebExchangeMatcher(String pattern, HttpMethod method) {
Assert.notNull(pattern, "pattern cannot be null");
this.pattern = DEFAULT_PATTERN_PARSER.parse(pattern);
this.method = method;
}
public PathPatternParserServerWebExchangeMatcher(String pattern) {
this(pattern, null);
}
@Override @Override
public Mono<MatchResult> matches(ServerWebExchange exchange) { public Mono<MatchResult> matches(ServerWebExchange exchange) {
ServerHttpRequest request = exchange.getRequest(); ServerHttpRequest request = exchange.getRequest();
if(this.method != null && !this.method.equals(request.getMethod())) { if(this.method != null && !this.method.equals(request.getMethod())) {
return MatchResult.notMatch(); return MatchResult.notMatch();
} }
String path = request.getPath().pathWithinApplication().value(); PathContainer path = request.getPath().pathWithinApplication();
boolean match = pathMatcher.match(pattern, path); boolean match = this.pattern.matches(path);
if(!match) { if(!match) {
return MatchResult.notMatch(); return MatchResult.notMatch();
} }
Map<String,String> pathVariables = pathMatcher.extractUriTemplateVariables(pattern, path); Map<String,String> pathVariables = this.pattern.matchAndExtract(path).getUriVariables();
Map<String,Object> variables = new HashMap<>(pathVariables); Map<String,Object> variables = new HashMap<>(pathVariables);
return MatchResult.match(variables); return MatchResult.match(variables);
} }
public void setPathMatcher(PathMatcher pathMatcher) {
Assert.notNull(pathMatcher, "pathMatcher cannot be null");
this.pathMatcher = pathMatcher;
}
@Override @Override
public String toString() { public String toString() {
return "PathMatcherServerWebExchangeMatcher{" + return "PathMatcherServerWebExchangeMatcher{" +

View File

@ -33,7 +33,7 @@ public abstract class ServerWebExchangeMatchers {
public static ServerWebExchangeMatcher pathMatchers(HttpMethod method, String... patterns) { public static ServerWebExchangeMatcher pathMatchers(HttpMethod method, String... patterns) {
List<ServerWebExchangeMatcher> matchers = new ArrayList<>(patterns.length); List<ServerWebExchangeMatcher> matchers = new ArrayList<>(patterns.length);
for (String pattern : patterns) { for (String pattern : patterns) {
matchers.add(new PathMatcherServerWebExchangeMatcher(pattern, method)); matchers.add(new PathPatternParserServerWebExchangeMatcher(pattern, method));
} }
return new OrServerWebExchangeMatcher(matchers); return new OrServerWebExchangeMatcher(matchers);
} }

View File

@ -17,11 +17,6 @@
*/ */
package org.springframework.security.web.server.util.matcher; package org.springframework.security.web.server.util.matcher;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
@ -31,9 +26,15 @@ import org.springframework.http.HttpMethod;
import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
import org.springframework.mock.http.server.reactive.MockServerHttpResponse; import org.springframework.mock.http.server.reactive.MockServerHttpResponse;
import org.springframework.mock.http.server.reactive.MockServerWebExchange; import org.springframework.mock.http.server.reactive.MockServerWebExchange;
import org.springframework.util.PathMatcher;
import org.springframework.web.server.adapter.DefaultServerWebExchange;
import org.springframework.web.server.session.DefaultWebSessionManager; import org.springframework.web.server.session.DefaultWebSessionManager;
import org.springframework.web.util.pattern.PathPattern;
import java.util.HashMap;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
/** /**
* @author Rob Winch * @author Rob Winch
@ -42,10 +43,11 @@ import org.springframework.web.server.session.DefaultWebSessionManager;
@RunWith(MockitoJUnitRunner.class) @RunWith(MockitoJUnitRunner.class)
public class PathMatcherServerWebExchangeMatcherTests { public class PathMatcherServerWebExchangeMatcherTests {
@Mock @Mock
PathMatcher pathMatcher; PathPattern pattern;
@Mock
PathPattern.PathMatchResult pathMatchResult;
MockServerWebExchange exchange; MockServerWebExchange exchange;
PathMatcherServerWebExchangeMatcher matcher; PathPatternParserServerWebExchangeMatcher matcher;
String pattern;
String path; String path;
@Before @Before
@ -54,44 +56,43 @@ public class PathMatcherServerWebExchangeMatcherTests {
MockServerHttpResponse response = new MockServerHttpResponse(); MockServerHttpResponse response = new MockServerHttpResponse();
DefaultWebSessionManager sessionManager = new DefaultWebSessionManager(); DefaultWebSessionManager sessionManager = new DefaultWebSessionManager();
exchange = request.toExchange(); exchange = request.toExchange();
pattern = "/pattern";
path = "/path"; path = "/path";
matcher = new PathMatcherServerWebExchangeMatcher(pattern); matcher = new PathPatternParserServerWebExchangeMatcher(pattern);
matcher.setPathMatcher(pathMatcher);
} }
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void constructorPatternWhenPatternNullThenThrowsException() { public void constructorPatternWhenPatternNullThenThrowsException() {
new PathMatcherServerWebExchangeMatcher(null); new PathPatternParserServerWebExchangeMatcher((PathPattern) null);
} }
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void constructorPatternAndMethodWhenPatternNullThenThrowsException() { public void constructorPatternAndMethodWhenPatternNullThenThrowsException() {
new PathMatcherServerWebExchangeMatcher(null, HttpMethod.GET); new PathPatternParserServerWebExchangeMatcher((PathPattern) null, HttpMethod.GET);
} }
@Test @Test
public void matchesWhenPathMatcherTrueThenReturnTrue() { public void matchesWhenPathMatcherTrueThenReturnTrue() {
when(pathMatcher.match(pattern, path)).thenReturn(true); when(pattern.matches(any())).thenReturn(true);
when(pattern.matchAndExtract(any())).thenReturn(pathMatchResult);
when(pathMatchResult.getUriVariables()).thenReturn(new HashMap<>());
assertThat(matcher.matches(exchange).block().isMatch()).isTrue(); assertThat(matcher.matches(exchange).block().isMatch()).isTrue();
} }
@Test @Test
public void matchesWhenPathMatcherFalseThenReturnFalse() { public void matchesWhenPathMatcherFalseThenReturnFalse() {
when(pathMatcher.match(pattern, path)).thenReturn(false); when(pattern.matches(any())).thenReturn(false);
assertThat(matcher.matches(exchange).block().isMatch()).isFalse(); assertThat(matcher.matches(exchange).block().isMatch()).isFalse();
verify(pathMatcher).match(pattern, path);
} }
@Test @Test
public void matchesWhenPathMatcherTrueAndMethodTrueThenReturnTrue() { public void matchesWhenPathMatcherTrueAndMethodTrueThenReturnTrue() {
matcher = new PathMatcherServerWebExchangeMatcher(pattern, exchange.getRequest().getMethod()); matcher = new PathPatternParserServerWebExchangeMatcher(pattern, exchange.getRequest().getMethod());
matcher.setPathMatcher(pathMatcher); when(pattern.matches(any())).thenReturn(true);
when(pathMatcher.match(pattern, path)).thenReturn(true); when(pattern.matchAndExtract(any())).thenReturn(pathMatchResult);
when(pathMatchResult.getUriVariables()).thenReturn(new HashMap<>());
assertThat(matcher.matches(exchange).block().isMatch()).isTrue(); assertThat(matcher.matches(exchange).block().isMatch()).isTrue();
} }
@ -100,16 +101,10 @@ public class PathMatcherServerWebExchangeMatcherTests {
public void matchesWhenPathMatcherTrueAndMethodFalseThenReturnFalse() { public void matchesWhenPathMatcherTrueAndMethodFalseThenReturnFalse() {
HttpMethod method = HttpMethod.OPTIONS; HttpMethod method = HttpMethod.OPTIONS;
assertThat(exchange.getRequest().getMethod()).isNotEqualTo(method); assertThat(exchange.getRequest().getMethod()).isNotEqualTo(method);
matcher = new PathMatcherServerWebExchangeMatcher(pattern, method); matcher = new PathPatternParserServerWebExchangeMatcher(pattern, method);
matcher.setPathMatcher(pathMatcher);
assertThat(matcher.matches(exchange).block().isMatch()).isFalse(); assertThat(matcher.matches(exchange).block().isMatch()).isFalse();
verifyZeroInteractions(pathMatcher); verifyZeroInteractions(pattern);
}
@Test(expected = IllegalArgumentException.class)
public void setPathMatcherWhenNullThenThrowException() {
matcher.setPathMatcher(null);
} }
} }