struts-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (WW-4900) NotSerializableException: com.opensymphony.xwork2.inject.ContainerImpl$ConstructorInjector when using ExecuteAndWait interceptor
Date Tue, 12 Dec 2017 16:43:00 GMT

    [ https://issues.apache.org/jira/browse/WW-4900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16287853#comment-16287853
] 

ASF GitHub Bot commented on WW-4900:
------------------------------------

yasserzamani closed pull request #188: WW-4900 WW-4873 Fixes DefaultActionInvocation &
BackgroundProcess serialization
URL: https://github.com/apache/struts/pull/188
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/src/main/java/com/opensymphony/xwork2/ActionInvocation.java b/core/src/main/java/com/opensymphony/xwork2/ActionInvocation.java
index 908349806..58d2f5320 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ActionInvocation.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ActionInvocation.java
@@ -180,14 +180,14 @@
 
     /**
      * Prepares instance of ActionInvocation to be serializable,
-     * which simple means removing all unserializable fields, eg. Container
+     * which simply means removing all unserializable but restorable references
      *
      * @return ActionInvocation which can be serialize (eg. into HttpSession)
      */
     ActionInvocation serialize();
 
     /**
-     * Performs opposite process to restore back ActionInvocation after deserialisation
+     * Performs opposite process to restore back ActionInvocation after serialisation
      *
      * @param actionContext current {@link ActionContext}
      * @return fully operational ActionInvocation
diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java
index 5777007c1..6a3bf59f8 100644
--- a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java
+++ b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java
@@ -36,6 +36,8 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -501,24 +503,57 @@ protected String saveResult(ActionConfig actionConfig, Object methodResult)
{
     }
 
     /**
-     * Version ready to be serialize
+     * Version ready to be serialize via removing all unserializable but restorable references
      *
-     * @return instance without reference to {@link Container}
+     * @return instance without unserializable references
      */
     public ActionInvocation serialize() {
         DefaultActionInvocation that = this;
         that.container = null;
+
+        if(that.getInvocationContext() != null && that.getInvocationContext().getContextMap()
!= null) {
+            Map<String, Object> thatContextMap = that.getInvocationContext().getContextMap();
+            List<String> keysToRemove = new ArrayList<>();
+            for (Map.Entry<String, Object> entry : thatContextMap.entrySet()) {
+                Object entryValue = entry.getValue();
+                // WW-4873 Remove Servlet Request and Response as they are not intended to
being serializable
+                if (entryValue instanceof ServletRequest ||
+                        entryValue instanceof ServletResponse) {
+                    keysToRemove.add(entry.getKey());
+                }
+            }
+            for (String keyToRemove : keysToRemove) {
+                thatContextMap.remove(keyToRemove);
+            }
+        }
+
         return that;
     }
 
     /**
-     * Restoring Container
+     * Restoring references
      *
      * @param actionContext current {@link ActionContext}
      * @return instance which can be used to invoke action
      */
     public ActionInvocation deserialize(ActionContext actionContext) {
         DefaultActionInvocation that = this;
+
+        if(that.getInvocationContext() != null && that.getInvocationContext().getContextMap()
!= null) {
+            Map<String, Object> thatContextMap = that.getInvocationContext().getContextMap();
+            Map<String, Object> acContextMap = actionContext.getContextMap();
+            for (Map.Entry<String, Object> entry : acContextMap.entrySet()) {
+                Object entryValue = entry.getValue();
+                String entryKey = entry.getKey();
+                // WW-4873 Restore Servlet Request and Response
+                if ((entryValue instanceof ServletRequest ||
+                        entryValue instanceof ServletResponse) &&
+                        !thatContextMap.containsKey(entryKey)) {
+                    thatContextMap.put(entryKey, entryValue);
+                }
+            }
+        }
+
         that.container = actionContext.getContainer();
         return that;
     }
diff --git a/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java b/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java
index 20b330eae..f7ca061d0 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java
@@ -32,7 +32,6 @@
     private static final long serialVersionUID = 3884464776311686443L;
 
     protected Object action;
-    protected ActionInvocation invocation;
     protected String result;
     protected Exception exception;
     protected boolean done;
@@ -45,13 +44,12 @@
      * @param threadPriority The thread priority
      */
     public BackgroundProcess(String threadName, final ActionInvocation invocation, int threadPriority)
{
-        this.invocation = invocation;
         this.action = invocation.getAction();
         try {
             final Thread t = new Thread(new Runnable() {
                 public void run() {
                     try {
-                        beforeInvocation();
+                        beforeInvocation(invocation);
                         result = invocation.invokeActionOnly();
                         afterInvocation();
                     } catch (Exception e) {
@@ -75,7 +73,7 @@ public void run() {
      *
      * @throws Exception any exception thrown will be thrown, in turn, by the ExecuteAndWaitInterceptor
      */
-    protected void beforeInvocation() throws Exception {
+    protected void beforeInvocation(ActionInvocation invocation) throws Exception {
         ActionContext.setContext(invocation.getInvocationContext());
     }
 
@@ -99,15 +97,6 @@ public Object getAction() {
         return action;
     }
 
-    /**
-     * Retrieves the action invocation.
-     *
-     * @return the action invocation
-     */
-    public ActionInvocation getInvocation() {
-        return invocation;
-    }
-
     /**
      * Gets the result of the background process.
      *
diff --git a/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java
b/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java
index 2b3e063e0..4e43e6acb 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java
@@ -178,6 +178,11 @@
     private static final Logger LOG = LogManager.getLogger(ExecuteAndWaitInterceptor.class);
 
     public static final String KEY = "__execWait";
+    /**
+     * WW-4900 Control the behaviour when we miss the background thread after session deserialization
+     * @since 2.6
+     */
+    public static final String KEY_HASHCODE = KEY + "_hashCode";
     public static final String WAIT = "wait";
     protected int delay;
     protected int delaySleepInterval = 100; // default sleep 100 millis before checking if
background process is done
@@ -243,9 +248,17 @@ protected String doIntercept(ActionInvocation actionInvocation) throws
Exception
         synchronized (httpSession) {
             BackgroundProcess bp = (BackgroundProcess) session.get(KEY + name);
 
+            Integer bpHashCode = (Integer) session.get(KEY_HASHCODE + name);
+            if (bp != null && bpHashCode != null && bp.hashCode() != bpHashCode)
{
+                //WW-4900 Is background thread missed? let's start a new one
+                session.remove(KEY + name);
+                bp = null;
+            }
+
             if ((!executeAfterValidationPass || secondTime) && bp == null) {
                 bp = getNewBackgroundProcess(name, actionInvocation, threadPriority);
                 session.put(KEY + name, bp);
+                session.put(KEY_HASHCODE + name, bp.hashCode());
                 performInitialDelay(bp); // first time let some time pass before showing
wait page
                 secondTime = false;
             }
diff --git a/core/src/main/java/org/apache/struts2/interceptor/TokenSessionStoreInterceptor.java
b/core/src/main/java/org/apache/struts2/interceptor/TokenSessionStoreInterceptor.java
index e08fb07be..6957e41b1 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/TokenSessionStoreInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/TokenSessionStoreInterceptor.java
@@ -28,7 +28,6 @@
 import org.apache.struts2.util.TokenHelper;
 
 import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
 /**
@@ -117,7 +116,6 @@ protected String handleInvalidToken(ActionInvocation invocation) throws
Exceptio
         ActionContext ac = invocation.getInvocationContext();
 
         HttpServletRequest request = (HttpServletRequest) ac.get(ServletActionContext.HTTP_REQUEST);
-        HttpServletResponse response = (HttpServletResponse) ac.get(ServletActionContext.HTTP_RESPONSE);
         String tokenName = TokenHelper.getTokenName();
         String token = TokenHelper.getToken(tokenName);
 
@@ -134,9 +132,6 @@ protected String handleInvalidToken(ActionInvocation invocation) throws
Exceptio
                 ValueStack stack = savedInvocation.getStack();
                 request.setAttribute(ServletActionContext.STRUTS_VALUESTACK_KEY, stack);
 
-                ActionContext savedContext = savedInvocation.getInvocationContext();
-                savedContext.getContextMap().put(ServletActionContext.HTTP_REQUEST, request);
-                savedContext.getContextMap().put(ServletActionContext.HTTP_RESPONSE, response);
                 Result result = savedInvocation.getResult();
 
                 if ((result != null) && (savedInvocation.getProxy().getExecuteResult()))
{
diff --git a/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java b/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java
index 334839082..19e4c87d0 100644
--- a/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java
@@ -25,12 +25,15 @@
 import com.opensymphony.xwork2.mock.MockActionProxy;
 import com.opensymphony.xwork2.mock.MockContainer;
 import com.opensymphony.xwork2.mock.MockInterceptor;
-import com.opensymphony.xwork2.mock.MockLazyInterceptor;
 import com.opensymphony.xwork2.ognl.OgnlUtil;
 import com.opensymphony.xwork2.util.ValueStack;
 import com.opensymphony.xwork2.util.ValueStackFactory;
 import org.apache.struts2.dispatcher.HttpParameters;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
 
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectOutputStream;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -44,6 +47,8 @@
  * @author <a href="mailto:kristian at zenior.no">Kristian Rosenvold</a>
  */
 public class DefaultActionInvocationTest extends XWorkTestCase {
+    private static final String HTTP_REQUEST = "com.opensymphony.xwork2.DefaultActionInvocationTest.HttpServletRequest";
+    private static final String HTTP_RESPONSE = "com.opensymphony.xwork2.DefaultActionInvocationTest.HttpServletResponse";
 
     /**
      * Tests interceptor chain invoke.
@@ -77,23 +82,53 @@ public void testInvoke() throws Exception {
 
     public void testSerialization() throws Exception {
         // given
-        DefaultActionInvocation actionInvocation = new DefaultActionInvocation(new HashMap<String,
Object>(), false);
+        HashMap<String, Object> extraContext = new HashMap<String, Object>();
+        DefaultActionInvocation actionInvocation = new DefaultActionInvocation(extraContext,
false);
         actionInvocation.setContainer(new MockContainer());
 
+        extraContext.put(HTTP_REQUEST, new MockHttpServletRequest());
+        extraContext.put(HTTP_RESPONSE, new MockHttpServletResponse());
+        actionInvocation.invocationContext = new ActionContext(extraContext);
+
         // when
         DefaultActionInvocation serializable = (DefaultActionInvocation) actionInvocation.serialize();
 
         // then
         assertNull(actionInvocation.container);
         assertNull(serializable.container);
+
+        assertFalse("request should be removed from actionInvocation",
+                actionInvocation.invocationContext.getContextMap().containsKey(HTTP_REQUEST));
+        assertFalse("response should be removed from actionInvocation",
+                actionInvocation.invocationContext.getContextMap().containsKey(HTTP_RESPONSE));
+        assertFalse("request should be removed from serializable",
+                serializable.invocationContext.getContextMap().containsKey(HTTP_REQUEST));
+        assertFalse("response should be removed from serializable",
+                serializable.invocationContext.getContextMap().containsKey(HTTP_RESPONSE));
+
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(baos);
+        oos.writeObject(serializable);
+        oos.close();
+        assertTrue("should have serialized data", baos.size() > 0);
+        baos.close();
     }
 
     public void testDeserialization() throws Exception {
         // given
-        DefaultActionInvocation actionInvocation = new DefaultActionInvocation(new HashMap<String,
Object>(), false);
+        HashMap<String, Object> extraContext = new HashMap<String, Object>();
+        DefaultActionInvocation actionInvocation = new DefaultActionInvocation(extraContext,
false);
+        actionInvocation.invocationContext = new ActionContext(extraContext);
+
         MockContainer mockContainer = new MockContainer();
         ActionContext.getContext().setContainer(mockContainer);
 
+        Map<String, Object> acContextMap = ActionContext.getContext().getContextMap();
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        acContextMap.put(HTTP_REQUEST, request);
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        acContextMap.put(HTTP_RESPONSE, response);
+
         // when
         DefaultActionInvocation deserializable = (DefaultActionInvocation) actionInvocation.deserialize(ActionContext.getContext());
 
@@ -101,6 +136,20 @@ public void testDeserialization() throws Exception {
         assertNotNull(actionInvocation.container);
         assertNotNull(deserializable.container);
         assertEquals(mockContainer, deserializable.container);
+
+        assertTrue("request should be restored into actionInvocation",
+                actionInvocation.invocationContext.getContextMap().containsKey(HTTP_REQUEST));
+        assertTrue("response should be restored into actionInvocation",
+                actionInvocation.invocationContext.getContextMap().containsKey(HTTP_RESPONSE));
+        assertTrue("request should be restored into deserializable",
+                deserializable.invocationContext.getContextMap().containsKey(HTTP_REQUEST));
+        assertTrue("response should be restored into deserializable",
+                deserializable.invocationContext.getContextMap().containsKey(HTTP_RESPONSE));
+
+        assertEquals(request, actionInvocation.invocationContext.getContextMap().get(HTTP_REQUEST));
+        assertEquals(response, actionInvocation.invocationContext.getContextMap().get(HTTP_RESPONSE));
+        assertEquals(request, deserializable.invocationContext.getContextMap().get(HTTP_REQUEST));
+        assertEquals(response, deserializable.invocationContext.getContextMap().get(HTTP_RESPONSE));
     }
 
     public void testInvokingExistingExecuteMethod() throws Exception {
diff --git a/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java
b/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java
new file mode 100644
index 000000000..4969e85fc
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java
@@ -0,0 +1,51 @@
+/*
+ * 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 org.apache.struts2.interceptor;
+
+import com.opensymphony.xwork2.ActionContext;
+import com.opensymphony.xwork2.mock.MockActionInvocation;
+import org.apache.struts2.StrutsInternalTestCase;
+
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectOutputStream;
+
+/**
+ * Test case for BackgroundProcessTest.
+ */
+public class BackgroundProcessTest extends StrutsInternalTestCase {
+
+    public void testIsSerializable() throws Exception {
+        MockActionInvocation invocation = new MockActionInvocation();
+        invocation.setResultCode("BackgroundProcessTest.testIsSerializable");
+        invocation.setInvocationContext(ActionContext.getContext());
+
+        BackgroundProcess bp = new BackgroundProcess("BackgroundProcessTest.testIsSerializable",
invocation
+                , Thread.MIN_PRIORITY);
+
+        bp.exception = new Exception();
+        bp.done = true;
+
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(baos);
+        oos.writeObject(bp);
+        oos.close();
+        assertTrue("should have serialized data", baos.size() > 0);
+        baos.close();
+    }
+}
diff --git a/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitDelayAction.java
b/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitDelayAction.java
index 67e270731..e781051f3 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitDelayAction.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitDelayAction.java
@@ -20,11 +20,13 @@
 
 import com.opensymphony.xwork2.Action;
 
+import java.io.Serializable;
+
 /**
  * Used by ExecuteAndWaitInterceptorTest.
  *
  */
-public class ExecuteAndWaitDelayAction implements Action {
+public class ExecuteAndWaitDelayAction implements Action, Serializable {
 
     public String execute() throws Exception {
         Thread.sleep(500);
diff --git a/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java
index 9106a1b6a..fd551b26d 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java
@@ -38,6 +38,10 @@
 import org.apache.struts2.views.jsp.StrutsMockHttpSession;
 
 import javax.servlet.http.HttpSession;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -159,6 +163,41 @@ public void testWaitDelayAndJobAlreadyDone2() throws Exception {
         assertTrue("Job done already after 500 so there should not be such long delay", diff
<= 1000);
     }
 
+    public void testFromDeserializedSession() throws Exception {
+        waitInterceptor.setDelay(0);
+        waitInterceptor.setDelaySleepInterval(0);
+
+        ActionProxy proxy = buildProxy("action1");
+        String result = proxy.execute();
+        assertEquals("wait", result);
+
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(baos);
+        oos.writeObject(session);
+        oos.close();
+        byte b[] = baos.toByteArray();
+        baos.close();
+
+        ByteArrayInputStream bais = new ByteArrayInputStream(b);
+        ObjectInputStream ois = new ObjectInputStream(bais);
+        session = (Map) ois.readObject();
+        context.put(ActionContext.SESSION, session);
+        ois.close();
+        bais.close();
+
+        Thread.sleep(1000);
+
+        ActionProxy proxy2 = buildProxy("action1");
+        String result2 = proxy2.execute();
+        assertEquals("wait", result2);
+
+        Thread.sleep(1000);
+
+        ActionProxy proxy3 = buildProxy("action1");
+        String result3 = proxy3.execute();
+        assertEquals("success", result3);
+    }
+
     protected ActionProxy buildProxy(String actionName) throws Exception {
         return actionProxyFactory.createActionProxy("", actionName, null, context);
     }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> NotSerializableException: com.opensymphony.xwork2.inject.ContainerImpl$ConstructorInjector
when using ExecuteAndWait interceptor
> --------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: WW-4900
>                 URL: https://issues.apache.org/jira/browse/WW-4900
>             Project: Struts 2
>          Issue Type: Bug
>    Affects Versions: 2.5.14.1
>            Reporter: Erica Kane
>            Assignee: Yasser Zamani
>             Fix For: 2.5.15
>
>
> We are running Struts 2.5.14.1 and working on externalizing Tomcat session state. This
requires Serializable sessions. However, our Action with the ExecuteAndWait interceptor fails.
Since our original code was quite complex I wrote a simpler one below which demonstrates the
exact same behavior.
> The simple action is shown here:
> {noformat}
> package com.sentrylink.web.actions;
> import java.util.concurrent.TimeUnit;
> import org.apache.struts2.convention.annotation.InterceptorRef;
> import org.apache.struts2.convention.annotation.InterceptorRefs;
> import org.apache.struts2.convention.annotation.Result;
> import org.apache.struts2.convention.annotation.Results;
> import com.opensymphony.xwork2.ActionSupport;
> @SuppressWarnings("serial")
> @Results({
>     @Result(name="wait", location="/"),
>     @Result(name=ActionSupport.SUCCESS, location="/WEB-INF/content/messagePage.jsp"),
> })
> @InterceptorRefs({
>     @InterceptorRef("webStack"),
>     @InterceptorRef("execAndWait")
> })
> public class TestExecuteAndWait extends ActionSupport {
>     public String execute() throws Exception {
>         TimeUnit.SECONDS.sleep(10);
>         return SUCCESS;
>     }
> }
> {noformat}
> Running this gives
> {noformat}
> WARNING: Cannot serialize session attribute __execWaittest-execute-and-wait for session
74CDB9F8D00BBC697030AFC6978E94F6 
> java.io.NotSerializableException: com.opensymphony.xwork2.inject.ContainerImpl$ConstructorInjector
> {noformat}
> Removing the ExecuteAndWait interceptor fixes the issue.
> According to [~yasser.zamani] in WW-4873 : I reviewed {{ExecuteAndWaitInterceptor}} and
seems has this bug when session goes to being serialized in middle of an background process.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message