You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2022/11/04 07:50:05 UTC
[struts] 01/02: WW-4514 Fixes building query string with empty parameters
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-4514-url
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 6330bb1ce34a253145f52eb2dff3f648003f0c8a
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Nov 4 07:58:47 2022 +0100
WW-4514 Fixes building query string with empty parameters
---
.../struts2/views/util/DefaultUrlHelper.java | 76 ++++++-----
.../apache/struts2/url/StrutsUrlDecoderTest.java | 8 ++
.../apache/struts2/url/StrutsUrlEncoderTest.java | 7 +
.../struts2/views/util/DefaultUrlHelperTest.java | 147 +++++++++++----------
4 files changed, 130 insertions(+), 108 deletions(-)
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 9f383c611..6998e24c6 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
@@ -31,7 +31,6 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -170,9 +169,9 @@ public class DefaultUrlHelper implements UrlHelper {
//if the action was not explicitly set grab the params from the request
if (escapeAmp) {
- buildParametersString(params, link, AMP, true);
+ buildParametersString(params, link, AMP);
} else {
- buildParametersString(params, link, "&", true);
+ buildParametersString(params, link, "&");
}
String result = link.toString();
@@ -198,62 +197,69 @@ 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("?");
- } else {
- link.append(paramSeparator);
- }
+ StringBuilder queryString = new StringBuilder();
// Set params
- Iterator<Map.Entry<String, Object>> iter = params.entrySet().iterator();
- while (iter.hasNext()) {
- Map.Entry<String, Object> entry = iter.next();
+ for (Map.Entry<String, Object> entry : params.entrySet()) {
String name = entry.getKey();
Object value = entry.getValue();
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, encode));
-
- if (iterator.hasNext()) {
- link.append(paramSeparator);
- }
+ for (Object o : (Iterable<?>) value) {
+ appendParameterSubstring(queryString, paramSeparator, name, o);
}
} else if (value instanceof Object[]) {
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, encode));
-
- if (i < array.length - 1) {
- link.append(paramSeparator);
- }
+ for (Object o : array) {
+ appendParameterSubstring(queryString, paramSeparator, name, o);
}
} else {
- link.append(buildParameterSubstring(name, value != null ? value.toString() : StringUtils.EMPTY, encode));
+ appendParameterSubstring(queryString, paramSeparator, name, value);
}
+ }
- if (iter.hasNext()) {
+ if (queryString.length() > 0) {
+ if (!link.toString().contains("?")) {
+ link.append("?");
+ } else {
link.append(paramSeparator);
}
+ link.append(queryString);
}
}
}
+ /**
+ * Builds parameters assigned to url - a query string
+ * @param params a set of params to assign
+ * @param link a based url
+ * @param paramSeparator separator used
+ * @param encode if true, parameters will be encoded - ignored
+ * @deprecated since Struts 6.1.0, use {@link #buildParametersString(Map, StringBuilder, String)}
+ */
+ @Deprecated
+ public void buildParametersString(Map<String, Object> params, StringBuilder link, String paramSeparator, boolean encode) {
+ buildParametersString(params, link, paramSeparator);
+ }
+
protected boolean isValidScheme(String scheme) {
return HTTP_PROTOCOL.equals(scheme) || HTTPS_PROTOCOL.equals(scheme);
}
- private String buildParameterSubstring(String name, String value, boolean encode) {
- String encodedName = encode ? encoder.encode(name) : name;
- String encodedValue = encode ? encoder.encode(value) : value;
- return encodedName + '=' + encodedValue;
+ private void appendParameterSubstring(StringBuilder queryString, String paramSeparator, String name, Object value) {
+ if (queryString.length() > 0) {
+ queryString.append(paramSeparator);
+ }
+
+ String encodedName = encoder.encode(name);
+ queryString.append(encodedName);
+
+ queryString.append('=');
+ if (value != null) {
+ String encodedValue = encoder.encode(value.toString());
+ queryString.append(encodedValue);
+ }
}
/**
diff --git a/core/src/test/java/org/apache/struts2/url/StrutsUrlDecoderTest.java b/core/src/test/java/org/apache/struts2/url/StrutsUrlDecoderTest.java
index 5dd44f897..fe70d0944 100644
--- a/core/src/test/java/org/apache/struts2/url/StrutsUrlDecoderTest.java
+++ b/core/src/test/java/org/apache/struts2/url/StrutsUrlDecoderTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.struts2.url;
+import org.apache.struts2.StrutsConstants;
import org.junit.Before;
import org.junit.Test;
@@ -105,6 +106,13 @@ public class StrutsUrlDecoderTest {
assertEquals("xxxxA", result);
}
+ @Test
+ public void testDecoding() {
+ String result = decoder.decode("%E6%96%B0%E8%81%9E");
+
+ assertEquals(result, "\u65b0\u805e");
+ }
+
@Before
public void setUp() throws Exception {
this.decoder = new StrutsUrlDecoder();
diff --git a/core/src/test/java/org/apache/struts2/url/StrutsUrlEncoderTest.java b/core/src/test/java/org/apache/struts2/url/StrutsUrlEncoderTest.java
index 95aaacda6..361d44133 100644
--- a/core/src/test/java/org/apache/struts2/url/StrutsUrlEncoderTest.java
+++ b/core/src/test/java/org/apache/struts2/url/StrutsUrlEncoderTest.java
@@ -82,6 +82,13 @@ public class StrutsUrlEncoderTest {
assertEquals("%25xxxx", result);
}
+ @Test
+ public void testEncoding() {
+ String result = encoder.encode("\u65b0\u805e");
+
+ assertEquals(result, "%E6%96%B0%E8%81%9E");
+ }
+
@Before
public void setUp() throws Exception {
this.encoder = new StrutsUrlEncoder();
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 3e70e7a44..b1836f42a 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
@@ -18,36 +18,31 @@
*/
package org.apache.struts2.views.util;
-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;
+import org.apache.struts2.StrutsInternalTestCase;
import org.apache.struts2.url.StrutsUrlDecoder;
import org.apache.struts2.url.StrutsUrlEncoder;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
/**
* Test case for DefaultUrlHelper.
- *
*/
public class DefaultUrlHelperTest extends StrutsInternalTestCase {
- private StubContainer stubContainer;
private DefaultUrlHelper urlHelper;
- public void testForceAddSchemeHostAndPort() throws Exception {
+ public void testForceAddSchemeHostAndPort() {
String expectedUrl = "http://localhost/contextPath/path1/path2/myAction.action";
Mock mockHttpServletRequest = new Mock(HttpServletRequest.class);
@@ -64,7 +59,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletRequest.verify();
}
- public void testDoNotForceAddSchemeHostAndPort() throws Exception {
+ public void testDoNotForceAddSchemeHostAndPort() {
String expectedUrl = "/contextPath/path1/path2/myAction.action";
Mock mockHttpServletRequest = new Mock(HttpServletRequest.class);
@@ -80,7 +75,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
assertEquals(expectedUrl, result);
}
- public void testForceAddSchemeHostAndPortWithNonStandardPort() throws Exception {
+ public void testForceAddSchemeHostAndPortWithNonStandardPort() {
String expectedUrl = "http://localhost:9090/contextPath/path1/path2/myAction.action";
Mock mockHttpServletRequest = new Mock(HttpServletRequest.class);
@@ -97,39 +92,64 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletRequest.verify();
}
- public void testBuildParametersStringWithUrlHavingSomeExistingParameters() throws Exception {
+ public void testBuildParametersStringWithUrlHavingSomeExistingParameters() {
String expectedUrl = "http://localhost:8080/myContext/myPage.jsp?initParam=initValue&param1=value1&param2=value2&param3%22%3CsCrIpT%3Ealert%281%29%3B%3C%2FsCrIpT%3E=value3";
- Map parameters = new LinkedHashMap();
+ Map<String, Object> parameters = new LinkedHashMap<>();
parameters.put("param1", "value1");
parameters.put("param2", "value2");
- parameters.put("param3\"<sCrIpT>alert(1);</sCrIpT>","value3");
+ parameters.put("param3\"<sCrIpT>alert(1);</sCrIpT>", "value3");
StringBuilder url = new StringBuilder("http://localhost:8080/myContext/myPage.jsp?initParam=initValue");
urlHelper.buildParametersString(parameters, url, UrlHelper.AMP);
assertEquals(
- expectedUrl, url.toString());
+ expectedUrl, url.toString());
}
- public void testBuildParametersStringWithJavaScriptInjected() throws Exception {
+ public void testBuildParametersStringWithJavaScriptInjected() {
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";
- Map parameters = new LinkedHashMap();
+ Map<String, Object> parameters = new LinkedHashMap<>();
parameters.put("param1", "value1");
parameters.put("param2", "value2");
- parameters.put("param3\"<script type=\"text/javascript\">alert(1);</script>","value3");
+ parameters.put("param3\"<script type=\"text/javascript\">alert(1);</script>", "value3");
StringBuilder url = new StringBuilder("http://localhost:8080/myContext/myPage.jsp?initParam=initValue");
urlHelper.buildParametersString(parameters, url, UrlHelper.AMP);
assertEquals(
- expectedUrl, url.toString());
+ expectedUrl, url.toString());
+ }
+
+ public void testBuildParametersStringWithEmptyListParameters() {
+ String expectedUrl = "https://www.nowhere.com/myworld.html";
+ Map<String, Object> parameters = new LinkedHashMap<>();
+ parameters.put("param1", new String[]{});
+ parameters.put("param2", new ArrayList<>());
+ StringBuilder url = new StringBuilder("https://www.nowhere.com/myworld.html");
+ urlHelper.buildParametersString(parameters, url, UrlHelper.AMP);
+ assertEquals(expectedUrl, url.toString());
}
- public void testForceAddNullSchemeHostAndPort() throws Exception {
+ public void testBuildParametersStringWithListParameters() {
+ String expectedUrl = "https://www.nowhere.com/myworld.html?param1=x¶m2=y¶m2=z";
+ Map<String, Object> parameters = new LinkedHashMap<>();
+ parameters.put("param1", new String[]{"x"});
+ parameters.put("param2", new ArrayList<String>() {
+ {
+ add("y");
+ add("z");
+ }
+ });
+ StringBuilder url = new StringBuilder("https://www.nowhere.com/myworld.html");
+ urlHelper.buildParametersString(parameters, url, "&");
+ assertEquals(expectedUrl, url.toString());
+ }
+
+ public void testForceAddNullSchemeHostAndPort() {
String expectedUrl = "http://localhost/contextPath/path1/path2/myAction.action";
Mock mockHttpServletRequest = new Mock(HttpServletRequest.class);
@@ -144,14 +164,14 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
expectedUrl);
String result = urlHelper.buildUrl("/path1/path2/myAction.action",
- (HttpServletRequest) mockHttpServletRequest.proxy(),
- (HttpServletResponse) mockHttpServletResponse.proxy(), null,
- null, true, true, true);
+ (HttpServletRequest) mockHttpServletRequest.proxy(),
+ (HttpServletResponse) mockHttpServletResponse.proxy(), null,
+ null, true, true, true);
assertEquals(expectedUrl, result);
mockHttpServletRequest.verify();
}
- public void testForceAddNullSchemeHostAndPort2() throws Exception {
+ public void testForceAddNullSchemeHostAndPort2() {
String expectedUrl = "http://localhost:8080/contextPath/path1/path2/myAction.action";
Mock mockHttpServletRequest = new Mock(HttpServletRequest.class);
@@ -166,9 +186,9 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
expectedUrl);
String result = urlHelper.buildUrl("/path1/path2/myAction.action",
- (HttpServletRequest) mockHttpServletRequest.proxy(),
- (HttpServletResponse) mockHttpServletResponse.proxy(), null,
- null, true, true, true);
+ (HttpServletRequest) mockHttpServletRequest.proxy(),
+ (HttpServletResponse) mockHttpServletResponse.proxy(), null,
+ null, true, true, true);
assertEquals(expectedUrl, result);
mockHttpServletRequest.verify();
}
@@ -184,7 +204,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedUrl, expectedUrl);
String actualUrl = urlHelper.buildUrl(expectedUrl, (HttpServletRequest) mockHttpServletRequest.proxy(),
- (HttpServletResponse) mockHttpServletResponse.proxy(), new HashMap());
+ (HttpServletResponse) mockHttpServletResponse.proxy(), new HashMap<>());
assertEquals(expectedUrl, actualUrl);
}
@@ -199,7 +219,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedString, expectedString);
String actionName = "my.actionName";
- TreeMap params = new TreeMap();
+ TreeMap<String, Object> params = new TreeMap<>();
params.put("hello", "world");
params.put("foo", "bar");
@@ -218,7 +238,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedString, expectedString);
String actionName = "my.actionName";
- TreeMap params = new TreeMap();
+ TreeMap<String, Object> params = new TreeMap<>();
params.put("hello", "world");
params.put("foo", "bar");
@@ -234,7 +254,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedString, expectedString);
String actionName = "my.actionName";
- TreeMap params = new TreeMap();
+ TreeMap<String, Object> params = new TreeMap<>();
params.put("hello", new String[]{"earth", "mars"});
params.put("foo", "bar");
@@ -259,7 +279,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedString, expectedString);
String actionName = "/MyAction.action";
- TreeMap params = new TreeMap();
+ TreeMap<String, Object> params = new TreeMap<>();
params.put("hello", new String[]{"earth", "mars"});
params.put("foo", "bar");
@@ -284,7 +304,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedString, expectedString);
String actionName = "/MyAction.action";
- TreeMap params = new TreeMap();
+ TreeMap<String, Object> params = new TreeMap<>();
params.put("hello", new String[]{"earth", "mars"});
params.put("foo", "bar");
@@ -313,7 +333,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedString, expectedString);
String actionName = "/MyAction.action";
- TreeMap params = new TreeMap();
+ TreeMap<String, Object> params = new TreeMap<>();
params.put("hello", new String[]{"earth", "mars"});
params.put("foo", "bar");
@@ -342,7 +362,7 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedString, expectedString);
String actionName = "/MyAction.action";
- TreeMap params = new TreeMap();
+ TreeMap<String, Object> params = new TreeMap<>();
params.put("hello", new String[]{"earth", "mars"});
params.put("foo", "bar");
@@ -371,15 +391,15 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
mockHttpServletResponse.expectAndReturn("encodeURL", expectedString, expectedString);
String actionName = "promo.html";
- Map params = new TreeMap();
+ TreeMap<String, Object> params = new TreeMap<>();
String urlString = urlHelper.buildUrl(actionName, (HttpServletRequest) mockHttpServletRequest.proxy(), (HttpServletResponse) mockHttpServletResponse.proxy(), params, "https", true, true);
assertEquals(expectedString, urlString);
}
- public void testParseQuery() throws Exception {
- Map result = urlHelper.parseQueryString("aaa=aaaval&bbb=bbbval&ccc=&%3Ca%22%3E=%3Cval%3E", false);
+ public void testParseQuery() {
+ Map<String, Object> result = urlHelper.parseQueryString("aaa=aaaval&bbb=bbbval&ccc=&%3Ca%22%3E=%3Cval%3E", false);
assertEquals(result.get("aaa"), "aaaval");
assertEquals(result.get("bbb"), "bbbval");
@@ -387,38 +407,21 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
assertEquals(result.get("<a\">"), "<val>");
}
- public void testParseEmptyQuery() throws Exception {
- Map result = urlHelper.parseQueryString("", false);
+ public void testParseEmptyQuery() {
+ Map<String, Object> result = urlHelper.parseQueryString("", false);
assertNotNull(result);
assertEquals(result.size(), 0);
}
- public void testParseNullQuery() throws Exception {
- Map result = urlHelper.parseQueryString(null, false);
+ public void testParseNullQuery() {
+ Map<String, Object> result = urlHelper.parseQueryString(null, false);
assertNotNull(result);
assertEquals(result.size(), 0);
}
-
- public void testEncode() throws Exception {
- setProp(StrutsConstants.STRUTS_I18N_ENCODING, "UTF-8");
- String result = urlHelper.encode("\u65b0\u805e");
- String expectedResult = "%E6%96%B0%E8%81%9E";
-
- assertEquals(result, expectedResult);
- }
-
- public void testDecode() throws Exception {
- setProp(StrutsConstants.STRUTS_I18N_ENCODING, "UTF-8");
- String result = urlHelper.decode("%E6%96%B0%E8%81%9E");
- String expectedResult = "\u65b0\u805e";
-
- assertEquals(result, expectedResult);
- }
-
- public void testDecodeSpacesInQueryString() throws Exception {
+ public void testDecodeSpacesInQueryString() {
Map<String, Object> queryParameters = urlHelper.parseQueryString("name=value+with+space", false);
assertTrue(queryParameters.containsKey("name"));
@@ -428,18 +431,14 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
public void setUp() throws Exception {
super.setUp();
- stubContainer = new StubContainer(container);
+ StubContainer stubContainer = new StubContainer(container);
ActionContext.getContext().withContainer(stubContainer);
urlHelper = new DefaultUrlHelper();
urlHelper.setEncoder(new StrutsUrlEncoder());
urlHelper.setDecoder(new StrutsUrlDecoder());
}
- private void setProp(String key, String val) {
- stubContainer.overrides.put(key, val);
- }
-
- class StubContainer implements Container {
+ static class StubContainer implements Container {
Container parent;
@@ -448,7 +447,9 @@ public class DefaultUrlHelperTest extends StrutsInternalTestCase {
this.parent = parent;
}
- public Map<String, Object> overrides = new HashMap<String,Object>();
+ public Map<String, Object> overrides = new HashMap<>();
+
+ @SuppressWarnings("unchecked")
public <T> T getInstance(Class<T> type, String name) {
if (String.class.isAssignableFrom(type) && overrides.containsKey(name)) {
return (T) overrides.get(name);