From 5e07b9574c66110b9a44338c6c7290a65be276f3 Mon Sep 17 00:00:00 2001 From: gregh3269 Date: Wed, 3 Aug 2016 10:50:44 +0100 Subject: [PATCH 1/4] ConversionErrorInterceptor to extend MethodFilterIntercept. See WW-4676 --- .../xwork2/interceptor/ConversionErrorInterceptor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java index 6a82e04767..f7234e7f8d 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java @@ -75,7 +75,7 @@ * * @author Jason Carreira */ -public class ConversionErrorInterceptor extends AbstractInterceptor { +public class ConversionErrorInterceptor extends MethodFilterInterceptor { public static final String ORIGINAL_PROPERTY_OVERRIDE = "original.property.override"; @@ -88,7 +88,7 @@ protected String escape(Object value) { } @Override - public String intercept(ActionInvocation invocation) throws Exception { + public String doIntercept(ActionInvocation invocation) throws Exception { ActionContext invocationContext = invocation.getInvocationContext(); Map conversionErrors = invocationContext.getConversionErrors(); From 50fa26533a43295e70db7eca6f4f7d8c69e0d2fe Mon Sep 17 00:00:00 2001 From: gregh3269 Date: Thu, 4 Aug 2016 08:31:32 +0100 Subject: [PATCH 2/4] Add java doc for MethodFilterInterceptor --- .../xwork2/interceptor/ConversionErrorInterceptor.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java index f7234e7f8d..67fc1ada5f 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java @@ -37,7 +37,13 @@ * display the original string ("abc") again rather than the int value (likely 0, which would make very little sense to * the user). *

- * + * + *

+ * Note: Since 2.5.2, this interceptor extends {@link MethodFilterInterceptor}, therefore being + * able to deal with excludeMethods / includeMethods parameters. See [Workflow Interceptor] + * (class {@link DefaultWorkflowInterceptor}) for documentation and examples on how to use this feature. + *

+ * * * *

Interceptor parameters:

From 51a49201adf73e33ba68d533f3535a32f507b531 Mon Sep 17 00:00:00 2001 From: gregh3269 Date: Thu, 4 Aug 2016 08:57:25 +0100 Subject: [PATCH 3/4] Revert "WW-4628: proper decoding of parameters in query-string" This reverts commit ef9c66118ede16f3ff239ea864641d5bdadeecae. --- .../apache/struts2/util/URLDecoderUtil.java | 12 ---- .../struts2/views/util/DefaultUrlHelper.java | 27 +++---- .../struts2/util/URLDecoderUtilTest.java | 7 -- .../views/util/DefaultUrlHelperTest.java | 71 +++++++++++++++---- 4 files changed, 64 insertions(+), 53 deletions(-) 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 3c61d1ec41..10f2a78554 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 static String decode(String sequence, String charset) { return UDecoder.URLDecode(sequence, charset); } - /** - * Decodes a x-www-form-urlencoded 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 true other decoding rules apply. - * @return the newly decoded String - * @exception IllegalArgumentException If the encoding is not valid - */ - public static String decode(String sequence, String charset, boolean isQueryString) { - return UDecoder.URLDecode(sequence, charset, isQueryString); - } - } 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 668d1a94ec..16739af632 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 String encode( String input ) { * @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 true 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 parseQueryString(String queryString, boolean forceValueArray) { Map queryParams = new LinkedHashMap(); if (queryString != null) { @@ -319,8 +308,8 @@ public Map parseQueryString(String queryString, boolean forceVal 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) 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 fd274bf343..f21c08fc8c 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 void testURLDecodeStringValidUtf8End() { 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 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 b4a5b26c42..57786c49be 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 void testBuildParametersStringWithJavaScriptInjected() throws Exception { 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\\\"","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 void testDecode() throws Exception { assertEquals(result, expectedResult); } - public void testDecodeSpacesInQueryString() throws Exception { - Map 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); } From 607fd00af00fb9a677dc69fdebcc42efedea735d Mon Sep 17 00:00:00 2001 From: gregh3269 Date: Thu, 4 Aug 2016 09:49:44 +0100 Subject: [PATCH 4/4] ConversionErrorInterceptor to extend MethodFilterIntercept See WW-4676 --- .../interceptor/ConversionErrorInterceptorTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java b/core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java index 0e34ae1f9f..33adf7ea4e 100644 --- a/core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java @@ -48,7 +48,7 @@ public void testFieldErrorAdded() throws Exception { stack.push(action); mockInvocation.matchAndReturn("getAction", action); assertNull(action.getFieldErrors().get("foo")); - interceptor.intercept(invocation); + interceptor.doIntercept(invocation); assertTrue(action.hasFieldErrors()); assertNotNull(action.getFieldErrors().get("foo")); } @@ -61,7 +61,7 @@ public void testFieldErrorWithMapKeyAdded() throws Exception { stack.push(action); mockInvocation.matchAndReturn("getAction", action); assertNull(action.getFieldErrors().get(fieldName)); - interceptor.intercept(invocation); + interceptor.doIntercept(invocation); assertTrue(action.hasFieldErrors()); // This fails! assertNotNull(action.getFieldErrors().get(fieldName)); } @@ -76,7 +76,7 @@ public void testWithPreResultListener() throws Exception { assertNull(action.getFieldErrors().get("foo")); assertEquals(55, stack.findValue("foo")); - interceptor.intercept(mai); + interceptor.doIntercept(mai); assertTrue(action.hasFieldErrors()); assertNotNull(action.getFieldErrors().get("foo")); @@ -100,7 +100,7 @@ public void testWithPreResultListenerAgainstMaliciousCode() throws Exception { assertNull(action.getFieldErrors().get("foo")); assertEquals(55, stack.findValue("foo")); - interceptor.intercept(mai); + interceptor.doIntercept(mai); assertTrue(action.hasFieldErrors()); assertNotNull(action.getFieldErrors().get("foo"));