You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2020/04/07 18:59:07 UTC

[tomcat] branch 9.0.x updated: Improve Connector creation

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 <re...@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org