Use undecoded pathWithinApplication in WebFlux

Introduce pathWithinApplication() in ServerHttpRequest and use it for
request mapping purposes instead of LookupPath.

In turn this means that for request mapping purposes:
1) the path is not decoded
2) suffix pattern matching is not supported

Issue: SPR-15640
This commit is contained in:
Rossen Stoyanchev
2017-06-06 17:44:39 -04:00
parent ff2af660cf
commit 95196e1aee
19 changed files with 109 additions and 411 deletions

View File

@@ -94,8 +94,6 @@ public class SimpleUrlHandlerMappingTests {
testUrl("/anotherTest", mainController, handlerMapping, "anotherTest");
testUrl("/stillAnotherTest", null, handlerMapping, null);
testUrl("outofpattern*ye", null, handlerMapping, null);
testUrl("/test%26t%20est/path%26m%20atching.html", null, handlerMapping, null);
}
private void testUrl(String url, Object bean, HandlerMapping handlerMapping, String pathWithinMapping) {

View File

@@ -16,15 +16,10 @@
package org.springframework.web.reactive.result.condition;
import java.util.Collections;
import java.util.Set;
import org.junit.Test;
import org.springframework.mock.http.server.reactive.test.MockServerWebExchange;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.support.HttpRequestPathHelper;
import org.springframework.web.server.support.LookupPath;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
@@ -82,7 +77,7 @@ public class PatternsRequestConditionTests {
@Test
public void matchDirectPath() throws Exception {
PatternsRequestCondition condition = new PatternsRequestCondition("/foo");
PatternsRequestCondition match = condition.getMatchingCondition(initExchange("/foo"));
PatternsRequestCondition match = condition.getMatchingCondition(get("/foo").toExchange());
assertNotNull(match);
}
@@ -90,7 +85,7 @@ public class PatternsRequestConditionTests {
@Test
public void matchPattern() throws Exception {
PatternsRequestCondition condition = new PatternsRequestCondition("/foo/*");
PatternsRequestCondition match = condition.getMatchingCondition(initExchange("/foo/bar"));
PatternsRequestCondition match = condition.getMatchingCondition(get("/foo/bar").toExchange());
assertNotNull(match);
}
@@ -98,69 +93,15 @@ public class PatternsRequestConditionTests {
@Test
public void matchSortPatterns() throws Exception {
PatternsRequestCondition condition = new PatternsRequestCondition("/*/*", "/foo/bar", "/foo/*");
PatternsRequestCondition match = condition.getMatchingCondition(initExchange("/foo/bar"));
PatternsRequestCondition match = condition.getMatchingCondition(get("/foo/bar").toExchange());
PatternsRequestCondition expected = new PatternsRequestCondition("/foo/bar", "/foo/*", "/*/*");
assertEquals(expected, match);
}
@Test
public void matchSuffixPattern() throws Exception {
ServerWebExchange exchange = initExchange("/foo.html");
PatternsRequestCondition condition = new PatternsRequestCondition("/{foo}");
PatternsRequestCondition match = condition.getMatchingCondition(exchange);
assertNotNull(match);
assertEquals("/{foo}.*", match.getPatterns().iterator().next());
condition = new PatternsRequestCondition(new String[] {"/{foo}"}, null,false, false, null);
match = condition.getMatchingCondition(exchange);
assertNotNull(match);
assertEquals("/{foo}", match.getPatterns().iterator().next());
}
// SPR-8410
@Test
public void matchSuffixPatternUsingFileExtensions() throws Exception {
String[] patterns = new String[] {"/jobs/{jobName}"};
Set<String> extensions = Collections.singleton("json");
PatternsRequestCondition condition = new PatternsRequestCondition(patterns, null, true, false, extensions);
MockServerWebExchange exchange = initExchange("/jobs/my.job");
PatternsRequestCondition match = condition.getMatchingCondition(exchange);
assertNotNull(match);
assertEquals("/jobs/{jobName}", match.getPatterns().iterator().next());
exchange = initExchange("/jobs/my.job.json");
match = condition.getMatchingCondition(exchange);
assertNotNull(match);
assertEquals("/jobs/{jobName}.json", match.getPatterns().iterator().next());
}
@Test
public void matchSuffixPatternUsingFileExtensions2() throws Exception {
PatternsRequestCondition condition1 = new PatternsRequestCondition(
new String[] {"/prefix"}, null, true, false, Collections.singleton("json"));
PatternsRequestCondition condition2 = new PatternsRequestCondition(
new String[] {"/suffix"}, null, true, false, null);
PatternsRequestCondition combined = condition1.combine(condition2);
MockServerWebExchange exchange = initExchange("/prefix/suffix.json");
PatternsRequestCondition match = combined.getMatchingCondition(exchange);
assertNotNull(match);
}
@Test
public void matchTrailingSlash() throws Exception {
MockServerWebExchange exchange = initExchange("/foo/");
MockServerWebExchange exchange = get("/foo/").toExchange();
PatternsRequestCondition condition = new PatternsRequestCondition("/foo");
PatternsRequestCondition match = condition.getMatchingCondition(exchange);
@@ -175,9 +116,8 @@ public class PatternsRequestConditionTests {
assertEquals("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)",
"/foo/", match.getPatterns().iterator().next());
exchange = initExchange("/foo/");
condition = new PatternsRequestCondition(new String[] {"/foo"}, null, false, false, null);
match = condition.getMatchingCondition(exchange);
match = condition.getMatchingCondition(get("/foo/").toExchange());
assertNull(match);
}
@@ -185,7 +125,7 @@ public class PatternsRequestConditionTests {
@Test
public void matchPatternContainsExtension() throws Exception {
PatternsRequestCondition condition = new PatternsRequestCondition("/foo.jpg");
PatternsRequestCondition match = condition.getMatchingCondition(initExchange("/foo.html"));
PatternsRequestCondition match = condition.getMatchingCondition(get("/foo.html").toExchange());
assertNull(match);
}
@@ -195,7 +135,7 @@ public class PatternsRequestConditionTests {
PatternsRequestCondition c1 = new PatternsRequestCondition("/foo*");
PatternsRequestCondition c2 = new PatternsRequestCondition("/foo*");
assertEquals(0, c1.compareTo(c2, initExchange("/foo")));
assertEquals(0, c1.compareTo(c2, get("/foo").toExchange()));
}
@Test
@@ -203,15 +143,15 @@ public class PatternsRequestConditionTests {
PatternsRequestCondition c1 = new PatternsRequestCondition("/fo*");
PatternsRequestCondition c2 = new PatternsRequestCondition("/foo");
assertEquals(1, c1.compareTo(c2, initExchange("/foo")));
assertEquals(1, c1.compareTo(c2, get("/foo").toExchange()));
}
@Test
public void compareNumberOfMatchingPatterns() throws Exception {
ServerWebExchange exchange = initExchange("/foo.html");
ServerWebExchange exchange = get("/foo.html").toExchange();
PatternsRequestCondition c1 = new PatternsRequestCondition("/foo", "*.jpeg");
PatternsRequestCondition c2 = new PatternsRequestCondition("/foo", "*.html");
PatternsRequestCondition c1 = new PatternsRequestCondition("/foo.*", "/foo.jpeg");
PatternsRequestCondition c2 = new PatternsRequestCondition("/foo.*", "/foo.html");
PatternsRequestCondition match1 = c1.getMatchingCondition(exchange);
PatternsRequestCondition match2 = c2.getMatchingCondition(exchange);
@@ -220,10 +160,4 @@ public class PatternsRequestConditionTests {
assertEquals(1, match1.compareTo(match2, exchange));
}
private MockServerWebExchange initExchange(String path) {
MockServerWebExchange exchange = get(path).toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
return exchange;
}
}

View File

@@ -30,8 +30,6 @@ import org.springframework.mock.http.server.reactive.test.MockServerWebExchange;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.reactive.result.method.RequestMappingInfo;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.support.HttpRequestPathHelper;
import org.springframework.web.server.support.LookupPath;
import static java.util.Arrays.asList;
import static org.junit.Assert.assertEquals;
@@ -67,7 +65,6 @@ public class RequestMappingInfoTests {
@Test
public void matchPatternsCondition() {
MockServerWebExchange exchange = MockServerHttpRequest.get("/foo").toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
RequestMappingInfo info = paths("/foo*", "/bar").build();
RequestMappingInfo expected = paths("/foo*").build();
@@ -83,7 +80,6 @@ public class RequestMappingInfoTests {
@Test
public void matchParamsCondition() {
ServerWebExchange exchange = MockServerHttpRequest.get("/foo?foo=bar").toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
RequestMappingInfo info = paths("/foo").params("foo=bar").build();
RequestMappingInfo match = info.getMatchingCondition(exchange);
@@ -99,7 +95,6 @@ public class RequestMappingInfoTests {
@Test
public void matchHeadersCondition() {
ServerWebExchange exchange = MockServerHttpRequest.get("/foo").header("foo", "bar").toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
RequestMappingInfo info = paths("/foo").headers("foo=bar").build();
RequestMappingInfo match = info.getMatchingCondition(exchange);
@@ -115,7 +110,6 @@ public class RequestMappingInfoTests {
@Test
public void matchConsumesCondition() {
ServerWebExchange exchange = MockServerHttpRequest.post("/foo").contentType(MediaType.TEXT_PLAIN).toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
RequestMappingInfo info = paths("/foo").consumes("text/plain").build();
RequestMappingInfo match = info.getMatchingCondition(exchange);
@@ -131,7 +125,6 @@ public class RequestMappingInfoTests {
@Test
public void matchProducesCondition() {
ServerWebExchange exchange = MockServerHttpRequest.get("/foo").accept(MediaType.TEXT_PLAIN).toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
RequestMappingInfo info = paths("/foo").produces("text/plain").build();
RequestMappingInfo match = info.getMatchingCondition(exchange);
@@ -147,8 +140,7 @@ public class RequestMappingInfoTests {
@Test
public void matchCustomCondition() {
ServerWebExchange exchange = MockServerHttpRequest.get("/foo?foo=bar").toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
RequestMappingInfo info = paths("/foo").params("foo=bar").build();
RequestMappingInfo match = info.getMatchingCondition(exchange);
@@ -169,7 +161,6 @@ public class RequestMappingInfoTests {
RequestMappingInfo oneMethodOneParam = paths().methods(RequestMethod.GET).params("foo").build();
ServerWebExchange exchange = MockServerHttpRequest.get("/foo").toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
Comparator<RequestMappingInfo> comparator = (info, otherInfo) -> info.compareTo(otherInfo, exchange);
List<RequestMappingInfo> list = asList(none, oneMethod, oneMethodOneParam);

View File

@@ -47,41 +47,41 @@ import org.springframework.web.method.HandlerMethod;
import org.springframework.web.reactive.BindingContext;
import org.springframework.web.reactive.HandlerMapping;
import org.springframework.web.reactive.HandlerResult;
import org.springframework.web.reactive.result.method.RequestMappingInfo.*;
import org.springframework.web.reactive.result.method.RequestMappingInfo.BuilderConfiguration;
import org.springframework.web.server.MethodNotAllowedException;
import org.springframework.web.server.NotAcceptableStatusException;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.ServerWebInputException;
import org.springframework.web.server.UnsupportedMediaTypeStatusException;
import org.springframework.web.server.support.HttpRequestPathHelper;
import org.springframework.web.server.support.LookupPath;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
import static org.springframework.web.bind.annotation.RequestMethod.*;
import static org.springframework.web.method.MvcAnnotationPredicates.*;
import static org.springframework.web.method.ResolvableMethod.*;
import static org.springframework.web.reactive.result.method.RequestMappingInfo.*;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get;
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.method;
import static org.springframework.web.bind.annotation.RequestMethod.GET;
import static org.springframework.web.bind.annotation.RequestMethod.HEAD;
import static org.springframework.web.bind.annotation.RequestMethod.OPTIONS;
import static org.springframework.web.method.MvcAnnotationPredicates.getMapping;
import static org.springframework.web.method.MvcAnnotationPredicates.requestMapping;
import static org.springframework.web.method.ResolvableMethod.on;
import static org.springframework.web.reactive.result.method.RequestMappingInfo.paths;
/**
* Unit tests for {@link RequestMappingInfoHandlerMapping}.
*
* @author Rossen Stoyanchev
*/
public class RequestMappingInfoHandlerMappingTests {
private TestRequestMappingInfoHandlerMapping handlerMapping;
private HttpRequestPathHelper pathHelper;
@Before
public void setup() throws Exception {
this.handlerMapping = new TestRequestMappingInfoHandlerMapping();
this.handlerMapping.registerHandler(new TestController());
this.pathHelper = new HttpRequestPathHelper();
this.handlerMapping.setPathHelper(this.pathHelper);
}
@@ -174,7 +174,6 @@ public class RequestMappingInfoHandlerMappingTests {
public void getHandlerTestMediaTypeNotAcceptable() throws Exception {
testMediaTypeNotAcceptable("/persons");
testMediaTypeNotAcceptable("/persons/");
testMediaTypeNotAcceptable("/persons.json");
}
@Test // SPR-12854
@@ -214,7 +213,7 @@ public class RequestMappingInfoHandlerMappingTests {
@SuppressWarnings("unchecked")
public void handleMatchUriTemplateVariables() throws Exception {
ServerWebExchange exchange = get("/1/2").toExchange();
LookupPath lookupPath = this.pathHelper.getLookupPathForRequest(exchange);
String lookupPath = exchange.getRequest().getPathWithinApplication();
RequestMappingInfo key = paths("/{path1}/{path2}").build();
this.handlerMapping.handleMatch(key, lookupPath, exchange);
@@ -231,12 +230,9 @@ public class RequestMappingInfoHandlerMappingTests {
public void handleMatchUriTemplateVariablesDecode() throws Exception {
RequestMappingInfo key = paths("/{group}/{identifier}").build();
URI url = URI.create("/group/a%2Fb");
ServerWebExchange exchange = MockServerHttpRequest.method(HttpMethod.GET, url).toExchange();
ServerWebExchange exchange = method(HttpMethod.GET, url).toExchange();
this.pathHelper.setUrlDecode(false);
LookupPath lookupPath = this.pathHelper.getLookupPathForRequest(exchange);
this.handlerMapping.setPathHelper(this.pathHelper);
String lookupPath = exchange.getRequest().getPathWithinApplication();
this.handlerMapping.handleMatch(key, lookupPath, exchange);
String name = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE;
@@ -252,7 +248,7 @@ public class RequestMappingInfoHandlerMappingTests {
public void handleMatchBestMatchingPatternAttribute() throws Exception {
RequestMappingInfo key = paths("/{path1}/2", "/**").build();
ServerWebExchange exchange = get("/1/2").toExchange();
LookupPath lookupPath = this.pathHelper.getLookupPathForRequest(exchange);
String lookupPath = exchange.getRequest().getPathWithinApplication();
this.handlerMapping.handleMatch(key, lookupPath, exchange);
assertEquals("/{path1}/2", exchange.getAttributes().get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE));
@@ -262,7 +258,7 @@ public class RequestMappingInfoHandlerMappingTests {
public void handleMatchBestMatchingPatternAttributeNoPatternsDefined() throws Exception {
RequestMappingInfo key = paths().build();
ServerWebExchange exchange = get("/1/2").toExchange();
LookupPath lookupPath = this.pathHelper.getLookupPathForRequest(exchange);
String lookupPath = exchange.getRequest().getPathWithinApplication();
this.handlerMapping.handleMatch(key, lookupPath, exchange);
assertEquals("/1/2", exchange.getAttributes().get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE));
@@ -309,11 +305,7 @@ public class RequestMappingInfoHandlerMappingTests {
@Test
public void handleMatchMatrixVariablesDecoding() throws Exception {
HttpRequestPathHelper urlPathHelper = new HttpRequestPathHelper();
urlPathHelper.setUrlDecode(false);
this.handlerMapping.setPathHelper(urlPathHelper);
ServerWebExchange exchange = get("/path;mvar=a%2fb").toExchange();
ServerWebExchange exchange = method(HttpMethod.GET, URI.create("/path;mvar=a%2fb")).toExchange();
handleMatch(exchange, "/path{filter}");
MultiValueMap<String, String> matrixVariables = getMatrixVariables(exchange, "filter");
@@ -375,7 +367,7 @@ public class RequestMappingInfoHandlerMappingTests {
private void handleMatch(ServerWebExchange exchange, String pattern) {
RequestMappingInfo info = paths(pattern).build();
LookupPath lookupPath = this.pathHelper.getLookupPathForRequest(exchange);
String lookupPath = exchange.getRequest().getPathWithinApplication();
this.handlerMapping.handleMatch(info, lookupPath, exchange);
}

View File

@@ -55,8 +55,6 @@ import org.springframework.web.reactive.accept.HeaderContentTypeResolver;
import org.springframework.web.reactive.accept.RequestedContentTypeResolver;
import org.springframework.web.server.NotAcceptableStatusException;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.support.HttpRequestPathHelper;
import org.springframework.web.server.support.LookupPath;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertEquals;
@@ -219,17 +217,14 @@ public class ViewResolutionResultHandlerTests {
ViewResolutionResultHandler handler = resultHandler(new TestViewResolver("account"));
MockServerWebExchange exchange = get("/account").toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
handler.handleResult(exchange, result).block(Duration.ofMillis(5000));
assertResponseBody(exchange, "account: {id=123}");
exchange = get("/account/").toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
handler.handleResult(exchange, result).block(Duration.ofMillis(5000));
assertResponseBody(exchange, "account: {id=123}");
exchange = get("/account.123").toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
handler.handleResult(exchange, result).block(Duration.ofMillis(5000));
assertResponseBody(exchange, "account: {id=123}");
}
@@ -256,8 +251,7 @@ public class ViewResolutionResultHandlerTests {
HandlerResult handlerResult = new HandlerResult(new Object(), value, returnType, this.bindingContext);
MockServerWebExchange exchange = get("/account").accept(APPLICATION_JSON).toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
TestView defaultView = new TestView("jsonView", APPLICATION_JSON);
resultHandler(Collections.singletonList(defaultView), new TestViewResolver("account"))
@@ -328,7 +322,6 @@ public class ViewResolutionResultHandlerTests {
model.addAttribute("id", "123");
HandlerResult result = new HandlerResult(new Object(), returnValue, returnType, this.bindingContext);
MockServerWebExchange exchange = get(path).toExchange();
LookupPath.getOrCreate(exchange, new HttpRequestPathHelper());
resultHandler(resolvers).handleResult(exchange, result).block(Duration.ofSeconds(5));
assertResponseBody(exchange, responseBody);
return exchange;

View File

@@ -29,7 +29,6 @@
/reservation.html=mainController
/payment.html=mainController
/confirmation.html=mainController
/test%26t%20est/path%26m%20atching.html=mainController
</value>
</property>
</bean>