Hi Mark,
2016-06-07 15:54 GMT+03:00 <ma...@apache.org>:
>
> Author: markt
> Date: Tue Jun 7 12:54:23 2016
> New Revision: 1747210
>
> URL: http://svn.apache.org/viewvc?rev=1747210&view=rev
> Log:
> Follow-up to r1746649
> r1746649 triggered the call to Processor.endRequest() in the correct
location but failed to remove the call that was in the wrong location. This
meant it could be called twice leading to request corruption.
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
> tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
> tomcat/trunk/java/org/apache/coyote/ActionCode.java
> tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
> tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java
> tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
> tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
> tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
> URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1747210&r1=1747209&r2=1747210&view=diff
>
==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
(original)
> +++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Tue
Jun 7 12:54:23 2016
> @@ -96,29 +96,6 @@ public class AsyncContextImpl implements
>
> @Override
> public void fireOnComplete() {
> - // Fire the listeners
> - doFireOnComplete();
> -
> - // The application doesn't know it has to stop read and/or
writing until
> - // it receives the complete event so the request and response
have to be
> - // closed after firing the event.
> - try {
> - // First of all ensure that any data written to the response
is
> - // written to the I/O layer.
> - request.getResponse().finishResponse();
> - // Close the request and the response.
> - request.getCoyoteRequest().action(ActionCode.END_REQUEST,
null);
> - } catch (Throwable t) {
> - ExceptionUtils.handleThrowable(t);
> - // Catch this here and allow async context complete to
continue
> - // normally so a dispatch takes place which ensures that the
> - // request and response objects are correctly recycled.
> -
log.debug(sm.getString("asyncContextImpl.finishResponseError"), t);
> - }
> - }
> -
> -
> - private void doFireOnComplete() {
> List<AsyncListenerWrapper> listenersCopy = new ArrayList<>();
> listenersCopy.addAll(listeners);
>
> @@ -367,9 +344,7 @@ public class AsyncContextImpl implements
> dispatch = null;
> runnable.run();
> if (!request.isAsync()) {
> - // Uses internal method since we don't want the
request/response
> - // to be closed. That will be handled in the adapter.
> - doFireOnComplete();
> + fireOnComplete();
> }
> } catch (RuntimeException x) {
> // doInternalComplete(true);
>
> Modified:
tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
> URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1747210&r1=1747209&r2=1747210&view=diff
>
==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
(original)
> +++ tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
Tue Jun 7 12:54:23 2016
> @@ -78,7 +78,6 @@ aprListener.tooLateForSSLRandomSeed=Cann
> aprListener.tooLateForFIPSMode=Cannot setFIPSMode: SSL has already been
initialized
> aprListener.initializedOpenSSL=OpenSSL successfully initialized ({0})
>
> -asyncContextImpl.finishResponseError=Response did not finish cleanly
after AsyncContext completed
> asyncContextImpl.request.ise=It is illegal to call getRequest() after
complete() or any of the dispatch() methods has been called
> asyncContextImpl.requestEnded=The request associated with the
AsyncContext has already completed processing.
> asyncContextImpl.response.ise=It is illegal to call getResponse() after
complete() or any of the dispatch() methods has been called
>
> Modified: tomcat/trunk/java/org/apache/coyote/ActionCode.java
> URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ActionCode.java?rev=1747210&r1=1747209&r2=1747210&view=diff
>
==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/ActionCode.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/ActionCode.java Tue Jun 7
12:54:23 2016
> @@ -239,12 +239,6 @@ public enum ActionCode {
> DISPATCH_EXECUTE,
>
> /**
> - * Trigger end of request processing (remaining input swallowed,
write any
> - * remaining parts of the response etc.).
> - */
> - END_REQUEST,
> -
> - /**
> * Is server push supported and allowed for the current request?
> */
> IS_PUSH_SUPPORTED,
>
> Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
> URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1747210&r1=1747209&r2=1747210&view=diff
>
==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Tue Jun 7
12:54:23 2016
> @@ -432,10 +432,6 @@ public class AjpProcessor extends Abstra
> setErrorState(ErrorState.CLOSE_CLEAN, null);
> break;
> }
> - case END_REQUEST: {
> - // NO-OP for AJP
> - break;
> - }
>
> // Request attribute support
> case REQ_HOST_ADDR_ATTRIBUTE: {
>
> Modified:
tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java
> URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java?rev=1747210&r1=1747209&r2=1747210&view=diff
>
==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java
(original)
> +++ tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java Tue
Jun 7 12:54:23 2016
> @@ -321,6 +321,9 @@ public class Http11InputBuffer implement
>
> lastValid = 0;
> pos = 0;
> +
> + System.out.println("Http11InputBuffer.recycle(): pos [" + pos +
"], lastValid [" + lastValid + "]");
> +
Most probably you do not want these System out statements.
Regards,
Violeta
> lastActiveFilter = -1;
> parsingHeader = true;
> swallowInput = true;
> @@ -352,6 +355,8 @@ public class Http11InputBuffer implement
> lastValid = lastValid - pos;
> pos = 0;
>
> + System.out.println("Http11InputBuffer.nextRequest(): pos [" +
pos + "], lastValid [" + lastValid + "]");
> +
> // Recycle filters
> for (int i = 0; i <= lastActiveFilter; i++) {
> activeFilters[i].recycle();
> @@ -412,7 +417,7 @@ public class Http11InputBuffer implement
> }
> if (!keptAlive && pos == 0 && lastValid >=
CLIENT_PREFACE_START.length - 1) {
> boolean prefaceMatch = true;
> - for (int i = 0; i < CLIENT_PREFACE_START.length;
i++) {
> + for (int i = 0; i < CLIENT_PREFACE_START.length &&
prefaceMatch; i++) {
> if (CLIENT_PREFACE_START[i] != buf[i]) {
> prefaceMatch = false;
> }
> @@ -631,6 +636,8 @@ public class Http11InputBuffer implement
> if (swallowInput && (lastActiveFilter != -1)) {
> int extraBytes = (int) activeFilters[lastActiveFilter].end();
> pos = pos - extraBytes;
> + System.out.println("Http11InputBuffer.endRequest(): pos [" +
pos + "], lastValid [" + lastValid + "]");
> + (new Exception()).printStackTrace();
> }
> }
>
> @@ -742,6 +749,7 @@ public class Http11InputBuffer implement
> int nRead = wrapper.read(block, buf, pos, buf.length - pos);
> if (nRead > 0) {
> lastValid = pos + nRead;
> + System.out.println("Http11InputBuffer.fill(): pos [" + pos +
"], lastValid [" + lastValid + "]");
> return true;
> } else if (nRead == -1) {
> throw new EOFException(sm.getString("iib.eof.error"));
> @@ -1077,6 +1085,7 @@ public class Http11InputBuffer implement
> int length = lastValid - pos;
> chunk.setBytes(buf, pos, length);
> pos = lastValid;
> + System.out.println("SocketInputBuffer.doRead(): pos [" + pos
+ "], lastValid [" + lastValid + "]");
>
> return length;
> }
>
> Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
> URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1747210&r1=1747209&r2=1747210&view=diff
>
==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
(original)
> +++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Tue
Jun 7 12:54:23 2016
> @@ -733,10 +733,6 @@ public class Http11Processor extends Abs
> inputBuffer.setSwallowInput(false);
> break;
> }
> - case END_REQUEST: {
> - endRequest();
> - break;
> - }
>
> // Request attribute support
> case REQ_HOST_ADDR_ATTRIBUTE: {
>
> Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
> URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1747210&r1=1747209&r2=1747210&view=diff
>
==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
(original)
> +++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Tue
Jun 7 12:54:23 2016
> @@ -169,12 +169,6 @@ public class StreamProcessor extends Abs
> // control windows are correctly tracked.
> break;
> }
> - case END_REQUEST: {
> - // NO-OP
> - // This action is geared towards handling HTTP/1.1
expectations and
> - // keep-alive. Does not apply to HTTP/2 streams.
> - break;
> - }
>
> // Request attribute support
> case REQ_HOST_ADDR_ATTRIBUTE: {
>
> Modified:
tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
> URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java?rev=1747210&r1=1747209&r2=1747210&view=diff
>
==============================================================================
> --- tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
(original)
> +++ tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
Tue Jun 7 12:54:23 2016
> @@ -717,11 +717,21 @@ public class TestHttp11Processor extends
> * async processing.
> */
> @Test
> - public void testBug57621() throws Exception {
> + public void testBug57621a() throws Exception {
> + doTestBug57621(true);
> + }
> +
> +
> + @Test
> + public void testBug57621b() throws Exception {
> + doTestBug57621(false);
> + }
>
> +
> + private void doTestBug57621(boolean delayAsyncThread) throws
Exception {
> Tomcat tomcat = getTomcatInstance();
> Context root = tomcat.addContext("", null);
> - Wrapper w = Tomcat.addServlet(root, "Bug57621", new
Bug57621Servlet());
> + Wrapper w = Tomcat.addServlet(root, "Bug57621", new
Bug57621Servlet(delayAsyncThread));
> w.setAsyncSupported(true);
> root.addServletMapping("/test", "Bug57621");
>
> @@ -752,6 +762,14 @@ public class TestHttp11Processor extends
>
> private static final long serialVersionUID = 1L;
>
> + private final boolean delayAsyncThread;
> +
> +
> + public Bug57621Servlet(boolean delayAsyncThread) {
> + this.delayAsyncThread = delayAsyncThread;
> + }
> +
> +
> @Override
> protected void doPut(HttpServletRequest req, HttpServletResponse
resp)
> throws ServletException, IOException {
> @@ -759,6 +777,15 @@ public class TestHttp11Processor extends
> ac.start(new Runnable() {
> @Override
> public void run() {
> + if (delayAsyncThread) {
> + // Makes the difference between calling complete
before
> + // the request body is received of after.
> + try {
> + Thread.sleep(1500);
> + } catch (InterruptedException e) {
> + e.printStackTrace();
> + }
> + }
> resp.setContentType("text/plain");
> resp.setCharacterEncoding("UTF-8");
> try {
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
