On 28/07/2017 17:09, Konstantin Kolinko wrote: Both issues fixed. Thanks for the review.
Mark > 2017-07-28 17:50 GMT+03:00 <[email protected]>: >> Author: markt >> Date: Fri Jul 28 14:50:36 2017 >> New Revision: 1803278 >> >> URL: http://svn.apache.org/viewvc?rev=1803278&view=rev >> Log: >> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61164 >> Add support for the %X pattern in the AccessLogValve that reports the >> connection status at the end of the request. Patch provided by Zemian Deng. >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/valves/AbstractAccessLogValve.java >> tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java >> tomcat/trunk/java/org/apache/coyote/ActionCode.java >> tomcat/trunk/webapps/docs/changelog.xml >> tomcat/trunk/webapps/docs/config/valve.xml >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/valves/AbstractAccessLogValve.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/AbstractAccessLogValve.java?rev=1803278&r1=1803277&r2=1803278&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/catalina/valves/AbstractAccessLogValve.java >> (original) >> +++ tomcat/trunk/java/org/apache/catalina/valves/AbstractAccessLogValve.java >> Fri Jul 28 14:50:36 2017 >> @@ -30,7 +30,9 @@ import java.util.List; >> import java.util.Locale; >> import java.util.Map; >> import java.util.TimeZone; >> +import java.util.concurrent.atomic.AtomicBoolean; >> >> +import javax.servlet.RequestDispatcher; >> import javax.servlet.ServletException; >> import javax.servlet.http.Cookie; >> import javax.servlet.http.HttpSession; >> @@ -40,9 +42,11 @@ import org.apache.catalina.Globals; >> import org.apache.catalina.LifecycleException; >> import org.apache.catalina.LifecycleState; >> import org.apache.catalina.Session; >> +import org.apache.catalina.connector.ClientAbortException; >> import org.apache.catalina.connector.Request; >> import org.apache.catalina.connector.Response; >> import org.apache.catalina.util.TLSUtil; >> +import org.apache.coyote.ActionCode; >> import org.apache.coyote.RequestInfo; >> import org.apache.juli.logging.Log; >> import org.apache.juli.logging.LogFactory; >> @@ -83,7 +87,14 @@ import org.apache.tomcat.util.collection >> * <li><b>%v</b> - Local server name >> * <li><b>%D</b> - Time taken to process the request, in millis >> * <li><b>%T</b> - Time taken to process the request, in seconds >> + * <li><b>%F</b> - Time taken to commit the response, in millis >> * <li><b>%I</b> - current Request thread name (can compare later with >> stacktraces) >> + * <li><b>%X</b> - Connection status when response is completed: >> + * <ul> >> + * <li><code>X</code> = Connection aborted before the response >> completed.</li> >> + * <li><code>+</code> = Connection may be kept alive after the response >> is sent.</li> >> + * <li><code>-</code> = Connection will be closed after the response is >> sent.</li> >> + * </ul> >> * </ul> >> * <p>In addition, the caller can specify one of the following aliases for >> * commonly utilized patterns:</p> >> @@ -1506,6 +1517,47 @@ public abstract class AbstractAccessLogV >> } >> } >> >> + /** >> + * Write connection status when response is completed - %X >> + */ >> + protected static class ConnectionStatusElement implements >> AccessLogElement { >> + @Override >> + public void addElement(CharArrayWriter buf, Date date, Request >> request, Response response, long time) { >> + if (response != null && request != null) { >> + boolean statusFound = false; >> + >> + // Check whether connection IO is in "not allowed" state >> + AtomicBoolean isIoAllowed = new AtomicBoolean(false); >> + request.getCoyoteRequest().action(ActionCode.IS_IO_ALLOWED, >> isIoAllowed); >> + if (!isIoAllowed.get()) { >> + buf.append('X'); >> + statusFound = true; >> + } else { >> + // Check for connection aborted cond >> + if (response.isError()) { >> + Throwable ex = (Throwable) >> request.getAttribute(RequestDispatcher.ERROR_EXCEPTION); >> + if (ex instanceof ClientAbortException) { >> + buf.append('X'); >> + statusFound = true; >> + } >> + } >> + } >> + >> + // If status is not found yet, cont to check whether >> connection is keep-alive or close >> + if (!statusFound) { >> + String connStatus = >> response.getHeader(org.apache.coyote.http11.Constants.CONNECTION); >> + if >> (org.apache.coyote.http11.Constants.CLOSE.equals(connStatus)) { > > I think it needs equalsIgnoreCase(), as the "Close" header can be set > by web application code as well and thus can be in any case > (to force Tomcat to close the connection). > > BTW, I wonder what happens if it is an HTTP/1.0 client. > > (I think there will be a "Close" header in the response anyway, so > this code works as well.) > >> + buf.append('-'); >> + } else { >> + buf.append('+'); >> + } >> + } >> + } else { >> + // Unknown connection status >> + buf.append('?'); >> + } >> + } >> + } >> >> /** >> * Parse pattern string and create the array of AccessLogElement. >> @@ -1636,6 +1688,8 @@ public abstract class AbstractAccessLogV >> return new LocalServerNameElement(); >> case 'I': >> return new ThreadNameElement(); >> + case 'X': >> + return new ConnectionStatusElement(); >> default: >> return new StringElement("???" + pattern + "???"); >> } >> >> Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1803278&r1=1803277&r2=1803278&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original) >> +++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Fri Jul 28 >> 14:50:36 2017 >> @@ -306,6 +306,10 @@ public abstract class AbstractProcessor >> ((AtomicBoolean) param).set(getErrorState().isError()); >> break; >> } >> + case IS_IO_ALLOWED: { >> + ((AtomicBoolean) param).set(getErrorState().isIoAllowed()); >> + break; >> + } >> case CLOSE_NOW: { >> // Prevent further writes to the response >> setSwallowResponse(); >> >> Modified: tomcat/trunk/java/org/apache/coyote/ActionCode.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ActionCode.java?rev=1803278&r1=1803277&r2=1803278&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/coyote/ActionCode.java (original) >> +++ tomcat/trunk/java/org/apache/coyote/ActionCode.java Fri Jul 28 14:50:36 >> 2017 >> @@ -54,6 +54,11 @@ public enum ActionCode { >> IS_ERROR, >> >> /** >> + * Is connection IO allowed after processor is in error stated. > > s/stated/state/, or rewrite the phrase as a whole. > >> + */ >> + IS_IO_ALLOWED, >> + > > Best regards, > Konstantin Kolinko > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
