SPR-6291 - UrlPathHelper is too aggressive decoding URLs

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java

@@ -33,6 +33,7 @@ import org.springframework.web.util.ExpressionEvaluationUtils;
 import org.springframework.web.util.HtmlUtils;
 import org.springframework.web.util.JavaScriptUtils;
 import org.springframework.web.util.TagUtils;
+import org.springframework.web.util.UriUtils;
 
 /**
  * JSP tag for creating URLs. Modeled after the JSTL c:url tag with backwards
@@ -296,41 +297,13 @@ public class UrlTag extends HtmlEscapingAwareTag implements ParamAware {
 		}
 		try {
 			String encoding = pageContext.getResponse().getCharacterEncoding();
-			String formUrlEncodedValue = URLEncoder.encode(value, encoding);
-			if (!formUrlEncodedValue.contains("+")) {
-				return formUrlEncodedValue;
-			}
-			String spaceEncoding = this.urlEncode(' ', encoding);
-			return formUrlEncodedValue.replace("+", spaceEncoding);
+			return UriUtils.encode(value, encoding);
 		}
 		catch (UnsupportedEncodingException ex) {
 			throw new JspException(ex);
 		}
 	}
 
-	/*
-	 * based on URLCodec from Apache Commons Codec
-	 */
-	protected String urlEncode(Character c, String enc) throws UnsupportedEncodingException {
-		if (c == null) {
-			return null;
-		}
-		byte[] bytes = c.toString().getBytes(enc);
-		StringBuilder builder = new StringBuilder();
-		for (int i = 0; i < bytes.length; i++) {
-			int b = bytes[i];
-			if (b < 0) {
-				b = 256 + b;
-			}
-			char hex1 = Character.toUpperCase(Character.forDigit((b >> 4) & 0xF, 16));
-			char hex2 = Character.toUpperCase(Character.forDigit(b & 0xF, 16));
-			builder.append('%');
-			builder.append(hex1);
-			builder.append(hex2);
-		}
-		return builder.toString();
-	}
-
 	/**
 	 * Internal enum that classifies URLs by type.
 	 */

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/handler/PathMatchingUrlHandlerMappingTests.java

@@ -26,11 +26,15 @@ import org.springframework.web.servlet.HandlerExecutionChain;
 import org.springframework.web.servlet.HandlerInterceptor;
 import org.springframework.web.servlet.HandlerMapping;
 
+import org.junit.Before;
+import org.junit.Test;
+import static org.junit.Assert.*;
+
 /**
  * @author Alef Arendsen
  * @author Juergen Hoeller
  */
-public class PathMatchingUrlHandlerMappingTests extends TestCase {
+public class PathMatchingUrlHandlerMappingTests {
 
 	public static final String CONF = "/org/springframework/web/servlet/handler/map3.xml";
 
@@ -38,6 +42,7 @@ public class PathMatchingUrlHandlerMappingTests extends TestCase {
 
 	private ConfigurableWebApplicationContext wac;
 
+	@Before
 	public void setUp() throws Exception {
 		MockServletContext sc = new MockServletContext("");
 		wac = new XmlWebApplicationContext();
@@ -47,7 +52,8 @@ public class PathMatchingUrlHandlerMappingTests extends TestCase {
 		hm = (HandlerMapping) wac.getBean("urlMapping");
 	}
 
-	public void testRequestsWithHandlers() throws Exception {
+	@Test
+	public void requestsWithHandlers() throws Exception {
 		Object bean = wac.getBean("mainController");
 
 		MockHttpServletRequest req = new MockHttpServletRequest("GET", "/welcome.html");
@@ -63,7 +69,8 @@ public class PathMatchingUrlHandlerMappingTests extends TestCase {
 		assertTrue("Handler is correct bean", hec != null && hec.getHandler() == bean);
 	}
 
-	public void testActualPathMatching() throws Exception {
+	@Test
+	public void actualPathMatching() throws Exception {
 		// there a couple of mappings defined with which we can test the
 		// path matching, let's do that...
 
@@ -222,14 +229,16 @@ public class PathMatchingUrlHandlerMappingTests extends TestCase {
 		assertTrue("Handler is correct bean", hec != null && hec.getHandler() == defaultBean);
 	}
 
-	public void testDefaultMapping() throws Exception {
+	@Test
+	public void defaultMapping() throws Exception {
 		Object bean = wac.getBean("starController");
 		MockHttpServletRequest req = new MockHttpServletRequest("GET", "/goggog.html");
 		HandlerExecutionChain hec = getHandler(req);
 		assertTrue("Handler is correct bean", hec != null && hec.getHandler() == bean);
 	}
 
-	public void testMappingExposedInRequest() throws Exception {
+	@Test
+	public void mappingExposedInRequest() throws Exception {
 		Object bean = wac.getBean("mainController");
 		MockHttpServletRequest req = new MockHttpServletRequest("GET", "/show.html");
 		HandlerExecutionChain hec = getHandler(req);
@@ -241,8 +250,8 @@ public class PathMatchingUrlHandlerMappingTests extends TestCase {
 		HandlerExecutionChain hec = hm.getHandler(req);
 		HandlerInterceptor[] interceptors = hec.getInterceptors();
 		if (interceptors != null) {
-			for (int i = 0; i < interceptors.length; i++) {
-				interceptors[i].preHandle(req, null, hec.getHandler());
+			for (HandlerInterceptor interceptor : interceptors) {
+				interceptor.preHandle(req, null, hec.getHandler());
 			}
 		}
 		return hec;

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java

@@ -554,13 +554,6 @@ public class UrlTagTests extends AbstractTagTests {
 		assertEquals("my%20name%2Bis", tag.urlEncode("my name+is"));
 	}
 	
-	public void testUrlEncode_character() throws UnsupportedEncodingException {
-		assertEquals("%20", tag.urlEncode(' ', "UTF-8"));
-		assertEquals(" ", URLDecoder.decode("%20", "UTF-8"));
-		assertEquals("%FE%FF%00%20", tag.urlEncode(' ', "UTF-16"));
-		assertEquals("%40", tag.urlEncode(' ', "IBM-Thai"));
-	}
-
 	public void testUrlEncodeNull() throws JspException {
 		assertNull(tag.urlEncode(null));
 	}