Revert "WW-4628: proper decoding of parameters in query-string" This reverts commit ef9c66118ede16f3ff239ea864641d5bdadeecae.
Project: http://git-wip-us.apache.org/repos/asf/struts/repo Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/51a49201 Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/51a49201 Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/51a49201 Branch: refs/heads/master Commit: 51a49201adf73e33ba68d533f3535a32f507b531 Parents: b6be359 Author: gregh3269 <gregh3...@gmail.com> Authored: Thu Aug 4 08:57:25 2016 +0100 Committer: gregh3269 <gregh3...@gmail.com> Committed: Thu Aug 4 08:57:25 2016 +0100 ---------------------------------------------------------------------- .../org/apache/struts2/util/URLDecoderUtil.java | 12 ---- .../struts2/views/util/DefaultUrlHelper.java | 27 +++----- .../apache/struts2/util/URLDecoderUtilTest.java | 7 -- .../views/util/DefaultUrlHelperTest.java | 71 +++++++++++++++----- 4 files changed, 64 insertions(+), 53 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/struts/blob/51a49201/core/src/main/java/org/apache/struts2/util/URLDecoderUtil.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/struts2/util/URLDecoderUtil.java b/core/src/main/java/org/apache/struts2/util/URLDecoderUtil.java index 3c61d1e..10f2a78 100644 --- a/core/src/main/java/org/apache/struts2/util/URLDecoderUtil.java +++ b/core/src/main/java/org/apache/struts2/util/URLDecoderUtil.java @@ -19,16 +19,4 @@ public class URLDecoderUtil { return UDecoder.URLDecode(sequence, charset); } - /** - * Decodes a <code>x-www-form-urlencoded</code> string. - * @param sequence the String to decode - * @param charset The name of a supported character encoding. - * @param isQueryString whether input is a query string. If <code>true</code> other decoding rules apply. - * @return the newly decoded <code>String</code> - * @exception IllegalArgumentException If the encoding is not valid - */ - public static String decode(String sequence, String charset, boolean isQueryString) { - return UDecoder.URLDecode(sequence, charset, isQueryString); - } - } http://git-wip-us.apache.org/repos/asf/struts/blob/51a49201/core/src/main/java/org/apache/struts2/views/util/DefaultUrlHelper.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/struts2/views/util/DefaultUrlHelper.java b/core/src/main/java/org/apache/struts2/views/util/DefaultUrlHelper.java index 668d1a9..16739af 100644 --- a/core/src/main/java/org/apache/struts2/views/util/DefaultUrlHelper.java +++ b/core/src/main/java/org/apache/struts2/views/util/DefaultUrlHelper.java @@ -284,25 +284,14 @@ public class DefaultUrlHelper implements UrlHelper { * @return the encoded string */ public String decode( String input ) { - return URLDecoderUtil.decode(input, encoding, false); + try { + return URLDecoderUtil.decode(input, encoding); + } catch (Exception e) { + LOG.warn("Could not decode URL parameter '{}', returning value un-decoded", input); + return input; + } } - /** - * Decodes the URL using {@link URLDecoderUtil#decode(String, String, boolean)} with the encoding specified in the configuration. - * - * @param input the input to decode - * @param isQueryString whether input is a query string. If <code>true</code> other decoding rules apply. - * @return the encoded string - */ - public String decode( String input, boolean isQueryString ) { - try { - return URLDecoderUtil.decode(input, encoding, isQueryString); - } catch (Exception e) { - LOG.warn("Could not decode URL parameter '{}', returning value un-decoded", input); - return input; - } - } - public Map<String, Object> parseQueryString(String queryString, boolean forceValueArray) { Map<String, Object> queryParams = new LinkedHashMap<String, Object>(); if (queryString != null) { @@ -319,8 +308,8 @@ public class DefaultUrlHelper implements UrlHelper { paramValue = tmpParams[1]; } if (paramName != null) { - paramName = decode(paramName, true); - String translatedParamValue = decode(paramValue, true); + paramName = decode(paramName); + String translatedParamValue = decode(paramValue); if (queryParams.containsKey(paramName) || forceValueArray) { // WW-1619 append new param value to existing value(s) http://git-wip-us.apache.org/repos/asf/struts/blob/51a49201/core/src/test/java/org/apache/struts2/util/URLDecoderUtilTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/struts2/util/URLDecoderUtilTest.java b/core/src/test/java/org/apache/struts2/util/URLDecoderUtilTest.java index fd274bf..f21c08f 100644 --- a/core/src/test/java/org/apache/struts2/util/URLDecoderUtilTest.java +++ b/core/src/test/java/org/apache/struts2/util/URLDecoderUtilTest.java @@ -68,11 +68,4 @@ public class URLDecoderUtilTest { assertEquals("xxxx\u00ea", result); } - @Test - public void testURLDecodePlusCharAsSpace() { - - String result = URLDecoderUtil.decode("a+b", "UTF-8", true); - assertEquals("a b", result); - } - } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/struts/blob/51a49201/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java b/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java index b4a5b26..57786c4 100644 --- a/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java +++ b/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java @@ -21,23 +21,22 @@ package org.apache.struts2.views.util; +import com.mockobjects.dynamic.Mock; +import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.inject.Container; +import com.opensymphony.xwork2.inject.Scope.Strategy; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.StrutsInternalTestCase; +import org.junit.Ignore; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; import java.util.TreeMap; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.apache.struts2.StrutsConstants; -import org.apache.struts2.StrutsInternalTestCase; - -import com.mockobjects.dynamic.Mock; -import com.opensymphony.xwork2.ActionContext; -import com.opensymphony.xwork2.inject.Container; -import com.opensymphony.xwork2.inject.Scope.Strategy; - /** * Test case for DefaultUrlHelper. @@ -130,6 +129,34 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase { expectedUrl, url.toString()); } + @Ignore + public void ignoreTestBuildUrlWithJavaScriptInjected() throws Exception { + String expectedUrl = "http://localhost:8080/myContext/myPage.jsp?initParam=initValue&param1=value1&param2=value2&param3%22%3Cscript+type%3D%22text%2Fjavascript%22%3Ealert%281%29%3B%3C%2Fscript%3E=value3";; + + // there is explicit escaping for EcmaScript before URL encoding + String expectedUrlBeforeEncoding = "http:\\/\\/localhost:8080\\/myContext\\/myPage.jsp?initParam=initValue&param1=value1&param2=value2&param3\\\"<script type=\\\"text\\/javascript\\\">alert(1);<\\/script>=value3"; + + Mock mockHttpServletRequest = new Mock(HttpServletRequest.class); + mockHttpServletRequest.expectAndReturn("getScheme", "http"); + mockHttpServletRequest.expectAndReturn("getServerName", "localhost"); + mockHttpServletRequest.expectAndReturn("getContextPath", "/myContext"); + mockHttpServletRequest.expectAndReturn("getServerPort", 8080); + + Mock mockHttpServletResponse = new Mock(HttpServletResponse.class); + mockHttpServletResponse.expectAndReturn("encodeURL", expectedUrlBeforeEncoding, expectedUrl); + + Map parameters = new LinkedHashMap(); + parameters.put("param1", "value1"); + parameters.put("param2", "value2"); + parameters.put("param3\"<script type=\"text/javascript\">alert(1);</script>","value3"); + + String result = urlHelper.buildUrl("/myPage.jsp?initParam=initValue", (HttpServletRequest) mockHttpServletRequest.proxy(), (HttpServletResponse) mockHttpServletResponse.proxy(), parameters, "http", true, true, true); + + assertEquals( + expectedUrl, result); + mockHttpServletRequest.verify(); + } + public void testForceAddNullSchemeHostAndPort() throws Exception { String expectedUrl = "http://localhost/contextPath/path1/path2/myAction.action";; @@ -396,11 +423,25 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase { assertEquals(result, expectedResult); } - public void testDecodeSpacesInQueryString() throws Exception { - Map<String, Object> queryParameters = urlHelper.parseQueryString("name=value+with+space", false); + @Ignore + public void ignoreTestDontEncode() throws Exception { + String expectedUrl = "http://localhost/contextPath/myAction.action?param1=value+with+spaces";; - assertTrue(queryParameters.containsKey("name")); - assertEquals("value with space", queryParameters.get("name")); + Mock mockHttpServletRequest = new Mock(HttpServletRequest.class); + mockHttpServletRequest.expectAndReturn("getScheme", "http"); + mockHttpServletRequest.expectAndReturn("getServerName", "localhost"); + mockHttpServletRequest.expectAndReturn("getContextPath", "/contextPath"); + mockHttpServletRequest.expectAndReturn("getServerPort", 80); + + Mock mockHttpServletResponse = new Mock(HttpServletResponse.class); + + Map parameters = new LinkedHashMap(); + parameters.put("param1", "value+with+spaces"); + + String result = urlHelper.buildUrl("/myAction.action", (HttpServletRequest) mockHttpServletRequest.proxy(), (HttpServletResponse) mockHttpServletResponse.proxy(), parameters, "http", true, false, true); + + assertEquals( + expectedUrl, result); }
