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 2016/06/07 12:57:55 UTC
svn commit: r1747213 - in /tomcat/tc8.5.x/trunk: ./
java/org/apache/catalina/core/ java/org/apache/coyote/
java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
java/org/apache/coyote/http2/ test/org/apache/coyote/http11/
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org