From 077055c8f2121c9d9f649b417a20991b32321e89 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Mon, 9 Nov 2009 12:15:17 +0000 Subject: [PATCH] SPR-6291 - UrlPathHelper is too aggressive decoding URLs --- .../web/servlet/tags/UrlTag.java | 31 +--- .../PathMatchingUrlHandlerMappingTests.java | 23 ++- .../web/servlet/tags/UrlTagTests.java | 7 - .../springframework/web/util/UriUtils.java | 147 ++++++++++++++++++ .../web/util/UrlPathHelper.java | 2 +- .../web/util/UriUtilsTest.java | 50 ++++++ .../web/util/UrlPathHelperTests.java | 48 ++++-- 7 files changed, 252 insertions(+), 56 deletions(-) create mode 100644 org.springframework.web/src/main/java/org/springframework/web/util/UriUtils.java create mode 100644 org.springframework.web/src/test/java/org/springframework/web/util/UriUtilsTest.java diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java index 6a10eed6f9..f6350a9cff 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java +++ b/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. */ diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/handler/PathMatchingUrlHandlerMappingTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/handler/PathMatchingUrlHandlerMappingTests.java index c0add0066d..3b86164300 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/handler/PathMatchingUrlHandlerMappingTests.java +++ b/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; diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java index e60ea953a4..9ba86bb613 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java +++ b/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)); } diff --git a/org.springframework.web/src/main/java/org/springframework/web/util/UriUtils.java b/org.springframework.web/src/main/java/org/springframework/web/util/UriUtils.java new file mode 100644 index 0000000000..c7e837b06d --- /dev/null +++ b/org.springframework.web/src/main/java/org/springframework/web/util/UriUtils.java @@ -0,0 +1,147 @@ +/* + * Copyright 2002-2009 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.util; + +import java.io.ByteArrayOutputStream; +import java.io.UnsupportedEncodingException; +import java.util.BitSet; + +import org.springframework.util.Assert; + +/** + * Utility class for URI encoding. Based on RFC 2396. + * + *

Effectively, the encoding and decoding methods in this class + * are similar to those found in {@link java.net.URLEncoder} and + * {@link java.net.URLDecoder}, except that the space character + * is encoded as {@code %20}, not {@code +}. + * + * @author Arjen Poutsma + * @since 3.0 + * @see RFC 2396 + */ +public abstract class UriUtils { + + private static final BitSet notEncoded = new BitSet(256); + + static { + for (int i = 'a'; i <= 'z'; i++) { + notEncoded.set(i); + } + for (int i = 'A'; i <= 'Z'; i++) { + notEncoded.set(i); + } + for (int i = '0'; i <= '9'; i++) { + notEncoded.set(i); + } + notEncoded.set('-'); + notEncoded.set('_'); + notEncoded.set('.'); + notEncoded.set('*'); + + } + + /** + * Encodes the given source URI into an encoded String. Based on the following + * rules: + *

+ * + * @param source the String to be encoded + * @param encoding the encoding to use + * @return the encoded string + * @throws UnsupportedEncodingException when the given encoding parameter is not supported + * @see java.net.URLEncoder#encode(String, String) + */ + public static String encode(String source, String encoding) throws UnsupportedEncodingException { + Assert.notNull(source, "'source' must not be null"); + Assert.hasLength(encoding, "'encoding' must not be empty"); + ByteArrayOutputStream bos = new ByteArrayOutputStream(source.length() * 2); + + for (int i = 0; i < source.length(); i++) { + int ch = source.charAt(i); + if (notEncoded.get(ch)) { + bos.write(ch); + } + else { + bos.write('%'); + char hex1 = Character.toUpperCase(Character.forDigit((ch >> 4) & 0xF, 16)); + char hex2 = Character.toUpperCase(Character.forDigit(ch & 0xF, 16)); + bos.write(hex1); + bos.write(hex2); + } + } + return new String(bos.toByteArray(), encoding); + } + + /** + * Decodes the given encoded source String into an URI. Based on the following + * rules: + * + * @param source + * @param encoding + * @return the decoded URI + * @throws UnsupportedEncodingException when the given encoding parameter is not supported + * @see java.net.URLDecoder#decode(String, String) + */ + public static String decode(String source, String encoding) throws UnsupportedEncodingException { + Assert.notNull(source, "'source' must not be null"); + Assert.hasLength(encoding, "'encoding' must not be empty"); + int length = source.length(); + ByteArrayOutputStream bos = new ByteArrayOutputStream(length); + for (int i = 0; i < length; i++) { + int ch = source.charAt(i); + if (ch == '%') { + if ((i + 2) < length) { + char hex1 = source.charAt(i + 1); + char hex2 = source.charAt(i + 2); + int u = Character.digit(hex1, 16); + int l = Character.digit(hex2, 16); + bos.write((char) ((u << 4) + l)); + i += 2; + } + else { + throw new IllegalArgumentException("Invalid encoded sequence \"" + source.substring(i) + "\""); + } + } + else { + bos.write(ch); + } + } + return new String(bos.toByteArray(), encoding); + } + +} diff --git a/org.springframework.web/src/main/java/org/springframework/web/util/UrlPathHelper.java b/org.springframework.web/src/main/java/org/springframework/web/util/UrlPathHelper.java index 52a11419bc..611443056e 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/util/UrlPathHelper.java +++ b/org.springframework.web/src/main/java/org/springframework/web/util/UrlPathHelper.java @@ -304,7 +304,7 @@ public class UrlPathHelper { if (this.urlDecode) { String enc = determineEncoding(request); try { - return URLDecoder.decode(source, enc); + return UriUtils.decode(source, enc); } catch (UnsupportedEncodingException ex) { if (logger.isWarnEnabled()) { diff --git a/org.springframework.web/src/test/java/org/springframework/web/util/UriUtilsTest.java b/org.springframework.web/src/test/java/org/springframework/web/util/UriUtilsTest.java new file mode 100644 index 0000000000..3fd5a9e4d0 --- /dev/null +++ b/org.springframework.web/src/test/java/org/springframework/web/util/UriUtilsTest.java @@ -0,0 +1,50 @@ +/* + * Copyright 2002-2009 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.util; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.regex.Pattern; +import java.util.regex.Matcher; +import java.io.UnsupportedEncodingException; + +import org.junit.Test; +import static org.junit.Assert.assertEquals; + +/** @author Arjen Poutsma */ +public class UriUtilsTest { + + @Test + public void encode() throws UnsupportedEncodingException { + assertEquals("Invalid encoded URI", "foobar", UriUtils.encode("foobar", "UTF-8")); + assertEquals("Invalid encoded URI", "foo%20bar", UriUtils.encode("foo bar", "UTF-8")); + assertEquals("Invalid encoded URI", "foo%2Bbar", UriUtils.encode("foo+bar", "UTF-8")); + } + + @Test + public void decode() throws UnsupportedEncodingException { + assertEquals("Invalid encoded URI", "foobar", UriUtils.decode("foobar", "UTF-8")); + assertEquals("Invalid encoded URI", "foo bar", UriUtils.decode("foo%20bar", "UTF-8")); + assertEquals("Invalid encoded URI", "foo+bar", UriUtils.decode("foo%2bbar", "UTF-8")); + } + + @Test(expected = IllegalArgumentException.class) + public void decodeInvalidSequence() throws UnsupportedEncodingException { + UriUtils.decode("foo%2", "UTF-8"); + } + +} diff --git a/org.springframework.web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java b/org.springframework.web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java index a48a3354bf..7cf9723cc7 100644 --- a/org.springframework.web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java +++ b/org.springframework.web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java @@ -16,44 +16,68 @@ package org.springframework.web.util; +import java.net.URI; +import java.net.URISyntaxException; + import junit.framework.TestCase; import org.springframework.mock.web.MockHttpServletRequest; +import org.junit.Test; +import org.junit.Before; +import static org.junit.Assert.assertEquals; + /** * @author Rob Harrop * @author Juergen Hoeller */ -public class UrlPathHelperTests extends TestCase { +public class UrlPathHelperTests { - public void testGetPathWithinApplication() { - UrlPathHelper helper = new UrlPathHelper(); + private UrlPathHelper helper; - MockHttpServletRequest request = new MockHttpServletRequest(); + private MockHttpServletRequest request; + + @Before + public void setUp() { + helper = new UrlPathHelper(); + request = new MockHttpServletRequest(); + } + + @Test + public void getPathWithinApplication() { request.setContextPath("/petclinic"); request.setRequestURI("/petclinic/welcome.html"); assertEquals("Incorrect path returned", "/welcome.html", helper.getPathWithinApplication(request)); } - public void testGetPathWithinApplicationForRootWithNoLeadingSlash() { - UrlPathHelper helper = new UrlPathHelper(); - - MockHttpServletRequest request = new MockHttpServletRequest(); + @Test + public void getPathWithinApplicationForRootWithNoLeadingSlash() { request.setContextPath("/petclinic"); request.setRequestURI("/petclinic"); assertEquals("Incorrect root path returned", "/", helper.getPathWithinApplication(request)); } - public void testGetPathWithinApplicationForSlashContextPath() { - UrlPathHelper helper = new UrlPathHelper(); - - MockHttpServletRequest request = new MockHttpServletRequest(); + @Test + public void getPathWithinApplicationForSlashContextPath() { request.setContextPath("/"); request.setRequestURI("/welcome.html"); assertEquals("Incorrect path returned", "/welcome.html", helper.getPathWithinApplication(request)); } + @Test + public void getRequestUri() { + request.setRequestURI("/welcome.html"); + assertEquals("Incorrect path returned", "/welcome.html", helper.getRequestUri(request)); + + request.setRequestURI("/foo%20bar"); + assertEquals("Incorrect path returned", "/foo bar", helper.getRequestUri(request)); + + request.setRequestURI("/foo+bar"); + assertEquals("Incorrect path returned", "/foo+bar", helper.getRequestUri(request)); + + } + }