This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push: new e540983 Improve Connector creation e540983 is described below commit e540983e76998b7b41c4ad230a2cc219239f7800 Author: remm <r...@apache.org> AuthorDate: Tue Apr 7 20:58:54 2020 +0200 Improve Connector creation Remove unneeded reflection. More importantly remove AJP specific hack in favor of a new default method in ProtocolHandler. No behavior change for existing API. --- java/org/apache/catalina/connector/Connector.java | 66 +++++++------------- java/org/apache/coyote/ProtocolHandler.java | 71 ++++++++++++++++++++++ .../org/apache/coyote/ajp/AbstractAjpProtocol.java | 6 ++ webapps/docs/changelog.xml | 8 +++ 4 files changed, 106 insertions(+), 45 deletions(-) diff --git a/java/org/apache/catalina/connector/Connector.java b/java/org/apache/catalina/connector/Connector.java index 534246f..f30f26e 100644 --- a/java/org/apache/catalina/connector/Connector.java +++ b/java/org/apache/catalina/connector/Connector.java @@ -35,7 +35,6 @@ import org.apache.coyote.AbstractProtocol; import org.apache.coyote.Adapter; import org.apache.coyote.ProtocolHandler; import org.apache.coyote.UpgradeProtocol; -import org.apache.coyote.ajp.AbstractAjpProtocol; import org.apache.coyote.http11.AbstractHttp11JsseProtocol; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -75,49 +74,37 @@ public class Connector extends LifecycleMBeanBase { * Defaults to using HTTP/1.1 NIO implementation. */ public Connector() { - this("org.apache.coyote.http11.Http11NioProtocol"); + this("HTTP/1.1"); } public Connector(String protocol) { - boolean aprConnector = AprLifecycleListener.isAprAvailable() && + boolean apr = AprLifecycleListener.isAprAvailable() && AprLifecycleListener.getUseAprConnector(); - - if ("HTTP/1.1".equals(protocol) || protocol == null) { - if (aprConnector) { - protocolHandlerClassName = "org.apache.coyote.http11.Http11AprProtocol"; - } else { - protocolHandlerClassName = "org.apache.coyote.http11.Http11NioProtocol"; - } - } else if ("AJP/1.3".equals(protocol)) { - if (aprConnector) { - protocolHandlerClassName = "org.apache.coyote.ajp.AjpAprProtocol"; - } else { - protocolHandlerClassName = "org.apache.coyote.ajp.AjpNioProtocol"; - } - } else { - protocolHandlerClassName = protocol; - } - - // Instantiate protocol handler ProtocolHandler p = null; try { - Class<?> clazz = Class.forName(protocolHandlerClassName); - p = (ProtocolHandler) clazz.getConstructor().newInstance(); + p = ProtocolHandler.create(protocol, apr); } catch (Exception e) { log.error(sm.getString( "coyoteConnector.protocolHandlerInstantiationFailed"), e); - } finally { - this.protocolHandler = p; } - + if (p != null) { + protocolHandler = p; + protocolHandlerClassName = protocolHandler.getClass().getName(); + } else { + protocolHandler = null; + protocolHandlerClassName = protocol; + } // Default for Connector depends on this system property setThrowOnFailure(Boolean.getBoolean("org.apache.catalina.startup.EXIT_ON_INIT_FAILURE")); + } - // Default for Connector depends on this (deprecated) system property - if (Boolean.parseBoolean(System.getProperty("org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH", "false"))) { - encodedSolidusHandling = EncodedSolidusHandling.DECODE; - } + + public Connector(ProtocolHandler protocolHandler) { + protocolHandlerClassName = protocolHandler.getClass().getName(); + this.protocolHandler = protocolHandler; + // Default for Connector depends on this system property + setThrowOnFailure(Boolean.getBoolean("org.apache.catalina.startup.EXIT_ON_INIT_FAILURE")); } @@ -621,18 +608,7 @@ public class Connector extends LifecycleMBeanBase { * @return the Coyote protocol handler in use. */ public String getProtocol() { - if (("org.apache.coyote.http11.Http11NioProtocol".equals(getProtocolHandlerClassName()) && - (!AprLifecycleListener.isAprAvailable() || !AprLifecycleListener.getUseAprConnector())) || - "org.apache.coyote.http11.Http11AprProtocol".equals(getProtocolHandlerClassName()) && - AprLifecycleListener.getUseAprConnector()) { - return "HTTP/1.1"; - } else if (("org.apache.coyote.ajp.AjpNioProtocol".equals(getProtocolHandlerClassName()) && - (!AprLifecycleListener.isAprAvailable() || !AprLifecycleListener.getUseAprConnector())) || - "org.apache.coyote.ajp.AjpAprProtocol".equals(getProtocolHandlerClassName()) && - AprLifecycleListener.getUseAprConnector()) { - return "AJP/1.3"; - } - return getProtocolHandlerClassName(); + return ProtocolHandler.getProtocol(getProtocolHandlerClassName(), AprLifecycleListener.getUseAprConnector()); } @@ -928,9 +904,9 @@ public class Connector extends LifecycleMBeanBase { * @return a new Servlet response object */ public Response createResponse() { - if (protocolHandler instanceof AbstractAjpProtocol<?>) { - int packetSize = ((AbstractAjpProtocol<?>) protocolHandler).getPacketSize(); - return new Response(packetSize - org.apache.coyote.ajp.Constants.SEND_HEAD_LEN); + int size = protocolHandler.getDesiredBufferSize(); + if (size > 0) { + return new Response(size); } else { return new Response(); } diff --git a/java/org/apache/coyote/ProtocolHandler.java b/java/org/apache/coyote/ProtocolHandler.java index b467558..acdf202 100644 --- a/java/org/apache/coyote/ProtocolHandler.java +++ b/java/org/apache/coyote/ProtocolHandler.java @@ -16,6 +16,7 @@ */ package org.apache.coyote; +import java.lang.reflect.InvocationTargetException; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; @@ -179,4 +180,74 @@ public interface ProtocolHandler { * @return the protocols */ public UpgradeProtocol[] findUpgradeProtocols(); + + + /** + * Some protocols, like AJP, have a packet length that + * shouldn't be exceeded, and this can be used to adjust the buffering + * used by the application layer. + * @return the desired buffer size, or -1 if not relevant + */ + public default int getDesiredBufferSize() { + return -1; + } + + + /** + * Create a new ProtocolHandler for the given protocol. + * @param protocol the protocol + * @param apr if <code>true</code> the APR protcol handler will be used + * @return the newly instantiated protocol handler + * @throws ClassNotFoundException Specified protocol was not found + * @throws InstantiationException Specified protocol could not be instantiated + * @throws IllegalAccessException Exception occurred + * @throws IllegalArgumentException Exception occurred + * @throws InvocationTargetException Exception occurred + * @throws NoSuchMethodException Exception occurred + * @throws SecurityException Exception occurred + */ + public static ProtocolHandler create(String protocol, boolean apr) + throws ClassNotFoundException, InstantiationException, IllegalAccessException, + IllegalArgumentException, InvocationTargetException, NoSuchMethodException, SecurityException { + if (protocol == null || "HTTP/1.1".equals(protocol) + || (!apr && org.apache.coyote.http11.Http11NioProtocol.class.getName().equals(protocol)) + || (apr && org.apache.coyote.http11.Http11AprProtocol.class.getName().equals(protocol))) { + if (apr) { + return new org.apache.coyote.http11.Http11AprProtocol(); + } else { + return new org.apache.coyote.http11.Http11NioProtocol(); + } + } else if ("AJP/1.3".equals(protocol) + || (!apr && org.apache.coyote.ajp.AjpNioProtocol.class.getName().equals(protocol)) + || (apr && org.apache.coyote.ajp.AjpAprProtocol.class.getName().equals(protocol))) { + if (apr) { + return new org.apache.coyote.ajp.AjpAprProtocol(); + } else { + return new org.apache.coyote.ajp.AjpNioProtocol(); + } + } else { + // Instantiate protocol handler + Class<?> clazz = Class.forName(protocol); + return (ProtocolHandler) clazz.getConstructor().newInstance(); + } + } + + + /** + * Get the protocol name associated with the protocol class. + * @param protocolClassName the protocol class name + * @param apr if <code>true</code> the APR protcol handler will be used + * @return the protocol name + */ + public static String getProtocol(String protocolClassName, boolean apr) { + if ((!apr && org.apache.coyote.http11.Http11NioProtocol.class.getName().equals(protocolClassName)) + || (apr && org.apache.coyote.http11.Http11AprProtocol.class.getName().equals(protocolClassName))) { + return "HTTP/1.1"; + } else if ((!apr && org.apache.coyote.ajp.AjpNioProtocol.class.getName().equals(protocolClassName)) + || (apr && org.apache.coyote.ajp.AjpAprProtocol.class.getName().equals(protocolClassName))) { + return "AJP/1.3"; + } + return protocolClassName; + } + } diff --git a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java index abf8e60..c583114 100644 --- a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java +++ b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java @@ -215,6 +215,12 @@ public abstract class AbstractAjpProtocol<S> extends AbstractProtocol<S> { } + @Override + public int getDesiredBufferSize() { + return getPacketSize() - Constants.SEND_HEAD_LEN; + } + + // --------------------------------------------- SSL is not supported in AJP @Override diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 279f78e..d84ab31 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -45,6 +45,14 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 9.0.35 (markt)" rtext="in development"> + <subsection name="Catalina"> + <changelog> + <fix> + Reduce reflection use and remove AJP specific code in the Connector. + (remm/markt/fhanik) + </fix> + </changelog> + </subsection> <subsection name="Coyote"> <changelog> <fix> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org