From: markt Date: Fri, 14 Oct 2011 20:50:57 +0000 (+0000) Subject: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52009 X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;p=tomcat7.0 Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52009 Fix the NPE if an error occurs during comet processing Add test cases for errors during comet processing Ensure access log entries are made if an error occurs git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1183494 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java index 379935ade..e38fcdcb8 100644 --- a/java/org/apache/catalina/connector/CoyoteAdapter.java +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java @@ -249,6 +249,10 @@ public class CoyoteAdapter implements Adapter { req.getRequestProcessor().setWorkerThreadName(null); // Recycle the wrapper request and response if (error || response.isClosed() || !request.isComet()) { + ((Context) request.getMappingData().context).logAccess( + request, response, + System.currentTimeMillis() - req.getStartTime(), + false); request.recycle(); request.setFilterChain(null); response.recycle(); @@ -430,9 +434,12 @@ public class CoyoteAdapter implements Adapter { } else if (!comet) { request.finishRequest(); response.finishResponse(); - if (postParseSuccess) { + if (postParseSuccess && + request.getMappingData().context != null) { // Log only if processing was invoked. // If postParseRequest() failed, it has already logged it. + // If context is null this was the start of a comet request + // that failed and has already been logged. ((Context) request.getMappingData().context).logAccess( request, response, System.currentTimeMillis() - req.getStartTime(), diff --git a/test/org/apache/catalina/comet/TestCometProcessor.java b/test/org/apache/catalina/comet/TestCometProcessor.java index 67cb3439d..481bdd3bf 100644 --- a/test/org/apache/catalina/comet/TestCometProcessor.java +++ b/test/org/apache/catalina/comet/TestCometProcessor.java @@ -36,12 +36,14 @@ import static org.junit.Assert.fail; import org.junit.Test; import org.apache.catalina.Context; +import org.apache.catalina.Wrapper; import org.apache.catalina.comet.CometEvent.EventType; import org.apache.catalina.connector.CometEventImpl; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.catalina.valves.TesterAccessLogValve; import org.apache.catalina.valves.ValveBase; public class TestCometProcessor extends TomcatBaseTest { @@ -116,7 +118,25 @@ public class TestCometProcessor extends TomcatBaseTest { @Test public void testSimpleCometClient() throws Exception { + doSimpleCometTest(null); + } + + @Test + public void testSimpleCometClientBeginFail() throws Exception { + doSimpleCometTest(SimpleCometServlet.FAIL_ON_BEGIN); + } + @Test + public void testSimpleCometClientReadFail() throws Exception { + doSimpleCometTest(SimpleCometServlet.FAIL_ON_READ); + } + + @Test + public void testSimpleCometClientEndFail() throws Exception { + doSimpleCometTest(SimpleCometServlet.FAIL_ON_END); + } + + private void doSimpleCometTest(String initParam) throws Exception { if (!isCometSupported()) { return; } @@ -124,8 +144,15 @@ public class TestCometProcessor extends TomcatBaseTest { // Setup Tomcat instance Tomcat tomcat = getTomcatInstance(); Context root = tomcat.addContext("", TEMP_DIR); - Tomcat.addServlet(root, "comet", new SimpleCometServlet()); + Wrapper w = Tomcat.addServlet(root, "comet", new SimpleCometServlet()); + if (initParam != null) { + w.addInitParameter(initParam, "true"); + } root.addServletMapping("/", "comet"); + + TesterAccessLogValve alv = new TesterAccessLogValve(); + root.getPipeline().addValve(alv); + tomcat.start(); // Create connection to Comet servlet @@ -151,36 +178,51 @@ public class TestCometProcessor extends TomcatBaseTest { os.close(); is.close(); - // Validate response String[] response = readThread.getResponse().split("\r\n"); - assertEquals("HTTP/1.1 200 OK", response[0]); - assertEquals("Server: Apache-Coyote/1.1", response[1]); - assertTrue(response[2].startsWith("Set-Cookie: JSESSIONID=")); - assertEquals("Content-Type: text/plain;charset=ISO-8859-1", response[3]); - assertEquals("Transfer-Encoding: chunked", response[4]); - assertTrue(response[5].startsWith("Date: ")); - assertEquals("", response[6]); - assertEquals("7", response[7]); - assertEquals("BEGIN", response[8]); - assertEquals("", response[9]); - assertEquals("17", response[10]); - assertEquals("Client: READ: 4 bytes", response[11]); - assertEquals("", response[12]); - assertEquals("17", response[13]); - assertEquals("Client: READ: 4 bytes", response[14]); - assertEquals("", response[15]); - assertEquals("17", response[16]); - assertEquals("Client: READ: 4 bytes", response[17]); - assertEquals("", response[18]); - assertEquals("17", response[19]); - assertEquals("Client: READ: 4 bytes", response[20]); - assertEquals("", response[21]); - assertEquals("d", response[22]); - assertEquals("Client: END", response[23]); - assertEquals("", response[24]); - assertEquals("0", response[25]); - // Expect 26 lines - assertEquals(26, response.length); + if (initParam == null) { + // Normal response expected + // Validate response + assertEquals("HTTP/1.1 200 OK", response[0]); + assertEquals("Server: Apache-Coyote/1.1", response[1]); + assertTrue(response[2].startsWith("Set-Cookie: JSESSIONID=")); + assertEquals("Content-Type: text/plain;charset=ISO-8859-1", response[3]); + assertEquals("Transfer-Encoding: chunked", response[4]); + assertTrue(response[5].startsWith("Date: ")); + assertEquals("", response[6]); + assertEquals("7", response[7]); + assertEquals("BEGIN", response[8]); + assertEquals("", response[9]); + assertEquals("17", response[10]); + assertEquals("Client: READ: 4 bytes", response[11]); + assertEquals("", response[12]); + assertEquals("17", response[13]); + assertEquals("Client: READ: 4 bytes", response[14]); + assertEquals("", response[15]); + assertEquals("17", response[16]); + assertEquals("Client: READ: 4 bytes", response[17]); + assertEquals("", response[18]); + assertEquals("17", response[19]); + assertEquals("Client: READ: 4 bytes", response[20]); + assertEquals("", response[21]); + assertEquals("d", response[22]); + assertEquals("Client: END", response[23]); + assertEquals("", response[24]); + assertEquals("0", response[25]); + // Expect 26 lines + assertEquals(26, response.length); + } else { + // Failure expected only expected for the fail on begin + // Failure at any later stage and the reponse headers (including the + // 200 response code will already have been sent to the client + if (initParam == SimpleCometServlet.FAIL_ON_BEGIN) { + assertEquals("HTTP/1.1 500 Internal Server Error", response[0]); + alv.validateAccessLog(1, 500, 0, 1000); + } else { + assertEquals("HTTP/1.1 200 OK", response[0]); + alv.validateAccessLog(1, 200, 0, 5000); + } + + } } /** @@ -267,6 +309,26 @@ public class TestCometProcessor extends TomcatBaseTest { private static final long serialVersionUID = 1L; + public static final String FAIL_ON_BEGIN = "failOnBegin"; + public static final String FAIL_ON_READ = "failOnRead"; + public static final String FAIL_ON_END = "failOnEnd"; + + private boolean failOnBegin = false; + private boolean failOnRead = false; + private boolean failOnEnd = false; + + + @Override + public void init() throws ServletException { + failOnBegin = Boolean.valueOf(getServletConfig().getInitParameter( + FAIL_ON_BEGIN)).booleanValue(); + failOnRead = Boolean.valueOf(getServletConfig().getInitParameter( + FAIL_ON_READ)).booleanValue(); + failOnEnd = Boolean.valueOf(getServletConfig().getInitParameter( + FAIL_ON_END)).booleanValue(); + } + + @Override public void event(CometEvent event) throws IOException, ServletException { @@ -278,9 +340,15 @@ public class TestCometProcessor extends TomcatBaseTest { session.setMaxInactiveInterval(30); if (event.getEventType() == EventType.BEGIN) { + if (failOnBegin) { + throw new IOException("Fail on begin"); + } response.setContentType("text/plain"); response.getWriter().print("BEGIN" + "\r\n"); } else if (event.getEventType() == EventType.READ) { + if (failOnRead) { + throw new IOException("Fail on read"); + } InputStream is = request.getInputStream(); int count = 0; while (is.available() > 0) { @@ -290,6 +358,9 @@ public class TestCometProcessor extends TomcatBaseTest { String msg = "READ: " + count + " bytes"; response.getWriter().print("Client: " + msg + "\r\n"); } else if (event.getEventType() == EventType.END) { + if (failOnEnd) { + throw new IOException("Fail on end"); + } String msg = "END"; response.getWriter().print("Client: " + msg + "\r\n"); event.close();