Author: markt
Date: Fri Oct 14 20:50:57 2011
New Revision: 1183494
URL: http://svn.apache.org/viewvc?rev=1183494&view=rev
Log:
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
Modified:
tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1183494&r1=1183493&r2=1183494&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Fri Oct
14 20:50:57 2011
@@ -249,6 +249,10 @@ public class CoyoteAdapter implements Ad
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 Ad
} 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(),
Modified: tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java?rev=1183494&r1=1183493&r2=1183494&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java Fri Oct
14 20:50:57 2011
@@ -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
@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
// 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
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
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
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
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();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
