Author: markt Date: Tue Jun 7 12:57:55 2016 New Revision: 1747213 URL: http://svn.apache.org/viewvc?rev=1747213&view=rev Log: Follow-up to r1746725 r1746725 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/tc8.5.x/trunk/ (props changed) tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/LocalStrings.properties tomcat/tc8.5.x/trunk/java/org/apache/coyote/ActionCode.java tomcat/tc8.5.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java tomcat/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java tomcat/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11Processor.java tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/StreamProcessor.java tomcat/tc8.5.x/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java Propchange: tomcat/tc8.5.x/trunk/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Tue Jun 7 12:57:55 2016 @@ -1 +1 @@ -/tomcat/trunk:1734785,1734799,1734845,1734928,1735041,1735044,1735480,1735577,1735597,1735599-1735600,1735615,1736145,1736162,1736209,1736280,1736297,1736299,1736489,1736646,1736703,1736836,1736849,1737104-1737105,1737112,1737117,1737119-1737120,1737155,1737157,1737192,1737280,1737339,1737632,1737664,1737715,1737748,1737785,1737834,1737860,1737959,1738005,1738007,1738014-1738015,1738018,1738022,1738039,1738043,1738059-1738060,1738147,1738149,1738174-1738175,1738261,1738589,1738623-1738625,1738643,1738816,1738850,1738855,1738946-1738948,1738953-1738954,1738979,1738982,1739079-1739081,1739087,1739113,1739153,1739172,1739176,1739191,1739474,1739726,1739762,1739775,1739814,1739817-1739818,1739975,1740131,1740324,1740465,1740495,1740508-1740509,1740520,1740535,1740707,1740803,1740810,1740969,1740980,1740991,1740997,1741015,1741033,1741036,1741058,1741060,1741080,1741147,1741159,1741164,1741173,1741181,1741190,1741197,1741202,1741208,1741213,1741221,1741225,1741232,1741409,1741501,1741677 ,1741892,1741896,1741984,1742023,1742042,1742071,1742090,1742093,1742101,1742105,1742111,1742139,1742146,1742148,1742166,1742181,1742184,1742187,1742246,1742248-1742251,1742263-1742264,1742268,1742276,1742369,1742387,1742448,1742509-1742512,1742917,1742919,1742933,1742975-1742976,1742984,1742986,1743019,1743115,1743117,1743124-1743125,1743134,1743425,1743554,1743679,1743696-1743698,1743700-1743701,1744058,1744064-1744065,1744125,1744194,1744229,1744270,1744323,1744432,1744684,1744697,1744705,1744713,1744760,1744786,1745142-1745143,1745145,1745177,1745179-1745180,1745227,1745248,1745254,1745337,1745467,1745576,1745735,1745744,1746304,1746306-1746307,1746319,1746327,1746338,1746340-1746341,1746344,1746427,1746441,1746473,1746490,1746492,1746495-1746496,1746499-1746501,1746503-1746507,1746509,1746549,1746551,1746554,1746556,1746558,1746584,1746620,1746649,1746724,1746939,1746989,1747014,1747028,1747035 +/tomcat/trunk:1734785,1734799,1734845,1734928,1735041,1735044,1735480,1735577,1735597,1735599-1735600,1735615,1736145,1736162,1736209,1736280,1736297,1736299,1736489,1736646,1736703,1736836,1736849,1737104-1737105,1737112,1737117,1737119-1737120,1737155,1737157,1737192,1737280,1737339,1737632,1737664,1737715,1737748,1737785,1737834,1737860,1737959,1738005,1738007,1738014-1738015,1738018,1738022,1738039,1738043,1738059-1738060,1738147,1738149,1738174-1738175,1738261,1738589,1738623-1738625,1738643,1738816,1738850,1738855,1738946-1738948,1738953-1738954,1738979,1738982,1739079-1739081,1739087,1739113,1739153,1739172,1739176,1739191,1739474,1739726,1739762,1739775,1739814,1739817-1739818,1739975,1740131,1740324,1740465,1740495,1740508-1740509,1740520,1740535,1740707,1740803,1740810,1740969,1740980,1740991,1740997,1741015,1741033,1741036,1741058,1741060,1741080,1741147,1741159,1741164,1741173,1741181,1741190,1741197,1741202,1741208,1741213,1741221,1741225,1741232,1741409,1741501,1741677 ,1741892,1741896,1741984,1742023,1742042,1742071,1742090,1742093,1742101,1742105,1742111,1742139,1742146,1742148,1742166,1742181,1742184,1742187,1742246,1742248-1742251,1742263-1742264,1742268,1742276,1742369,1742387,1742448,1742509-1742512,1742917,1742919,1742933,1742975-1742976,1742984,1742986,1743019,1743115,1743117,1743124-1743125,1743134,1743425,1743554,1743679,1743696-1743698,1743700-1743701,1744058,1744064-1744065,1744125,1744194,1744229,1744270,1744323,1744432,1744684,1744697,1744705,1744713,1744760,1744786,1745142-1745143,1745145,1745177,1745179-1745180,1745227,1745248,1745254,1745337,1745467,1745576,1745735,1745744,1746304,1746306-1746307,1746319,1746327,1746338,1746340-1746341,1746344,1746427,1746441,1746473,1746490,1746492,1746495-1746496,1746499-1746501,1746503-1746507,1746509,1746549,1746551,1746554,1746556,1746558,1746584,1746620,1746649,1746724,1746939,1746989,1747014,1747028,1747035,1747210 Modified: tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1747213&r1=1747212&r2=1747213&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Tue Jun 7 12:57:55 2016 @@ -97,29 +97,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); @@ -380,9 +357,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/tc8.5.x/trunk/java/org/apache/catalina/core/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1747213&r1=1747212&r2=1747213&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/LocalStrings.properties (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/catalina/core/LocalStrings.properties Tue Jun 7 12:57:55 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/tc8.5.x/trunk/java/org/apache/coyote/ActionCode.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/coyote/ActionCode.java?rev=1747213&r1=1747212&r2=1747213&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/coyote/ActionCode.java (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/coyote/ActionCode.java Tue Jun 7 12:57:55 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/tc8.5.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1747213&r1=1747212&r2=1747213&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Tue Jun 7 12:57:55 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/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java?rev=1747213&r1=1747212&r2=1747213&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java Tue Jun 7 12:57:55 2016 @@ -321,6 +321,9 @@ public class Http11InputBuffer implement lastValid = 0; pos = 0; + + System.out.println("Http11InputBuffer.recycle(): pos [" + pos + "], lastValid [" + lastValid + "]"); + 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/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11Processor.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1747213&r1=1747212&r2=1747213&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11Processor.java (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/coyote/http11/Http11Processor.java Tue Jun 7 12:57:55 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/tc8.5.x/trunk/java/org/apache/coyote/http2/StreamProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1747213&r1=1747212&r2=1747213&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/StreamProcessor.java (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/StreamProcessor.java Tue Jun 7 12:57:55 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/tc8.5.x/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java?rev=1747213&r1=1747212&r2=1747213&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java (original) +++ tomcat/tc8.5.x/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java Tue Jun 7 12:57:55 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, final 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