You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/12/03 15:21:13 UTC

svn commit: r1416537 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/valves/ErrorReportValve.java test/org/apache/catalina/startup/TomcatBaseTest.java test/org/apache/catalina/valves/TestErrorReportValve.java webapps/docs/changelog.xml

Author: markt
Date: Mon Dec  3 14:21:12 2012
New Revision: 1416537

URL: http://svn.apache.org/viewvc?rev=1416537&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54220
The ErrorReportValve should only generate an error response if the error flag on the response is true.

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
    tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
    tomcat/tc7.0.x/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1416534-1416535

Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1416537&r1=1416536&r2=1416537&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java Mon Dec  3 14:21:12 2012
@@ -160,7 +160,8 @@ public class ErrorReportValve extends Va
 
         // Do nothing on a 1xx, 2xx and 3xx status
         // Do nothing if anything has been written already
-        if ((statusCode < 400) || (response.getContentWritten() > 0)) {
+        if (statusCode < 400 || response.getContentWritten() > 0 ||
+                !response.isError()) {
             return;
         }
 

Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java?rev=1416537&r1=1416536&r2=1416537&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java Mon Dec  3 14:21:12 2012
@@ -253,20 +253,22 @@ public abstract class TomcatBaseTest ext
         } else {
             is = connection.getErrorStream();
         }
-        BufferedInputStream bis = null;
-        try {
-            bis = new BufferedInputStream(is);
-            byte[] buf = new byte[2048];
-            int rd = 0;
-            while((rd = bis.read(buf)) > 0) {
-                out.append(buf, 0, rd);
-            }
-        } finally {
-            if (bis != null) {
-                try {
-                    bis.close();
-                } catch (IOException e) {
-                    // Ignore
+        if (is != null) {
+            BufferedInputStream bis = null;
+            try {
+                bis = new BufferedInputStream(is);
+                byte[] buf = new byte[2048];
+                int rd = 0;
+                while((rd = bis.read(buf)) > 0) {
+                    out.append(buf, 0, rd);
+                }
+            } finally {
+                if (bis != null) {
+                    try {
+                        bis.close();
+                    } catch (IOException e) {
+                        // Ignore
+                    }
                 }
             }
         }

Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java?rev=1416537&r1=1416536&r2=1416537&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java Mon Dec  3 14:21:12 2012
@@ -66,4 +66,67 @@ public class TestErrorReportValve extend
             resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
         }
     }
+
+
+    @Test
+    public void testBug54220DoNotSetNotFound() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // Must have a real docBase - just use temp
+        Context ctx =
+            tomcat.addContext("", System.getProperty("java.io.tmpdir"));
+
+        Tomcat.addServlet(ctx, "bug54220", new Bug54220Servlet(false));
+        ctx.addServletMapping("/", "bug54220");
+
+        tomcat.start();
+
+        ByteChunk res = new ByteChunk();
+        int rc = getUrl("http://localhost:" + getPort(), res, null);
+
+        Assert.assertNull(res.toString());
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+    }
+
+
+    @Test
+    public void testBug54220SetNotFound() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // Must have a real docBase - just use temp
+        Context ctx =
+            tomcat.addContext("", System.getProperty("java.io.tmpdir"));
+
+        Tomcat.addServlet(ctx, "bug54220", new Bug54220Servlet(true));
+        ctx.addServletMapping("/", "bug54220");
+
+        tomcat.start();
+
+        ByteChunk res = new ByteChunk();
+        int rc = getUrl("http://localhost:" + getPort(), res, null);
+
+        Assert.assertNull(res.toString());
+        Assert.assertEquals(HttpServletResponse.SC_NOT_FOUND, rc);
+    }
+
+
+    private static final class Bug54220Servlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        private boolean setNotFound;
+
+        private Bug54220Servlet(boolean setNotFound) {
+            this.setNotFound = setNotFound;
+        }
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+
+            if (setNotFound) {
+                resp.setStatus(HttpServletResponse.SC_NOT_FOUND);
+            }
+        }
+    }
 }

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1416537&r1=1416536&r2=1416537&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Dec  3 14:21:12 2012
@@ -78,6 +78,10 @@
         session timeout is correctly tested. Also refactor unit test to make it
         easier to add additional tests. Patch by Brian Burch. (markt)
       </fix>
+      <fix>
+        <bug>54220</bug>: Ensure the ErrorReportValve only generates an error
+        report if the error flag on the response has been set. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Web applications">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org