SPR-8646 Encode URI template variables in the target URL of RedirectView.

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java

@@ -19,23 +19,22 @@ package org.springframework.web.servlet.view;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Array;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.net.URLEncoder;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.beans.BeanUtils;
 import org.springframework.http.HttpStatus;
+import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
@@ -44,7 +43,7 @@ import org.springframework.web.servlet.HandlerMapping;
 import org.springframework.web.servlet.SmartView;
 import org.springframework.web.servlet.View;
 import org.springframework.web.servlet.support.RequestContextUtils;
-import org.springframework.web.util.UriTemplate;
+import org.springframework.web.util.UriUtils;
 import org.springframework.web.util.WebUtils;
 
 /**
@@ -89,6 +88,8 @@ import org.springframework.web.util.WebUtils;
  */
 public class RedirectView extends AbstractUrlBasedView implements SmartView {
 
+	private static final Pattern URI_TEMPLATE_VARIABLE_PATTERN = Pattern.compile("\\{([^/]+?)\\}");
+
 	private boolean contextRelative = false;
 
 	private boolean http10Compatible = true;
@@ -234,7 +235,18 @@ public class RedirectView extends AbstractUrlBasedView implements SmartView {
 	protected void renderMergedOutputModel(
 			Map<String, Object> model, HttpServletRequest request, HttpServletResponse response)
 			throws IOException {
+
 		String targetUrl = createTargetUrl(model, request);
+
+		FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request);
+		if (!CollectionUtils.isEmpty(flashMap)) {
+			String targetPath = WebUtils.extractUrlPath(targetUrl.toString());
+			flashMap.setTargetRequestPath(targetPath);
+			if (this.exposeModelAttributes) {
+				flashMap.addTargetRequestParams(model);
+			}				
+		}
+		
 		sendRedirect(request, response, targetUrl.toString(), this.http10Compatible);
 	}
 
@@ -263,65 +275,53 @@ public class RedirectView extends AbstractUrlBasedView implements SmartView {
 		}
 
 		if (StringUtils.hasText(targetUrl)) {
-			UriTemplate uriTemplate = createUriTemplate(targetUrl, enc);
+			Map<String, String> variables = getCurrentRequestUriVariables(request);
-			if (uriTemplate.getVariableNames().size() > 0) {
+			targetUrl = replaceUriTemplateVariables(targetUrl.toString(), model, variables, enc);
-				Map<String, Object> vars = new HashMap<String, Object>();
-				vars.putAll(getCurrentUriVars(request));
-				vars.putAll(model);
-				targetUrl = new StringBuilder(uriTemplate.expand(vars).toString());
-				model = removeKeys(model, uriTemplate.getVariableNames());
-			}
-		}
-		
-		FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request);
-		if (!CollectionUtils.isEmpty(flashMap)) {
-			String targetPath = WebUtils.extractUrlPath(targetUrl.toString());
-			flashMap.setTargetRequestPath(targetPath);
 		}
 		
 		if (this.exposeModelAttributes) {
 			appendQueryProperties(targetUrl, model, enc);
-			if (!CollectionUtils.isEmpty(flashMap)) {
-				flashMap.addTargetRequestParams(model);
-			}
 		}
 
 		return targetUrl.toString();
 	}
 
 	/**
-	 * Returns the URI template variables extracted from the current request.
+	 * Replace URI template variables in the target URL with encoded model 
+	 * attributes or URI variables from the current request. Model attributes
+	 * referenced in the URL are removed from the model. 
+	 * @param targetUrl the redirect URL
+	 * @param model Map that contains model attributes
+	 * @param currentUriVariables current request URI variables to use
+	 * @param encodingScheme the encoding scheme to use
+	 * @throws UnsupportedEncodingException if string encoding failed
 	 */
-	@SuppressWarnings("unchecked")
+	protected StringBuilder replaceUriTemplateVariables(
-	private Map<String, String> getCurrentUriVars(HttpServletRequest request) {
+			String targetUrl, Map<String, Object> model, Map<String, String> currentUriVariables, String encodingScheme)
-		String name = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE;
+			throws UnsupportedEncodingException {
-		Map<String, String> map = (Map<String, String>) request.getAttribute(name);
-		return (map != null) ? map : new HashMap<String, String>();
-	}
 		
-	@SuppressWarnings("serial")
+		StringBuilder result = new StringBuilder();
-	private UriTemplate createUriTemplate(StringBuilder targetUrl, final String encoding) {
+		Matcher m = URI_TEMPLATE_VARIABLE_PATTERN.matcher(targetUrl);
-		return new UriTemplate(targetUrl.toString()) {
+		int endLastMatch = 0;
-			@Override
+		while (m.find()) {
-			protected URI encodeUri(String uri) {
+			String name = m.group(1);
-				try {
+			Object value = model.containsKey(name) ? model.remove(name) : currentUriVariables.get(name);
-					return new URI(uri);
+			Assert.notNull(value, "Model has no value for '" + name + "'");
-				}
+			result.append(targetUrl.substring(endLastMatch, m.start()));
-				catch (URISyntaxException ex) {
+			result.append(UriUtils.encodePathSegment(value.toString(), encodingScheme));
-					throw new IllegalArgumentException("Could not create URI from [" + uri + "]: " + ex, ex);
+			endLastMatch = m.end();
-				}
-			}
-		};
-	}
-	
-	private static Map<String, Object> removeKeys(Map<String, Object> map, List<String> keysToRemove) {
-		Map<String, Object> result = new HashMap<String, Object>(map);
-		for (String key : keysToRemove) {
-			result.remove(key);
 		}
+		result.append(targetUrl.substring(endLastMatch, targetUrl.length()));
 		return result;
 	}
 
+	@SuppressWarnings("unchecked")
+	private Map<String, String> getCurrentRequestUriVariables(HttpServletRequest request) {
+		Map<String, String> uriVars = 
+			(Map<String, String>) request.getAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
+		return (uriVars != null) ? uriVars : Collections.<String, String> emptyMap();
+	}
+	
 	/**
 	 * Append query properties to the redirect URL.
 	 * Stringifies, URL-encodes and formats model attributes as query properties.

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java

@@ -25,6 +25,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.ui.ModelMap;
 import org.springframework.web.servlet.HandlerMapping;
 
 public class RedirectViewUriTemplateTests {
@@ -40,7 +41,7 @@ public class RedirectViewUriTemplateTests {
 	}
 
 	@Test
-	public void uriTemplateVar() throws Exception {
+	public void uriTemplate() throws Exception {
 		Map<String, Object> model = new HashMap<String, Object>();
 		model.put("foo", "bar");
 
@@ -52,7 +53,19 @@ public class RedirectViewUriTemplateTests {
 	}
 	
 	@Test
-	public void uriTemplateVarAndArrayParam() throws Exception {
+	public void uriTemplateEncode() throws Exception {
+		Map<String, Object> model = new HashMap<String, Object>();
+		model.put("foo", "bar/bar baz");
+
+		String baseUrl = "http://url.somewhere.com";
+		RedirectView redirectView = new RedirectView(baseUrl + "/context path/{foo}");
+		redirectView.renderMergedOutputModel(model, request, response);
+
+		assertEquals(baseUrl + "/context path/bar%2Fbar%20baz", response.getRedirectedUrl());
+	}
+	
+	@Test
+	public void uriTemplateAndArrayQueryParam() throws Exception {
 		Map<String, Object> model = new HashMap<String, Object>();
 		model.put("foo", "bar");
 		model.put("fooArr", new String[] { "baz", "bazz" });
@@ -64,7 +77,7 @@ public class RedirectViewUriTemplateTests {
 	}
 
 	@Test
-	public void uriTemplateVarWithObjectConversion() throws Exception {
+	public void uriTemplateWithObjectConversion() throws Exception {
 		Map<String, Object> model = new HashMap<String, Object>();
 		model.put("foo", new Long(611));
 
@@ -75,17 +88,17 @@ public class RedirectViewUriTemplateTests {
 	}
 
 	@Test
-	public void currentRequestUriTemplateVars() throws Exception {
+	public void uriTemplateReuseCurrentRequestVars() throws Exception {
 		Map<String, Object> model = new HashMap<String, Object>();
 		model.put("key1", "value1");
 		model.put("name", "value2");
 		model.put("key3", "value3");
 		
-		Map<String, String> vars = new HashMap<String, String>();
+		Map<String, String> currentRequestUriTemplateVars = new HashMap<String, String>();
-		vars.put("var1", "v1");
+		currentRequestUriTemplateVars.put("var1", "v1");
-		vars.put("name", "v2");
+		currentRequestUriTemplateVars.put("name", "v2");
-		vars.put("var3", "v3");
+		currentRequestUriTemplateVars.put("var3", "v3");
-		request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, vars);
+		request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, currentRequestUriTemplateVars);
 
 		String url = "http://url.somewhere.com";
 		RedirectView redirectView = new RedirectView(url + "/{key1}/{var1}/{name}");
@@ -94,6 +107,11 @@ public class RedirectViewUriTemplateTests {
 		assertEquals(url + "/value1/v1/value2?key3=value3", response.getRedirectedUrl());
 	}
 
+	@Test(expected=IllegalArgumentException.class)
+	public void uriTemplateNullValue() throws Exception {
+		new RedirectView("/{foo}").renderMergedOutputModel(new ModelMap(), request, response);
+	}
+	
 	@Test
 	public void emptyRedirectString() throws Exception {
 		Map<String, Object> model = new HashMap<String, Object>();