Author: markt
Date: Fri Oct 14 20:59:19 2011
New Revision: 1183499
URL: http://svn.apache.org/viewvc?rev=1183499&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52009
Port fix (r1183494) from trunk
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/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Oct 14 20:59:19 2011
@@ -1 +1 @@
-/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096,1173241,1173256
,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175690,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245,1177850,1177862,1177978,1178209,1178228,1178233,1178449,1178542,1178681,1178721,1180261,1180907,1181028,1181123,1181125,1181136,1181291,1181743,1183328,1183492-1183493
+/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096,1173241,1173256
,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175690,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245,1177850,1177862,1177978,1178209,1178228,1178233,1178449,1178542,1178681,1178721,1180261,1180907,1181028,1181123,1181125,1181136,1181291,1181743,1183328,1183492-1183494
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1183499&r1=1183498&r2=1183499&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
Fri Oct 14 20:59:19 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/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java?rev=1183499&r1=1183498&r2=1183499&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
(original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
Fri Oct 14 20:59:19 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();
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=1183499&r1=1183498&r2=1183499&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Fri Oct 14 20:59:19 2011
@@ -83,6 +83,10 @@
Simplify the deployment code and use full paths in log messages to
remove any ambiguity in where a context is being deployed from.
(markt)
</scode>
+ <fix>
+ <bug>52009</bug>: Fix a NPE during access log entry recording when an
+ error occurred during the processing of a Comet request. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
@@ -123,8 +127,9 @@
Update the package re-named copy of Commons BCEL (formerly Jakarta
BCEL)
to the latest code from Commons BCEL trunk. (markt)
</update>
- <scode>Remove some unused code from the packaged renamed Commons BCEL.
- (markt)</scode>
+ <scode>
+ Remove some unused code from the packaged renamed Commons BCEL. (markt)
+ </scode>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
