On 07/06/2016 14:12, Violeta Georgieva wrote:
> 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.
Thanks. I'll get rid of those.
Mark
>
> 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
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
