Author: markt
Date: Fri Nov 15 13:46:25 2013
New Revision: 1542269
URL: http://svn.apache.org/r1542269
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55772
Ensure that the request and response are recycled after an error during async
processing.
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/java/org/apache/coyote/ActionCode.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1542267
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=1542269&r1=1542268&r2=1542269&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 Nov 15 13:46:25 2013
@@ -20,6 +20,7 @@ import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.nio.charset.Charset;
import java.util.EnumSet;
+import java.util.concurrent.atomic.AtomicBoolean;
import javax.servlet.RequestDispatcher;
import javax.servlet.SessionTrackingMode;
@@ -451,8 +452,10 @@ public class CoyoteAdapter implements Ad
// Ignore
} finally {
req.getRequestProcessor().setWorkerThreadName(null);
+ AtomicBoolean error = new AtomicBoolean(false);
+ res.action(ActionCode.IS_ERROR, error);
// Recycle the wrapper request and response
- if (!comet && !async) {
+ if (!comet && !async || error.get()) {
request.recycle();
response.recycle();
} else {
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ActionCode.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ActionCode.java?rev=1542269&r1=1542268&r2=1542269&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ActionCode.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ActionCode.java Fri Nov 15
13:46:25 2013
@@ -48,6 +48,12 @@ public enum ActionCode {
POST_REQUEST,
/**
+ * Has the processor been placed into the error state? Note that the
+ * response may not have an appropriate error code set.
+ */
+ IS_ERROR,
+
+ /**
* Hook called if swallowing request input should be disabled.
* Example: Cancel a large file upload.
*
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1542269&r1=1542268&r2=1542269&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
Fri Nov 15 13:46:25 2013
@@ -359,6 +359,9 @@ public abstract class AbstractAjpProcess
error = true;
}
+ } else if (actionCode == ActionCode.IS_ERROR) {
+ ((AtomicBoolean) param).set(error);
+
} else if (actionCode == ActionCode.DISABLE_SWALLOW_INPUT) {
// TODO: Do not swallow request input but
// make sure we are closing the connection
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1542269&r1=1542268&r2=1542269&view=diff
==============================================================================
---
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
(original)
+++
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
Fri Nov 15 13:46:25 2013
@@ -809,6 +809,9 @@ public abstract class AbstractHttp11Proc
response.setErrorException(e);
}
+ } else if (actionCode == ActionCode.IS_ERROR) {
+ ((AtomicBoolean) param).set(error);
+
} else if (actionCode == ActionCode.DISABLE_SWALLOW_INPUT) {
// Do not swallow request input but
// make sure we are closing the connection
Modified:
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java?rev=1542269&r1=1542268&r2=1542269&view=diff
==============================================================================
---
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
(original)
+++
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
Fri Nov 15 13:46:25 2013
@@ -18,12 +18,19 @@ package org.apache.coyote.http11;
import java.io.File;
import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.io.Writer;
+import java.net.Socket;
import java.nio.CharBuffer;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import javax.servlet.AsyncContext;
import javax.servlet.ServletException;
+import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -32,6 +39,7 @@ import static org.junit.Assert.assertEqu
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import org.junit.Assert;
import org.junit.Test;
import org.apache.catalina.Context;
@@ -39,6 +47,7 @@ import org.apache.catalina.startup.Simpl
import org.apache.catalina.startup.TesterServlet;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.B2CConverter;
import org.apache.tomcat.util.buf.ByteChunk;
public class TestAbstractHttp11Processor extends TomcatBaseTest {
@@ -397,6 +406,134 @@ public class TestAbstractHttp11Processor
}
}
+
+ private static CountDownLatch bug55772Latch1 = new CountDownLatch(1);
+ private static CountDownLatch bug55772Latch2 = new CountDownLatch(1);
+ private static CountDownLatch bug55772Latch3 = new CountDownLatch(1);
+ private static boolean bug55772IsSecondRequest = false;
+ private static boolean bug55772RequestStateLeaked = false;
+
+
+ @Test
+ public void testBug55772() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ tomcat.getConnector().setProperty("processorCache", "1");
+ tomcat.getConnector().setProperty("maxThreads", "1");
+
+ // Must have a real docBase - just use temp
+ Context ctxt = tomcat.addContext("",
+ System.getProperty("java.io.tmpdir"));
+
+ Tomcat.addServlet(ctxt, "async", new Bug55772Servlet());
+ ctxt.addServletMapping("/*", "async");
+
+ tomcat.start();
+
+ String request1 = "GET /async HTTP/1.1\r\n" +
+ "Host: localhost:" + getPort() + "\r\n" +
+ "Connection: keep-alive\r\n" +
+ "Cache-Control: max-age=0\r\n" +
+ "Accept:
text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8\r\n"
+
+ "User-Agent: Request1\r\n" +
+ "Accept-Encoding: gzip,deflate,sdch\r\n" +
+ "Accept-Language: en-US,en;q=0.8,fr;q=0.6,es;q=0.4\r\n" +
+ "Cookie: something.that.should.not.leak=true\r\n" +
+ "\r\n";
+
+ String request2 = "GET /async HTTP/1.1\r\n" +
+ "Host: localhost:" + getPort() + "\r\n" +
+ "Connection: keep-alive\r\n" +
+ "Cache-Control: max-age=0\r\n" +
+ "Accept:
text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8\r\n"
+
+ "User-Agent: Request2\r\n" +
+ "Accept-Encoding: gzip,deflate,sdch\r\n" +
+ "Accept-Language: en-US,en;q=0.8,fr;q=0.6,es;q=0.4\r\n" +
+ "\r\n";
+
+ Socket connection = null;
+ try {
+ connection = new Socket("localhost", getPort());
+ connection.setSoLinger(true, 0);
+ Writer writer = new
OutputStreamWriter(connection.getOutputStream(),
+ B2CConverter.getCharset("US-ASCII"));
+ writer.write(request1);
+ writer.flush();
+
+ bug55772Latch1.await();
+ connection.close();
+ } finally {
+ if (connection != null) {
+ try {
+ connection.close();
+ } catch (IOException ioe) {
+ // Ignore
+ }
+ }
+ }
+
+ bug55772Latch2.await();
+ bug55772IsSecondRequest = true;
+
+ try {
+ connection = new Socket("localhost", getPort());
+ connection.setSoLinger(true, 0);
+ Writer writer = new
OutputStreamWriter(connection.getOutputStream(),
+ B2CConverter.getCharset("US-ASCII"));
+ writer.write(request2);
+ writer.flush();
+ connection.getInputStream().read();
+ } finally {
+ if (connection != null) {
+ try {
+ connection.close();
+ } catch (IOException ioe) {
+ // Ignore
+ }
+ }
+ }
+
+ bug55772Latch3.await();
+ if (bug55772RequestStateLeaked) {
+ Assert.fail("State leaked between requests!");
+ }
+ }
+
+
+ private static class Bug55772Servlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
+ if (bug55772IsSecondRequest) {
+ Cookie[] cookies = req.getCookies();
+ if (cookies != null && cookies.length > 0) {
+ for (Cookie cookie : req.getCookies()) {
+ if
(cookie.getName().equalsIgnoreCase("something.that.should.not.leak")) {
+ bug55772RequestStateLeaked = true;
+ }
+ }
+ }
+ bug55772Latch3.countDown();
+ } else {
+ req.getCookies(); // We have to do this so Tomcat will
actually parse the cookies from the request
+ }
+
+ req.setAttribute("org.apache.catalina.ASYNC_SUPPORTED",
Boolean.TRUE);
+ AsyncContext asyncContext = req.startAsync();
+ asyncContext.setTimeout(5000);
+
+ bug55772Latch1.countDown();
+
+ PrintWriter writer = asyncContext.getResponse().getWriter();
+ writer.print('\n');
+ writer.flush();
+
+ bug55772Latch2.countDown();
+ }
+ }
+
+
private static final class LargeHeaderServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
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=1542269&r1=1542268&r2=1542269&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Fri Nov 15 13:46:25 2013
@@ -124,6 +124,11 @@
unnecessary, it causes problems with using SPNEGO with IBM JDKs. Patch
provided by Arunav Sanyal. (markt)
</fix>
+ <fix>
+ <bug>55772</bug>: Ensure that the request and response are recycled
+ after an error during asynchronous processing. Includes a test case
+ based on code contributed by Todd West. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
