From 85e8634810ea7f5dabd1683cfd7da9495deffbd5 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 24 May 2018 15:08:39 -0400 Subject: [PATCH] Properly initialize URI/Matrix vars w/ urlDecode=false Issue: SPR-16867 --- ...RequestMappingInfoHandlerMappingTests.java | 54 +++++++++---------- .../RequestMappingInfoHandlerMapping.java | 7 ++- ...RequestMappingInfoHandlerMappingTests.java | 8 +-- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java index 1bd343e68b..af1f5443ee 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -91,14 +91,14 @@ public class RequestMappingInfoHandlerMappingTests { @Before - public void setup() throws Exception { + public void setup() { this.handlerMapping = new TestRequestMappingInfoHandlerMapping(); this.handlerMapping.registerHandler(new TestController()); } @Test - public void getHandlerDirectMatch() throws Exception { + public void getHandlerDirectMatch() { Method expected = on(TestController.class).annot(getMapping("/foo").params()).resolveMethod(); ServerWebExchange exchange = MockServerWebExchange.from(get("/foo")); HandlerMethod hm = (HandlerMethod) this.handlerMapping.getHandler(exchange).block(); @@ -107,7 +107,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void getHandlerGlobMatch() throws Exception { + public void getHandlerGlobMatch() { Method expected = on(TestController.class).annot(requestMapping("/ba*").method(GET, HEAD)).resolveMethod(); ServerWebExchange exchange = MockServerWebExchange.from(get("/bar")); HandlerMethod hm = (HandlerMethod) this.handlerMapping.getHandler(exchange).block(); @@ -116,7 +116,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void getHandlerEmptyPathMatch() throws Exception { + public void getHandlerEmptyPathMatch() { Method expected = on(TestController.class).annot(requestMapping("")).resolveMethod(); ServerWebExchange exchange = MockServerWebExchange.from(get("")); HandlerMethod hm = (HandlerMethod) this.handlerMapping.getHandler(exchange).block(); @@ -128,7 +128,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void getHandlerBestMatch() throws Exception { + public void getHandlerBestMatch() { Method expected = on(TestController.class).annot(getMapping("/foo").params("p")).resolveMethod(); ServerWebExchange exchange = MockServerWebExchange.from(get("/foo?p=anything")); HandlerMethod hm = (HandlerMethod) this.handlerMapping.getHandler(exchange).block(); @@ -137,7 +137,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void getHandlerRequestMethodNotAllowed() throws Exception { + public void getHandlerRequestMethodNotAllowed() { ServerWebExchange exchange = MockServerWebExchange.from(post("/bar")); Mono mono = this.handlerMapping.getHandler(exchange); @@ -146,7 +146,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test // SPR-9603 - public void getHandlerRequestMethodMatchFalsePositive() throws Exception { + public void getHandlerRequestMethodMatchFalsePositive() { ServerWebExchange exchange = MockServerWebExchange.from(get("/users").accept(MediaType.APPLICATION_XML)); this.handlerMapping.registerHandler(new UserController()); Mono mono = this.handlerMapping.getHandler(exchange); @@ -157,14 +157,14 @@ public class RequestMappingInfoHandlerMappingTests { } @Test // SPR-8462 - public void getHandlerMediaTypeNotSupported() throws Exception { + public void getHandlerMediaTypeNotSupported() { testHttpMediaTypeNotSupportedException("/person/1"); testHttpMediaTypeNotSupportedException("/person/1/"); testHttpMediaTypeNotSupportedException("/person/1.json"); } @Test - public void getHandlerTestInvalidContentType() throws Exception { + public void getHandlerTestInvalidContentType() { MockServerHttpRequest request = put("/person/1").header("content-type", "bogus").build(); ServerWebExchange exchange = MockServerWebExchange.from(request); Mono mono = this.handlerMapping.getHandler(exchange); @@ -175,13 +175,13 @@ public class RequestMappingInfoHandlerMappingTests { } @Test // SPR-8462 - public void getHandlerTestMediaTypeNotAcceptable() throws Exception { + public void getHandlerTestMediaTypeNotAcceptable() { testMediaTypeNotAcceptable("/persons"); testMediaTypeNotAcceptable("/persons/"); } @Test // SPR-12854 - public void getHandlerTestRequestParamMismatch() throws Exception { + public void getHandlerTestRequestParamMismatch() { ServerWebExchange exchange = MockServerWebExchange.from(get("/params")); Mono mono = this.handlerMapping.getHandler(exchange); assertError(mono, ServerWebInputException.class, ex -> { @@ -191,7 +191,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void getHandlerHttpOptions() throws Exception { + public void getHandlerHttpOptions() { List allMethodExceptTrace = new ArrayList<>(Arrays.asList(HttpMethod.values())); allMethodExceptTrace.remove(HttpMethod.TRACE); @@ -202,7 +202,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void getHandlerProducibleMediaTypesAttribute() throws Exception { + public void getHandlerProducibleMediaTypesAttribute() { ServerWebExchange exchange = MockServerWebExchange.from(get("/content").accept(MediaType.APPLICATION_XML)); this.handlerMapping.getHandler(exchange).block(); @@ -218,7 +218,7 @@ public class RequestMappingInfoHandlerMappingTests { @Test @SuppressWarnings("unchecked") - public void handleMatchUriTemplateVariables() throws Exception { + public void handleMatchUriTemplateVariables() { ServerWebExchange exchange = MockServerWebExchange.from(get("/1/2")); RequestMappingInfo key = paths("/{path1}/{path2}").build(); @@ -233,7 +233,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test // SPR-9098 - public void handleMatchUriTemplateVariablesDecode() throws Exception { + public void handleMatchUriTemplateVariablesDecode() { RequestMappingInfo key = paths("/{group}/{identifier}").build(); URI url = URI.create("/group/a%2Fb"); ServerWebExchange exchange = MockServerWebExchange.from(method(HttpMethod.GET, url)); @@ -250,7 +250,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void handleMatchBestMatchingPatternAttribute() throws Exception { + public void handleMatchBestMatchingPatternAttribute() { RequestMappingInfo key = paths("/{path1}/2", "/**").build(); ServerWebExchange exchange = MockServerWebExchange.from(get("/1/2")); this.handlerMapping.handleMatch(key, handlerMethod, exchange); @@ -263,7 +263,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void handleMatchBestMatchingPatternAttributeNoPatternsDefined() throws Exception { + public void handleMatchBestMatchingPatternAttributeNoPatternsDefined() { RequestMappingInfo key = paths().build(); ServerWebExchange exchange = MockServerWebExchange.from(get("/1/2")); this.handlerMapping.handleMatch(key, handlerMethod, exchange); @@ -273,7 +273,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void handleMatchMatrixVariables() throws Exception { + public void handleMatchMatrixVariables() { MultiValueMap matrixVariables; Map uriVariables; @@ -290,17 +290,17 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - public void handleMatchMatrixVariablesDecoding() throws Exception { - MockServerHttpRequest request = method(HttpMethod.GET, URI.create("/path;mvar=a%2fb")).build(); + public void handleMatchMatrixVariablesDecoding() { + MockServerHttpRequest request = method(HttpMethod.GET, URI.create("/cars;mvar=a%2Fb")).build(); ServerWebExchange exchange = MockServerWebExchange.from(request); - handleMatch(exchange, "/{filter}"); + handleMatch(exchange, "/{cars}"); - MultiValueMap matrixVariables = getMatrixVariables(exchange, "filter"); + MultiValueMap matrixVariables = getMatrixVariables(exchange, "cars"); Map uriVariables = getUriTemplateVariables(exchange); assertNotNull(matrixVariables); assertEquals(Collections.singletonList("a/b"), matrixVariables.get("mvar")); - assertEquals("path", uriVariables.get("filter")); + assertEquals("cars", uriVariables.get("cars")); } @@ -314,7 +314,7 @@ public class RequestMappingInfoHandlerMappingTests { .verify(); } - private void testHttpMediaTypeNotSupportedException(String url) throws Exception { + private void testHttpMediaTypeNotSupportedException(String url) { MockServerHttpRequest request = put(url).contentType(MediaType.APPLICATION_JSON).build(); ServerWebExchange exchange = MockServerWebExchange.from(request); Mono mono = this.handlerMapping.getHandler(exchange); @@ -325,7 +325,7 @@ public class RequestMappingInfoHandlerMappingTests { ex.getSupportedMediaTypes())); } - private void testHttpOptions(String requestURI, Set allowedMethods) throws Exception { + private void testHttpOptions(String requestURI, Set allowedMethods) { ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.options(requestURI)); HandlerMethod handlerMethod = (HandlerMethod) this.handlerMapping.getHandler(exchange).block(); @@ -342,7 +342,7 @@ public class RequestMappingInfoHandlerMappingTests { assertEquals(allowedMethods, ((HttpHeaders) value).getAllow()); } - private void testMediaTypeNotAcceptable(String url) throws Exception { + private void testMediaTypeNotAcceptable(String url) { ServerWebExchange exchange = MockServerWebExchange.from(get(url).accept(MediaType.APPLICATION_JSON)); Mono mono = this.handlerMapping.getHandler(exchange); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java index 50dad4c2f5..2f04e56b98 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java @@ -113,28 +113,27 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe String bestPattern; Map uriVariables; - Map decodedUriVariables; Set patterns = info.getPatternsCondition().getPatterns(); if (patterns.isEmpty()) { bestPattern = lookupPath; uriVariables = Collections.emptyMap(); - decodedUriVariables = Collections.emptyMap(); } else { bestPattern = patterns.iterator().next(); uriVariables = getPathMatcher().extractUriTemplateVariables(bestPattern, lookupPath); - decodedUriVariables = getUrlPathHelper().decodePathVariables(request, uriVariables); } request.setAttribute(BEST_MATCHING_PATTERN_ATTRIBUTE, bestPattern); - request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, decodedUriVariables); if (isMatrixVariableContentAvailable()) { Map> matrixVars = extractMatrixVariables(request, uriVariables); request.setAttribute(HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE, matrixVars); } + Map decodedUriVariables = getUrlPathHelper().decodePathVariables(request, uriVariables); + request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, decodedUriVariables); + if (!info.getProducesCondition().getProducibleMediaTypes().isEmpty()) { Set mediaTypes = info.getProducesCondition().getProducibleMediaTypes(); request.setAttribute(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, mediaTypes); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java index 4a19f6b4fb..4f6ae1a1be 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java @@ -345,7 +345,7 @@ public class RequestMappingInfoHandlerMappingTests { assertEquals("", uriVariables.get("params")); } - @Test + @Test // SPR-10140, SPR-16867 public void handleMatchMatrixVariablesDecoding() { MockHttpServletRequest request; @@ -357,14 +357,14 @@ public class RequestMappingInfoHandlerMappingTests { this.handlerMapping.setUrlPathHelper(urlPathHelper); request = new MockHttpServletRequest(); - handleMatch(request, "/path{filter}", "/path;mvar=a%2fb"); + handleMatch(request, "/{cars}", "/cars;mvar=a%2Fb"); - MultiValueMap matrixVariables = getMatrixVariables(request, "filter"); + MultiValueMap matrixVariables = getMatrixVariables(request, "cars"); Map uriVariables = getUriTemplateVariables(request); assertNotNull(matrixVariables); assertEquals(Collections.singletonList("a/b"), matrixVariables.get("mvar")); - assertEquals(";mvar=a/b", uriVariables.get("filter")); + assertEquals("cars", uriVariables.get("cars")); }