Merge pull request #461 from apache/WW-5063-null-check

[WW-5063] Adds null check for ActionInvocation

core/src/main/java/com/opensymphony/xwork2/ActionChainResult.java

@@ -202,6 +202,10 @@ public class ActionChainResult implements Result {
      * @param invocation the DefaultActionInvocation calling the action call stack
      */
     public void execute(ActionInvocation invocation) throws Exception {
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
         ValueStack stack = invocation.getInvocationContext().getValueStack();
         String finalNamespace = this.namespace != null
                 ? TextParseUtil.translateVariables(namespace, stack)

core/src/main/java/com/opensymphony/xwork2/Result.java

@@ -45,6 +45,6 @@ public interface Result extends Serializable {
      * @param invocation  the invocation context.
      * @throws Exception can be thrown.
      */
-    public void execute(ActionInvocation invocation) throws Exception;
+    void execute(ActionInvocation invocation) throws Exception;
 
 }

core/src/main/java/org/apache/struts2/result/HttpHeaderResult.java

@@ -172,8 +172,12 @@ public class HttpHeaderResult implements Result {
      * @throws Exception if an error occurs when re-setting the headers.
      */
     public void execute(ActionInvocation invocation) throws Exception {
-        HttpServletResponse response = ServletActionContext.getResponse();
-        ValueStack stack = ActionContext.getContext().getValueStack();
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
+        HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
+        ValueStack stack = invocation.getStack();
 
         if (status != -1) {
             response.setStatus(status);

core/src/main/java/org/apache/struts2/result/PlainResult.java

@@ -45,6 +45,10 @@ public interface PlainResult extends Result {
 
     @Override
     default void execute(ActionInvocation invocation) throws Exception {
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
         LOG.debug("Executing plain result");
         ResponseBuilder builder = new ResponseBuilder();
         write(builder);

core/src/main/java/org/apache/struts2/result/PostbackResult.java

@@ -110,6 +110,10 @@ public class PostbackResult extends StrutsResultSupport {
 
     @Override
     public void execute(ActionInvocation invocation) throws Exception {
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
         String postbackUri = makePostbackUri(invocation);
         setLocation(postbackUri);
         super.execute(invocation);

core/src/main/java/org/apache/struts2/result/ServletActionRedirectResult.java

@@ -158,6 +158,10 @@ public class ServletActionRedirectResult extends ServletRedirectResult implement
      * @see com.opensymphony.xwork2.Result#execute(com.opensymphony.xwork2.ActionInvocation)
      */
     public void execute(ActionInvocation invocation) throws Exception {
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
         actionName = conditionalParse(actionName, invocation);
         parseLocation = false;
         if (namespace == null) {

core/src/main/java/org/apache/struts2/result/ServletRedirectResult.java

@@ -143,6 +143,10 @@ public class ServletRedirectResult extends StrutsResultSupport implements Reflec
     }
 
     public void execute(ActionInvocation invocation) throws Exception {
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
         if (anchor != null) {
             anchor = conditionalParse(anchor, invocation);
         }

core/src/main/java/org/apache/struts2/views/xslt/XSLTResult.java

@@ -146,6 +146,10 @@ public class XSLTResult implements Result {
     }
 
     public void execute(ActionInvocation invocation) throws Exception {
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
         long startTime = System.currentTimeMillis();
         String location = getStylesheetLocation();
 
@@ -154,12 +158,12 @@ public class XSLTResult implements Result {
         }
 
         if (parse) {
-            ValueStack stack = ActionContext.getContext().getValueStack();
+            ValueStack stack = invocation.getStack();
             location = TextParseUtil.translateVariables(location, stack);
         }
 
         try {
-            HttpServletResponse response = ServletActionContext.getResponse();
+            HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
             response.setStatus(status);
             response.setCharacterEncoding(encoding);
             PrintWriter writer = response.getWriter();

core/src/site/resources/tags/action-attributes.html

@@ -67,6 +67,6 @@
             <td align="left" valign="top"></td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack</td>
+            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack (scope: action).</td>
         </tr>
 </table>

core/src/site/resources/tags/bean-attributes.html

@@ -27,6 +27,6 @@
             <td align="left" valign="top"></td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack</td>
+            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack (scope: action).</td>
         </tr>
 </table>

core/src/site/resources/tags/date-attributes.html

@@ -51,6 +51,6 @@
             <td align="left" valign="top"></td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack</td>
+            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack (scope: action).</td>
         </tr>
 </table>

core/src/site/resources/tags/iterator-attributes.html

@@ -59,6 +59,6 @@
             <td align="left" valign="top"></td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack</td>
+            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack (scope: action).</td>
         </tr>
 </table>

core/src/site/resources/tags/number-attributes.html

@@ -99,6 +99,6 @@
             <td align="left" valign="top"></td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack</td>
+            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack (scope: action).</td>
         </tr>
 </table>

core/src/site/resources/tags/set-attributes.html

@@ -19,7 +19,7 @@
             <td align="left" valign="top">action</td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">The scope in which to assign the variable. Can be <b>application</b>, <b>session</b>, <b>request</b>, <b>page</b>, or <b>action</b>.</td>
+            <td align="left" valign="top">The scope in which to assign the variable. Can be <b>application</b>, <b>session</b>, <b>request</b>, <b>page</b>, or <b>action</b> (action scope <em>also</em> adds it to the page scope).</td>
         </tr>
         <tr>
             <td align="left" valign="top">trimBody</td>
@@ -43,6 +43,6 @@
             <td align="left" valign="top"></td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack</td>
+            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack (default scope: action,<em>override</em> with the scope attribute).</td>
         </tr>
 </table>

core/src/site/resources/tags/text-attributes.html

@@ -59,6 +59,6 @@
             <td align="left" valign="top"></td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack</td>
+            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack (scope: action).</td>
         </tr>
 </table>

core/src/site/resources/tags/url-attributes.html

@@ -123,7 +123,7 @@
             <td align="left" valign="top"></td>
             <td align="left" valign="top">false</td>
             <td align="left" valign="top">String</td>
-            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack</td>
+            <td align="left" valign="top">Name used to reference the value pushed into the Value Stack (scope: action).</td>
         </tr>
         <tr>
             <td align="left" valign="top">windowState</td>

core/src/test/java/com/opensymphony/xwork2/ActionChainResultTest.java

@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.opensymphony.xwork2;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+public class ActionChainResultTest {
+
+    @Test
+    public void testPassingNullInvocation() throws Exception{
+        Result result = new ActionChainResult();
+        try {
+            result.execute(null);
+            fail("Exception should be thrown!");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invocation cannot be null!", e.getMessage());
+        }
+    }
+
+}

core/src/test/java/org/apache/struts2/result/HttpHeaderResultTest.java

@@ -25,7 +25,6 @@ import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.util.reflection.ReflectionProvider;
 import org.apache.struts2.ServletActionContext;
 import org.apache.struts2.StrutsInternalTestCase;
-import org.apache.struts2.result.HttpHeaderResult;
 
 import javax.servlet.http.HttpServletResponse;
 import java.util.HashMap;
@@ -36,18 +35,19 @@ import java.util.Map;
  */
 public class HttpHeaderResultTest extends StrutsInternalTestCase {
 
-    ActionInvocation invocation;
-    HttpHeaderResult result;
-    HttpServletResponse response;
-    Mock responseMock;
-    ReflectionProvider reflectionProvider;
+    private Mock invocationMock;
+    private ActionInvocation invocation;
+    private HttpHeaderResult result;
+    private HttpServletResponse response;
+    private Mock responseMock;
+    private ReflectionProvider reflectionProvider;
 
     public void testHeaderValuesAreNotParsedWhenParseIsFalse() throws Exception {
-        Map<String, String> params = new HashMap<String, String>();
+        Map<String, String> params = new HashMap<>();
         params.put("headers.foo", "${bar}");
         params.put("headers.baz", "baz");
 
-        Map<String, String> values = new HashMap<String, String>();
+        Map<String, String> values = new HashMap<>();
         values.put("bar", "abc");
         ActionContext.getContext().getValueStack().push(values);
 
@@ -61,11 +61,11 @@ public class HttpHeaderResultTest extends StrutsInternalTestCase {
     }
 
     public void testHeaderValuesAreParsedAndSet() throws Exception {
-        Map<String, String> params = new HashMap<String, String>();
+        Map<String, String> params = new HashMap<>();
         params.put("headers.foo", "${bar}");
         params.put("headers.baz", "baz");
 
-        Map<String, String> values = new HashMap<String, String>();
+        Map<String, String> values = new HashMap<>();
         values.put("bar", "abc");
         ActionContext.getContext().getValueStack().push(values);
 
@@ -113,12 +113,24 @@ public class HttpHeaderResultTest extends StrutsInternalTestCase {
         responseMock.verify();
     }
 
+    public void testPassingNullInvocation() throws Exception {
+        try {
+            result.execute(null);
+            fail("Exception should be thrown!");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invocation cannot be null!", e.getMessage());
+        }
+    }
+
     protected void setUp() throws Exception {
         super.setUp();
         result = new HttpHeaderResult();
         responseMock = new Mock(HttpServletResponse.class);
         response = (HttpServletResponse) responseMock.proxy();
-        invocation = (ActionInvocation) new Mock(ActionInvocation.class).proxy();
+        invocationMock = new Mock(ActionInvocation.class);
+        invocationMock.expectAndReturn("getInvocationContext", ActionContext.getContext());
+        invocationMock.expectAndReturn("getStack", ActionContext.getContext().getValueStack());
+        invocation = (ActionInvocation) invocationMock.proxy();
         reflectionProvider = container.getInstance(ReflectionProvider.class);
         ServletActionContext.setResponse(response);
     }

core/src/test/java/org/apache/struts2/result/PlainResultTest.java

@@ -19,6 +19,7 @@
 package org.apache.struts2.result;
 
 import com.opensymphony.xwork2.ActionContext;
+import com.opensymphony.xwork2.Result;
 import com.opensymphony.xwork2.mock.MockActionInvocation;
 import org.apache.struts2.StrutsException;
 import org.apache.struts2.StrutsInternalTestCase;
@@ -127,6 +128,16 @@ public class PlainResultTest extends StrutsInternalTestCase {
         }
     }
 
+    public void testPassingNullInvocation() throws Exception{
+        Result result = (PlainResult) response -> response.write("ignore");
+        try {
+            result.execute(null);
+            fail("Exception should be thrown!");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invocation cannot be null!", e.getMessage());
+        }
+    }
+
     public void setUp() throws Exception {
         super.setUp();
         invocation = new MockActionInvocation();

core/src/test/java/org/apache/struts2/result/PostbackResultTest.java

@@ -21,6 +21,7 @@ package org.apache.struts2.result;
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.ActionProxy;
+import com.opensymphony.xwork2.Result;
 import com.opensymphony.xwork2.util.ValueStack;
 import org.apache.struts2.ServletActionContext;
 import org.apache.struts2.StrutsInternalTestCase;
@@ -32,7 +33,6 @@ import org.springframework.mock.web.MockHttpServletResponse;
 import static org.easymock.EasyMock.createControl;
 import static org.easymock.EasyMock.expect;
 
-
 public class PostbackResultTest extends StrutsInternalTestCase {
 
     public void testWithNoNamespace() throws Exception {
@@ -93,4 +93,15 @@ public class PostbackResultTest extends StrutsInternalTestCase {
         control.verify();
     }
 
+    public void testPassingNullInvocation() throws Exception{
+        Result result = new PostbackResult();
+        try {
+            result.execute(null);
+            fail("Exception should be thrown!");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invocation cannot be null!", e.getMessage());
+        }
+    }
+
+
 }

core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java

@@ -23,6 +23,7 @@ import com.mockobjects.dynamic.Mock;
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.ActionProxy;
+import com.opensymphony.xwork2.Result;
 import com.opensymphony.xwork2.config.entities.ActionConfig;
 import com.opensymphony.xwork2.config.entities.PackageConfig;
 import com.opensymphony.xwork2.config.entities.ResultConfig;
@@ -376,7 +377,7 @@ public class ServletRedirectResultTest extends StrutsInternalTestCase implements
      * the desired log warning when an IllegalStateException is thrown for statusCode SC_FOUND.
      */
     public void testSendRedirectSCFoundIllegalStateException() {
-        HttpServletResponse httpServletResponseMock = (HttpServletResponse) createMock(HttpServletResponse.class);
+        HttpServletResponse httpServletResponseMock = createMock(HttpServletResponse.class);
         boolean iseCaught = false;
         view.setLocation("/bar/foo.jsp");
         view.setStatusCode(HttpServletResponse.SC_FOUND);
@@ -432,6 +433,16 @@ public class ServletRedirectResultTest extends StrutsInternalTestCase implements
         }
     }
 
+    public void testPassingNullInvocation() throws Exception{
+        Result result = new ServletRedirectResult();
+        try {
+            result.execute(null);
+            fail("Exception should be thrown!");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invocation cannot be null!", e.getMessage());
+        }
+    }
+
     protected void setUp() throws Exception {
         super.setUp();
         configurationManager.getConfiguration().

core/src/test/java/org/apache/struts2/views/xslt/XSLTResultTest.java

@@ -19,7 +19,9 @@
 package org.apache.struts2.views.xslt;
 
 import com.opensymphony.xwork2.Action;
+import com.opensymphony.xwork2.ActionChainResult;
 import com.opensymphony.xwork2.ActionContext;
+import com.opensymphony.xwork2.Result;
 import com.opensymphony.xwork2.mock.MockActionInvocation;
 import com.opensymphony.xwork2.util.ClassLoaderUtil;
 import com.opensymphony.xwork2.util.ValueStack;
@@ -241,6 +243,16 @@ public class XSLTResultTest extends StrutsInternalTestCase {
         assertEquals(actual, "ISO-8859-1");
     }
 
+    public void testPassingNullInvocation() throws Exception{
+        Result result = new XSLTResult();
+        try {
+            result.execute(null);
+            fail("Exception should be thrown!");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invocation cannot be null!", e.getMessage());
+        }
+    }
+
     protected void setUp() throws Exception {
         super.setUp();
         request = new MockHttpServletRequest();

plugins/gxp/src/main/java/org/apache/struts2/views/gxp/GxpResult.java

@@ -104,9 +104,13 @@ public class GxpResult extends AbstractGxpResult {
     /**
      * Tells the GXP to write itself to the output stream.
      *
-     * @param actionInvocation the action invocation
+     * @param invocation the action invocation
      */
-    public void execute(ActionInvocation actionInvocation) {
+    public void execute(ActionInvocation invocation) {
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
         GxpResourceProvider provider = getProvider();
         try {
             getGxpClosure().write(provider.getWriter(), new GxpContext(provider.getLocale(), outputXml));
@@ -114,7 +118,7 @@ public class GxpResult extends AbstractGxpResult {
             throw new RuntimeException("Exception while rendering "
                     + getGxpName()
                     + " coming from "
-                    + actionInvocation.getAction().getClass().getName() + ".",
+                    + invocation.getAction().getClass().getName() + ".",
                     e);
         }
     }

plugins/json/src/main/java/org/apache/struts2/json/JSONResult.java

@@ -185,6 +185,10 @@ public class JSONResult implements Result {
     }
 
     public void execute(ActionInvocation invocation) throws Exception {
+        if (invocation == null) {
+            throw new IllegalArgumentException("Invocation cannot be null!");
+        }
+
         ActionContext actionContext = invocation.getInvocationContext();
         HttpServletRequest request = actionContext.getServletRequest();
         HttpServletResponse response = actionContext.getServletResponse();

plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java

@@ -35,6 +35,7 @@ import java.util.regex.Pattern;
 
 import javax.servlet.http.HttpServletResponse;
 
+import com.opensymphony.xwork2.Result;
 import org.apache.struts2.StrutsStatics;
 import org.apache.struts2.StrutsTestCase;
 import org.apache.struts2.util.TestUtils;
@@ -713,6 +714,16 @@ public class JSONResultTest extends StrutsTestCase {
         assertEquals("UTF-8", encoding);
     }
 
+    public void testPassingNullInvocation() throws Exception{
+        Result result = new JSONResult();
+        try {
+            result.execute(null);
+            fail("Exception should be thrown!");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invocation cannot be null!", e.getMessage());
+        }
+    }
+
     @Override
     protected void setUp() throws Exception {
         super.setUp();

plugins/portlet/src/main/java/org/apache/struts2/portlet/result/PortletActionRedirectResult.java

@@ -171,6 +171,10 @@ public class PortletActionRedirectResult extends PortletResult {
 	 * @see com.opensymphony.xwork2.Result#execute(com.opensymphony.xwork2.ActionInvocation)
 	 */
 	public void execute(ActionInvocation invocation) throws Exception {
+		if (invocation == null) {
+			throw new IllegalArgumentException("Invocation cannot be null!");
+		}
+
 		actionName = conditionalParse(actionName, invocation);
 		parseLocation = false;