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