Commits

Steve Lancashire committed fe773b0

Fix ConversionErrorInterceptor to prevent parameter values being passed to the OgnlValueStack to be evaluated as expressions

  • Participants
  • Parent commits 582c579
  • Branches CONF-27294, xwork_1-0-3 1
    1. xwork_1-0-3_branch

Comments (0)

Files changed (1)

File src/java/com/opensymphony/xwork/interceptor/ConversionErrorInterceptor.java

-/*
- * Copyright (c) 2002-2003 by OpenSymphony
- * All rights reserved.
- */
 package com.opensymphony.xwork.interceptor;
+ 
+import java.util.HashMap;
+import java.util.Map;
 
 import com.opensymphony.xwork.ActionContext;
 import com.opensymphony.xwork.ActionInvocation;
-import com.opensymphony.xwork.util.OgnlValueStack;
+import com.opensymphony.xwork.ValidationAware;
 import com.opensymphony.xwork.util.XWorkConverter;
-
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-
-
+ 
 /**
- * ConversionErrorInterceptor adds conversion errors from the ActionContext to the Action's field errors
- * @author Jason Carreira
- * Date: Nov 27, 2003 3:57:06 PM
+ * A simple {@link ConversionErrorInterceptor} that sets the value of the field directly back in the context rather than
+ * in the stack, which get evaluated as an OGNL expression. Workaround for:
+ * https://www.sec-consult.com/files/20120104-0_Apache_Struts2_Multiple_Critical_Vulnerabilities.txt
+ *
+ * @since v3.4
  */
-public class ConversionErrorInterceptor extends AroundInterceptor {
-    //~ Static fields/initializers /////////////////////////////////////////////
-
-    public static final String ORIGINAL_PROPERTY_OVERRIDE = "original.property.override";
-
-    //~ Methods ////////////////////////////////////////////////////////////////
-
-    protected Object getOverrideExpr(ActionInvocation invocation, Object value) {
-        return "'" + value + "'";
-    }
-
-    protected void after(ActionInvocation dispatcher, String result) throws Exception {
-    }
-
-    protected void before(ActionInvocation invocation) throws Exception {
-        ActionContext invocationContext = invocation.getInvocationContext();
-        Map conversionErrors = invocationContext.getConversionErrors();
-        OgnlValueStack stack = invocationContext.getValueStack();
-
-        HashMap fakie = null;
-
-        for (Iterator iterator = conversionErrors.entrySet().iterator();
-                iterator.hasNext();) {
-            Map.Entry entry = (Map.Entry) iterator.next();
-            String propertyName = (String) entry.getKey();
-            Object value = entry.getValue();
-
-            if (shouldAddError(propertyName, value)) {
-                String message = XWorkConverter.getConversionErrorMessage(propertyName, stack);
-                String addFieldErrorExpression = "addFieldError('" + propertyName + "','" + message + "')";
-                stack.findValue(addFieldErrorExpression);
-
-                if (fakie == null) {
-                    fakie = new HashMap();
-                }
-
-                fakie.put(propertyName, getOverrideExpr(invocation, value));
-            }
-        }
-
-        if (fakie != null) {
-            // if there were some errors, put the original (fake) values in place right before the result
-            stack.getContext().put(ORIGINAL_PROPERTY_OVERRIDE, fakie);
-            invocation.addPreResultListener(new PreResultListener() {
-                    public void beforeResult(ActionInvocation invocation, String resultCode) {
-                        Map fakie = (Map) invocation.getInvocationContext().get(ORIGINAL_PROPERTY_OVERRIDE);
-
-                        if (fakie != null) {
-                            invocation.getStack().setExprOverrides(fakie);
-                        }
-                    }
-                });
-        }
-    }
-
-    protected boolean shouldAddError(String propertyName, Object value) {
-        return true;
-    }
-}
+@SuppressWarnings({"unchecked"})
+public class ConversionErrorInterceptor implements Interceptor
+{
+   // ------------------------------------------------------------------------------------------------------- Constants
+   // ------------------------------------------------------------------------------------------------- Type Properties
+   // ---------------------------------------------------------------------------------------------------- Dependencies
+   // ---------------------------------------------------------------------------------------------------- Constructors
+   // ----------------------------------------------------------------------------------------------- Interface Methods
+   public void destroy()
+   {
+   }
+ 
+   public void init()
+   {
+   }
+ 
+   public String intercept(final ActionInvocation invocation) throws Exception
+   {
+       ActionContext invocationContext = invocation.getInvocationContext();
+       final Map<String, Object> conversionErrors = invocationContext.getConversionErrors();
+       
+       ValidationAware action = null;
+       if(invocation.getAction() instanceof ValidationAware)
+       {
+    	   action = (ValidationAware)action;
+       }
+       final Map<String, String> filteredConversionErrors = new HashMap<String, String>();
+ 
+       for (final Map.Entry<String, Object> stringObjectEntry : conversionErrors.entrySet())
+       {
+           Map.Entry<String, Object> entry = stringObjectEntry;
+           String propertyName = entry.getKey();
+           String value = getStringValue(entry.getValue());
+ 
+           if (value != null && !value.isEmpty())
+           {
+               filteredConversionErrors.put(propertyName, value);
+ 
+               if (action != null)
+               {
+                   String message = XWorkConverter.getConversionErrorMessage(propertyName, invocationContext.getValueStack());
+                   action.addFieldError(propertyName, message);
+               }
+           }
+       }
+ 
+       // Add the value back to the page context, doesn't go through all the OGNL malarky. Form fields will see the value in the action first though
+       invocation.addPreResultListener(new PreResultListener()
+       {
+           public void beforeResult(ActionInvocation invocation, String resultCode)
+           {
+               if (invocation.getStack() != null && invocation.getStack().getContext() != null)
+               {
+                   Map<String, Object> map = invocation.getStack().getContext();
+ 
+                   map.putAll(filteredConversionErrors);
+               }
+           }
+       });
+ 
+       return invocation.invoke();
+   }
+ 
+   // -------------------------------------------------------------------------------------------------- Public Methods
+   // -------------------------------------------------------------------------------------------------- Helper Methods
+   // -------------------------------------------------------------------------------------- Basic Accessors / Mutators
+   private static String getStringValue(final Object o)
+   {
+       if (o instanceof String[])
+       {
+           String[] strings = (String[]) o;
+           if (strings.length > 0)
+           {
+               return strings[0];
+           }
+           else
+           {
+               return null;
+           }
+       }
+       else
+       {
+           return (String) o;
+       }
+   }
+}