struts-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lukaszlen...@apache.org
Subject [struts] branch master updated: Merge pull request #367 from JCgH4164838Gh792C124B5/local_25x_SendRedirectEnh
Date Tue, 01 Oct 2019 06:37:34 GMT
This is an automated email from the ASF dual-hosted git repository.

lukaszlenart pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/struts.git


The following commit(s) were added to refs/heads/master by this push:
     new 0987143  Merge pull request #367 from JCgH4164838Gh792C124B5/local_25x_SendRedirectEnh
     new 070f16c  Merge pull request #370 from JCgH4164838Gh792C124B5/local_26x_CPickPR367
0987143 is described below

commit 0987143e5d0fec1f3e390ebb1f20ebdef575346d
Author: Lukasz Lenart <lukaszlenart@apache.org>
AuthorDate: Mon Sep 23 08:54:14 2019 +0200

    Merge pull request #367 from JCgH4164838Gh792C124B5/local_25x_SendRedirectEnh
    
    Minor improvement proposed for ServletRedirectResult sendRedirect()
    
    (cherry picked from commit 706bb560e45df0d31e0f01895fb209d2685e2bcd)
---
 .../struts2/result/ServletRedirectResult.java      |  25 +++--
 .../struts2/result/ServletRedirectResultTest.java  | 119 +++++++++++++++++++++
 2 files changed, 137 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/org/apache/struts2/result/ServletRedirectResult.java b/core/src/main/java/org/apache/struts2/result/ServletRedirectResult.java
index 0879ab0..defb5b5 100644
--- a/core/src/main/java/org/apache/struts2/result/ServletRedirectResult.java
+++ b/core/src/main/java/org/apache/struts2/result/ServletRedirectResult.java
@@ -250,13 +250,24 @@ public class ServletRedirectResult extends StrutsResultSupport implements
Reflec
      * @throws IOException in case of IO errors
      */
     protected void sendRedirect(HttpServletResponse response, String finalLocation) throws
IOException {
-        if (SC_FOUND == statusCode) {
-            response.sendRedirect(finalLocation);
-        } else {
-            response.setStatus(statusCode);
-            response.setHeader("Location", finalLocation);
-            response.getWriter().write(finalLocation);
-            response.getWriter().close();
+        try {
+            if (SC_FOUND == statusCode) {
+                response.sendRedirect(finalLocation);
+            } else {
+                response.setStatus(statusCode);
+                response.setHeader("Location", finalLocation);
+                try {
+                    response.getWriter().write(finalLocation);
+                } finally {
+                    response.getWriter().close();
+                }
+            }
+        } catch (IOException ioe) {
+            LOG.warn("Unable to redirect to: {}, code: {}; {}", finalLocation, statusCode,
ioe);
+            throw ioe;  // Re-throw required to preserve existing default behaviour (no stacktrace
in above warn for this reason)
+        } catch (IllegalStateException ise) {
+            LOG.warn("Unable to redirect to: {}, code: {}; isCommited: {}; {}", finalLocation,
statusCode, response.isCommitted(), ise);
+            throw ise;  // Re-throw required to preserve existing default behaviour (no stacktrace
in above warn for this reason)
         }
 
     }
diff --git a/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java b/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java
index caec815..1bcee85 100644
--- a/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java
+++ b/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java
@@ -21,9 +21,12 @@ package org.apache.struts2.result;
 import static javax.servlet.http.HttpServletResponse.SC_SEE_OTHER;
 import static org.easymock.EasyMock.createControl;
 import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.createMock;
 import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
 import static org.easymock.EasyMock.replay;
 
+import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.ArrayList;
@@ -322,6 +325,122 @@ public class ServletRedirectResultTest extends StrutsInternalTestCase
implements
         control.verify();
     }
 
+    /**
+     * Test to exercise the code path and prove sendRedirect() will output 
+     * the desired log warning when an IOException is thrown for statusCode SC_FOUND.
+     */
+    public void testSendRedirectSCFoundIOException() {
+        HttpServletResponse httpServletResponseMock = (HttpServletResponse) createMock(HttpServletResponse.class);
+        boolean ioeCaught = false;
+        view.setLocation("/bar/foo.jsp");
+        view.setStatusCode(HttpServletResponse.SC_FOUND);
+        try {
+            httpServletResponseMock.sendRedirect(view.getLocation());
+            expectLastCall().andStubThrow(new IOException("Fake IO Exception (SC_FOUND)"));
+            replay(httpServletResponseMock);
+        } catch (IOException ioe) {
+            fail("Mock sendRedirect call setup failed.  Ex: " + ioe);
+        }
+        try {
+            view.sendRedirect(httpServletResponseMock, view.getLocation());
+        } catch (IOException ioe) {
+            ioeCaught = true;  // Verify expected exception was thrown
+        }
+        if (!ioeCaught) {
+            fail("sendRedirect (SC_FOUND) with forced IOException did not propagate from
setLocation!");
+        }
+    }
+
+    /**
+     * Test to exercise the code path and prove sendRedirect() will output 
+     * the desired log warning when an IOException is thrown for statusCode SC_MOVED_PERMANENTLY.
+     */
+    public void testSendRedirectSCMovedPermanentlyIOException() {
+        HttpServletResponse httpServletResponseMock = (HttpServletResponse) createMock(HttpServletResponse.class);
+        boolean ioeCaught = false;
+        view.setLocation("/bar/foo.jsp");
+        view.setStatusCode(HttpServletResponse.SC_MOVED_PERMANENTLY);  // Any non SC_FOUND
will suffice
+        try {
+            httpServletResponseMock.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
+            expectLastCall();
+            httpServletResponseMock.setHeader("Location", view.getLocation());
+            expectLastCall();
+            expect(httpServletResponseMock.getWriter()).andStubThrow(new IOException("Fake
IO Exception (SC_MOVED_PERMANENTLY)"));
+            replay(httpServletResponseMock);
+        } catch (IOException ioe) {
+            fail("Mock getWriter call setup failed.  Ex: " + ioe);
+        }
+        try {
+            view.sendRedirect(httpServletResponseMock, view.getLocation());
+        } catch (IOException ioe) {
+            ioeCaught = true;  // Verify expected exception was thrown
+        }
+        if (!ioeCaught) {
+            fail("sendRedirect (SC_MOVED_PERMANENTLY) with forced IOException did not propagate
from setLocation!");
+        }
+    }
+
+    /**
+     * Test to exercise the code path and prove sendRedirect() will output 
+     * the desired log warning when an IllegalStateException is thrown for statusCode SC_FOUND.
+     */
+    public void testSendRedirectSCFoundIllegalStateException() {
+        HttpServletResponse httpServletResponseMock = (HttpServletResponse) createMock(HttpServletResponse.class);
+        boolean iseCaught = false;
+        view.setLocation("/bar/foo.jsp");
+        view.setStatusCode(HttpServletResponse.SC_FOUND);
+        try {
+            httpServletResponseMock.sendRedirect(view.getLocation());
+            expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception
(SC_FOUND)"));
+            expect(httpServletResponseMock.isCommitted()).andStubReturn(Boolean.TRUE);
+            replay(httpServletResponseMock);
+        } catch (IOException ioe) {
+            fail("Mock sendRedirect call setup failed.  Ex: " + ioe);
+        }
+        try {
+            view.sendRedirect(httpServletResponseMock, view.getLocation());
+        } catch (IOException ioe) {
+            iseCaught = false;
+        } catch (IllegalStateException ise) {
+            iseCaught = true;  // Verify expected exception was thrown
+        }
+        if (!iseCaught) {
+            fail("sendRedirect (SC_FOUND) with forced IllegalStateException did not propagate
from setLocation!");
+        }
+    }
+
+    /**
+     * Test to exercise the code path and prove sendRedirect() will output 
+     * the desired log warning when an IllegalStateException is thrown for statusCode SC_MOVED_PERMANENTLY.
+     */
+    public void testSendRedirectSCMovedPermanentlyIllegalStateException() {
+        HttpServletResponse httpServletResponseMock = (HttpServletResponse) createMock(HttpServletResponse.class);
+        boolean iseCaught = false;
+        view.setLocation("/bar/foo.jsp");
+        view.setStatusCode(HttpServletResponse.SC_MOVED_PERMANENTLY);  // Any non SC_FOUND
will suffice
+        try {
+            httpServletResponseMock.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
+            expectLastCall();
+            httpServletResponseMock.setHeader("Location", view.getLocation());
+            expectLastCall();
+            expect(httpServletResponseMock.getWriter()).andStubThrow(new IllegalStateException("Fake
IllegalState Exception (SC_MOVED_PERMANENTLY)"));
+            expect(httpServletResponseMock.isCommitted()).andStubReturn(Boolean.TRUE);
+            replay(httpServletResponseMock);
+        } catch (IOException ioe) {
+            fail("Mock getWriter call setup failed.  Ex: " + ioe);
+        }
+        try {
+            view.sendRedirect(httpServletResponseMock, view.getLocation());
+        } catch (IOException ioe) {
+            iseCaught = false;
+        } catch (IllegalStateException ise) {
+            iseCaught = true;  // Verify expected exception was thrown
+        }
+        if (!iseCaught) {
+            fail("sendRedirect (SC_MOVED_PERMANENTLY) with forced IllegalStateException did
not propagate from setLocation!");
+        }
+    }
+
     protected void setUp() throws Exception {
         super.setUp();
         configurationManager.getConfiguration().


Mime
View raw message