ConversionErrorInterceptor to extend MethodFilterIntercept

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).
  * </p>
- *
+ * 
+ * <p>
+ * <b>Note:</b> 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.
+ * </p>
+ * 
  * <!-- END SNIPPET: description -->
  *
  * <p><u>Interceptor parameters:</u></p>
@@ -75,7 +81,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 +94,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<String, Object> conversionErrors = invocationContext.getConversionErrors();

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 <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);
-    }
-
 }

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 <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 Map<String, Object> 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)

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"));

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);
-    }
-
 }

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&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";
 
@@ -396,11 +423,25 @@ public void testDecode() throws Exception {
         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);
     }