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 2013/11/15 14:39:53 UTC
svn commit: r1542267 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/coyote/ java/org/apache/coyote/ajp/
java/org/apache/coyote/http11/ java/org/apache/coyote/spdy/
test/org/apache/coyote/http11/
Author: markt
Date: Fri Nov 15 13:39:52 2013
New Revision: 1542267
URL: http://svn.apache.org/r1542267
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/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/trunk/java/org/apache/coyote/ActionCode.java
tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.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=1542267&r1=1542266&r2=1542267&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Fri Nov 15 13:39:52 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.ReadListener;
import javax.servlet.RequestDispatcher;
@@ -574,8 +575,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/trunk/java/org/apache/coyote/ActionCode.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ActionCode.java?rev=1542267&r1=1542266&r2=1542267&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ActionCode.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ActionCode.java Fri Nov 15 13:39:52 2013
@@ -41,6 +41,12 @@ public enum ActionCode {
RESET,
/**
+ * 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/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1542267&r1=1542266&r2=1542267&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java Fri Nov 15 13:39:52 2013
@@ -421,6 +421,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/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1542267&r1=1542266&r2=1542267&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Fri Nov 15 13:39:52 2013
@@ -768,6 +768,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/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java?rev=1542267&r1=1542266&r2=1542267&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java Fri Nov 15 13:39:52 2013
@@ -237,6 +237,9 @@ public class SpdyProcessor<S> extends Ab
// 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/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java?rev=1542267&r1=1542266&r2=1542267&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java Fri Nov 15 13:39:52 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 {
@@ -395,6 +404,115 @@ 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";
+
+ try (final Socket 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();
+ }
+
+ bug55772Latch2.await();
+ bug55772IsSecondRequest = true;
+
+ try (final Socket 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();
+ }
+
+ 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;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org