WW-4628: avoid double encoding of url parameters, e.g. when spaces are present in parameter values

This commit is contained in:
cnenning
2016-08-01 13:09:12 +02:00
parent 8b559afc76
commit af50018f11
2 changed files with 76 additions and 17 deletions
@@ -21,7 +21,18 @@
package org.apache.struts2.views.util;
import com.opensymphony.xwork2.inject.Inject;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.StringEscapeUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
@@ -29,12 +40,7 @@ import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.util.URLDecoderUtil;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.util.*;
import com.opensymphony.xwork2.inject.Inject;
/**
* Default implementation of UrlHelper
@@ -174,10 +180,11 @@ public class DefaultUrlHelper implements UrlHelper {
}
//if the action was not explicitly set grab the params from the request
//always pass encode=false here as encoding might happen for complete URL later
if (escapeAmp) {
buildParametersString(params, link, AMP);
buildParametersString(params, link, AMP, false);
} else {
buildParametersString(params, link, "&");
buildParametersString(params, link, "&", false);
}
String result = link.toString();
@@ -196,6 +203,10 @@ public class DefaultUrlHelper implements UrlHelper {
}
public void buildParametersString(Map<String, Object> params, StringBuilder link, String paramSeparator) {
buildParametersString(params, link, paramSeparator, true);
}
public void buildParametersString(Map<String, Object> params, StringBuilder link, String paramSeparator, boolean encode) {
if ((params != null) && (params.size() > 0)) {
if (!link.toString().contains("?")) {
link.append("?");
@@ -213,7 +224,7 @@ public class DefaultUrlHelper implements UrlHelper {
if (value instanceof Iterable) {
for (Iterator iterator = ((Iterable) value).iterator(); iterator.hasNext();) {
Object paramValue = iterator.next();
link.append(buildParameterSubstring(name, paramValue != null ? paramValue.toString() : StringUtils.EMPTY));
link.append(buildParameterSubstring(name, paramValue != null ? paramValue.toString() : StringUtils.EMPTY, encode));
if (iterator.hasNext()) {
link.append(paramSeparator);
@@ -223,14 +234,14 @@ public class DefaultUrlHelper implements UrlHelper {
Object[] array = (Object[]) value;
for (int i = 0; i < array.length; i++) {
Object paramValue = array[i];
link.append(buildParameterSubstring(name, paramValue != null ? paramValue.toString() : StringUtils.EMPTY));
link.append(buildParameterSubstring(name, paramValue != null ? paramValue.toString() : StringUtils.EMPTY, encode));
if (i < array.length - 1) {
link.append(paramSeparator);
}
}
} else {
link.append(buildParameterSubstring(name, value != null ? value.toString() : StringUtils.EMPTY));
link.append(buildParameterSubstring(name, value != null ? value.toString() : StringUtils.EMPTY, encode));
}
if (iter.hasNext()) {
@@ -244,11 +255,11 @@ public class DefaultUrlHelper implements UrlHelper {
return HTTP_PROTOCOL.equals(scheme) || HTTPS_PROTOCOL.equals(scheme);
}
private String buildParameterSubstring(String name, String value) {
private String buildParameterSubstring(String name, String value, boolean encode) {
StringBuilder builder = new StringBuilder();
builder.append(encode(name));
builder.append(encode ? encode(name) : name);
builder.append('=');
builder.append(encode(value));
builder.append(encode ? encode(value) : value);
return builder.toString();
}
@@ -262,7 +273,7 @@ public class DefaultUrlHelper implements UrlHelper {
try {
return URLEncoder.encode(input, encoding);
} catch (UnsupportedEncodingException e) {
LOG.warn("Could not encode URL parameter '{}', returning value un-encoded", input);
LOG.warn("Could not encode URL parameter '{}', returning value un-encoded", input);
return input;
}
}
@@ -128,6 +128,33 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
expectedUrl, url.toString());
}
public void testBuildUrlWithJavaScriptInjected() throws Exception {
String expectedUrl = "http://localhost:8080/myContext/myPage.jsp?initParam=initValue&amp;param1=value1&amp;param2=value2&amp;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&amp;param1=value1&amp;param2=value2&amp;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";
@@ -393,7 +420,28 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
assertEquals(result, expectedResult);
}
public void testDontEncode() throws Exception {
String expectedUrl = "http://localhost/contextPath/myAction.action?param1=value+with+spaces";
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);
}
public void setUp() throws Exception {
super.setUp();
stubContainer = new StubContainer(container);