WW-4729 Fixes issue with passing params in location

RYOMAE · Dec 30, 2016 · 3144b6c · 3144b6c
1 parent 3ce2140
commit 3144b6c
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 6 deletions.
diff --git a/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java b/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java
@@ -29,6 +29,7 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.struts2.ServletActionContext;
 import org.apache.struts2.StrutsStatics;
+import org.apache.struts2.dispatcher.HttpParameters;
 import org.apache.struts2.views.util.UrlHelper;
 
 import javax.servlet.RequestDispatcher;
@@ -142,14 +143,17 @@ public void doExecute(String finalLocation, ActionInvocation invocation) throws
             // see WW-2120
             if (StringUtils.isNotEmpty(finalLocation) && finalLocation.indexOf("?") > 0) {
                 String queryString = finalLocation.substring(finalLocation.indexOf("?") + 1);
-                Map<String, Object> parameters = getParameters(invocation);
+                HttpParameters parameters = getParameters(invocation);
                 Map<String, Object> queryParams = urlHelper.parseQueryString(queryString, true);
-                if (queryParams != null && !queryParams.isEmpty())
-                    parameters.putAll(queryParams);
+                if (queryParams != null && !queryParams.isEmpty()) {
+                    parameters = HttpParameters.create(queryParams).withParent(parameters).build();
+                    invocation.getInvocationContext().setParameters(parameters);
+                }
             }
 
             // if the view doesn't exist, let's do a 404
             if (dispatcher == null) {
+                LOG.warn("Location {} not found!", finalLocation);
                 response.sendError(404, "result '" + finalLocation + "' not found");
                 return;
             }
@@ -171,9 +175,8 @@ public void doExecute(String finalLocation, ActionInvocation invocation) throws
         }
     }
 
-    @SuppressWarnings("unchecked")
-    private Map<String, Object> getParameters(ActionInvocation invocation) {
-        return (Map<String, Object>) invocation.getInvocationContext().getContextMap().get("parameters");
+    protected HttpParameters getParameters(ActionInvocation invocation) {
+        return invocation.getInvocationContext().getParameters();
     }
 
 }
diff --git a/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java b/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java
@@ -25,6 +25,9 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import com.opensymphony.xwork2.mock.MockActionInvocation;
+import com.opensymphony.xwork2.util.ValueStack;
+import com.opensymphony.xwork2.util.ValueStackFactory;
 import ognl.Ognl;
 
 import org.apache.struts2.ServletActionContext;
@@ -107,4 +110,46 @@ public void testSimple() {
         requestMock.verify();
         dispatcherMock.verify();
     }
+
+    public void testWithParameter() {
+        ServletDispatcherResult view = container.inject(ServletDispatcherResult.class);
+        view.setLocation("foo.jsp?bar=1");
+
+        Mock dispatcherMock = new Mock(RequestDispatcher.class);
+        dispatcherMock.expect("forward", C.ANY_ARGS);
+
+        Mock requestMock = new Mock(HttpServletRequest.class);
+        requestMock.expectAndReturn("getAttribute", "struts.actiontag.invocation", null);
+        requestMock.expectAndReturn("getAttribute", "javax.servlet.include.servlet_path", null);
+        requestMock.expectAndReturn("getRequestDispatcher", C.args(C.eq("foo.jsp?bar=1")), dispatcherMock.proxy());
+        requestMock.expect("setAttribute", C.ANY_ARGS); // this is a bad mock, but it works
+        requestMock.expect("setAttribute", C.ANY_ARGS); // this is a bad mock, but it works
+        requestMock.matchAndReturn("getRequestURI", "foo.jsp");
+
+        Mock responseMock = new Mock(HttpServletResponse.class);
+        responseMock.expectAndReturn("isCommitted", Boolean.FALSE);
+
+        ActionContext ac = new ActionContext(Ognl.createDefaultContext(null));
+        ac.setContainer(container);
+        ActionContext.setContext(ac);
+        ServletActionContext.setRequest((HttpServletRequest) requestMock.proxy());
+        ServletActionContext.setResponse((HttpServletResponse) responseMock.proxy());
+
+        MockActionInvocation mockActionInvocation = new MockActionInvocation();
+        mockActionInvocation.setInvocationContext(ac);
+        mockActionInvocation.setStack(container.getInstance(ValueStackFactory.class).createValueStack());
+
+        try {
+            view.execute(mockActionInvocation);
+        } catch (Exception e) {
+            e.printStackTrace();
+            fail();
+        }
+
+        assertTrue(mockActionInvocation.getInvocationContext().getParameters().contains("bar"));
+        assertEquals("1", mockActionInvocation.getInvocationContext().getParameters().get("bar").getValue());
+        dispatcherMock.verify();
+        requestMock.verify();
+        dispatcherMock.verify();
+    }
 }