Author: markt
Date: Fri Mar 24 20:51:20 2017
New Revision: 1788544

URL: http://svn.apache.org/viewvc?rev=1788544&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60918
When sendfile processing passes to the Poller for completion and then completes 
before Http11Processor.service() exists, the Processor is recycled which clears 
sendfileData causing the Processor to return CLOSED or OPEN rather than 
SENDFILE.

Modified:
    tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
    tomcat/trunk/webapps/docs/changelog.xml

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=1788544&r1=1788543&r2=1788544&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Fri Mar 24 
20:51:20 2017
@@ -55,6 +55,7 @@ import org.apache.tomcat.util.log.UserDa
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.SSLSupport;
 import org.apache.tomcat.util.net.SendfileDataBase;
+import org.apache.tomcat.util.net.SendfileState;
 import org.apache.tomcat.util.net.SocketWrapperBase;
 import org.apache.tomcat.util.res.StringManager;
 
@@ -368,9 +369,10 @@ public class Http11Processor extends Abs
         openSocket = false;
         readComplete = true;
         boolean keptAlive = false;
+        SendfileState sendfileState = SendfileState.DONE;
 
-        while (!getErrorState().isError() && keepAlive && !isAsync() &&
-                upgradeToken == null && !protocol.isPaused()) {
+        while (!getErrorState().isError() && keepAlive && !isAsync() && 
upgradeToken == null &&
+                sendfileState == SendfileState.DONE && !protocol.isPaused()) {
 
             // Parsing the request header
             try {
@@ -561,9 +563,7 @@ public class Http11Processor extends Abs
 
             rp.setStage(org.apache.coyote.Constants.STAGE_KEEPALIVE);
 
-            if (breakKeepAliveLoop(socketWrapper)) {
-                break;
-            }
+            sendfileState = processSendfile(socketWrapper);
         }
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
@@ -575,7 +575,7 @@ public class Http11Processor extends Abs
         } else if (isUpgrade()) {
             return SocketState.UPGRADING;
         } else {
-            if (sendfileData != null) {
+            if (sendfileState == SendfileState.PENDING) {
                 return SocketState.SENDFILE;
             } else {
                 if (openSocket) {
@@ -651,7 +651,6 @@ public class Http11Processor extends Abs
         http11 = true;
         http09 = false;
         contentDelimitation = false;
-        sendfileData = null;
 
         if (protocol.isSSLEnabled()) {
             request.scheme().setString("https");
@@ -858,15 +857,14 @@ public class Http11Processor extends Abs
         }
 
         // Sendfile support
-        boolean sendingWithSendfile = false;
         if (protocol.getUseSendfile()) {
-            sendingWithSendfile = prepareSendfile(outputFilters);
+            prepareSendfile(outputFilters);
         }
 
         // Check for compression
         boolean isCompressible = false;
         boolean useCompression = false;
-        if (entityBody && (protocol.getCompressionLevel() > 0) && 
!sendingWithSendfile) {
+        if (entityBody && (protocol.getCompressionLevel() > 0) && sendfileData 
== null) {
             isCompressible = isCompressible();
             if (isCompressible) {
                 useCompression = useCompression();
@@ -1009,10 +1007,12 @@ public class Http11Processor extends Abs
         return connection.equals(Constants.CLOSE);
     }
 
-    private boolean prepareSendfile(OutputFilter[] outputFilters) {
+    private void prepareSendfile(OutputFilter[] outputFilters) {
         String fileName = (String) request.getAttribute(
                 org.apache.coyote.Constants.SENDFILE_FILENAME_ATTR);
-        if (fileName != null) {
+        if (fileName == null) {
+            sendfileData = null;
+        } else {
             // No entity body sent here
             outputBuffer.addActiveFilter(outputFilters[Constants.VOID_FILTER]);
             contentDelimitation = true;
@@ -1021,9 +1021,7 @@ public class Http11Processor extends Abs
             long end = ((Long) request.getAttribute(
                     
org.apache.coyote.Constants.SENDFILE_FILE_END_ATTR)).longValue();
             sendfileData = socketWrapper.createSendfileData(fileName, pos, end 
- pos);
-            return true;
         }
-        return false;
     }
 
     /**
@@ -1304,34 +1302,31 @@ public class Http11Processor extends Abs
 
 
     /**
-     * Checks to see if the keep-alive loop should be broken, performing any
-     * processing (e.g. sendfile handling) that may have an impact on whether
-     * or not the keep-alive loop should be broken.
+     * Trigger sendfile processing if required.
      *
-     * @return true if the keep-alive loop should be broken
+     * @return The state of send file processing
      */
-    private boolean breakKeepAliveLoop(SocketWrapperBase<?> socketWrapper) {
+    private SendfileState processSendfile(SocketWrapperBase<?> socketWrapper) {
         openSocket = keepAlive;
+        // Done is equivalent to sendfile not being used
+        SendfileState result = SendfileState.DONE;
         // Do sendfile as needed: add socket to sendfile and end
         if (sendfileData != null && !getErrorState().isError()) {
             sendfileData.keepAlive = keepAlive;
-            switch (socketWrapper.processSendfile(sendfileData)) {
-            case DONE:
-                // If sendfile is complete, no need to break keep-alive loop
-                sendfileData = null;
-                return false;
-            case PENDING:
-                return true;
+            result = socketWrapper.processSendfile(sendfileData);
+            switch (result) {
             case ERROR:
                 // Write failed
                 if (log.isDebugEnabled()) {
                     log.debug(sm.getString("http11processor.sendfile.error"));
                 }
                 setErrorState(ErrorState.CLOSE_CONNECTION_NOW, null);
-                return true;
+                //$FALL-THROUGH$
+            default:
+                sendfileData = null;
             }
         }
-        return false;
+        return result;
     }
 
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1788544&r1=1788543&r2=1788544&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Mar 24 20:51:20 2017
@@ -132,6 +132,11 @@
         Improve HPACK specification compliance by fixing some test failures
         reported by the h2spec tool written by Moto Ishizawa. (markt)
       </fix>
+      <fix>
+        <bug>60918</bug>: Fix sendfile processing error that could lead to
+        subsequent requests experiencing and 
<code>IllegalStateException</code>.
+        (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to