This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new c7457dc  Add new connector attribute to control facade recycling
c7457dc is described below

commit c7457dcbd2e7b347addfa204a33604435da9c4c6
Author: remm <r...@apache.org>
AuthorDate: Wed Jan 22 15:35:37 2020 +0100

    Add new connector attribute to control facade recycling
    
    Enabled by default for now due to likely low performance impact [I will
    need to actually verify that]. The benefit of the default switch is that
    we don't need to initialize the default value with the system property.
    One system property down.
---
 java/org/apache/catalina/connector/Connector.java | 37 ++++++++++++++++++-----
 java/org/apache/catalina/connector/Request.java   | 14 +++++++--
 java/org/apache/catalina/connector/Response.java  |  3 +-
 webapps/docs/changelog.xml                        |  4 +++
 webapps/docs/config/http.xml                      |  9 ++++++
 webapps/docs/config/systemprops.xml               |  6 ----
 webapps/docs/security-howto.xml                   | 10 +++---
 7 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/java/org/apache/catalina/connector/Connector.java 
b/java/org/apache/catalina/connector/Connector.java
index 515d2af..f37f027 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -25,6 +25,7 @@ import java.util.HashSet;
 
 import javax.management.ObjectName;
 
+import org.apache.catalina.Globals;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.LifecycleState;
 import org.apache.catalina.Service;
@@ -56,13 +57,6 @@ public class Connector extends LifecycleMBeanBase  {
     private static final Log log = LogFactory.getLog(Connector.class);
 
 
-    /**
-     * Alternate flag to enable recycling of facades.
-     */
-    public static final boolean RECYCLE_FACADES =
-        
Boolean.parseBoolean(System.getProperty("org.apache.catalina.connector.RECYCLE_FACADES",
 "false"));
-
-
     public static final String INTERNAL_EXECUTOR_NAME = "Internal";
 
 
@@ -165,6 +159,16 @@ public class Connector extends LifecycleMBeanBase  {
 
 
     /**
+     * The flag that controls recycling of the facades of the request
+     * processing objects. If set to <code>true</code> the object facades
+     * will be discarded when the request is recycled. If the security
+     * manager is enabled, this setting is ignored and object facades are
+     * always discarded.
+     */
+    protected boolean discardFacades = true;
+
+
+    /**
      * The redirect port for non-SSL to SSL redirects.
      */
     protected int redirectPort = 443;
@@ -373,6 +377,25 @@ public class Connector extends LifecycleMBeanBase  {
 
 
     /**
+     * @return <code>true</code> if the object facades are discarded, either
+     *   when the discardFacades value is <code>true</code> or when the
+     *   security manager is enabled.
+     */
+    public boolean getDiscardFacades() {
+        return discardFacades || Globals.IS_SECURITY_ENABLED;
+    }
+
+
+    /**
+     * Set the recycling strategy for the object facades.
+     * @param discardFacades the new value of the flag
+     */
+    public void setDiscardFacades(boolean discardFacades) {
+        this.discardFacades = discardFacades;
+    }
+
+
+    /**
      * @return the "enable DNS lookups" flag.
      */
     public boolean getEnableLookups() {
diff --git a/java/org/apache/catalina/connector/Request.java 
b/java/org/apache/catalina/connector/Request.java
index a4860d5..edff176 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -463,7 +463,7 @@ public class Request implements HttpServletRequest {
         recycleSessionInfo();
         recycleCookieInfo(false);
 
-        if (Globals.IS_SECURITY_ENABLED || Connector.RECYCLE_FACADES) {
+        if (getDiscardFacades()) {
             parameterMap = new ParameterMap<>();
         } else {
             parameterMap.setLocked(false);
@@ -474,7 +474,7 @@ public class Request implements HttpServletRequest {
         applicationMapping.recycle();
 
         applicationRequest = null;
-        if (Globals.IS_SECURITY_ENABLED || Connector.RECYCLE_FACADES) {
+        if (getDiscardFacades()) {
             if (facade != null) {
                 facade.clear();
                 facade = null;
@@ -554,6 +554,16 @@ public class Request implements HttpServletRequest {
 
 
     /**
+     * Get the recycling strategy of the facade objects.
+     * @return the value of the flag as set on the connector, or
+     *   <code>true</code> if no connector is associated with this request
+     */
+    public boolean getDiscardFacades() {
+        return (connector == null) ? true : connector.getDiscardFacades();
+    }
+
+
+    /**
      * Filter chain associated with the request.
      */
     protected FilterChain filterChain = null;
diff --git a/java/org/apache/catalina/connector/Response.java 
b/java/org/apache/catalina/connector/Response.java
index 70baffb..fce5570 100644
--- a/java/org/apache/catalina/connector/Response.java
+++ b/java/org/apache/catalina/connector/Response.java
@@ -43,7 +43,6 @@ import jakarta.servlet.http.HttpServletResponse;
 import jakarta.servlet.http.HttpServletResponseWrapper;
 
 import org.apache.catalina.Context;
-import org.apache.catalina.Globals;
 import org.apache.catalina.Session;
 import org.apache.catalina.security.SecurityUtil;
 import org.apache.catalina.util.SessionConfig;
@@ -219,7 +218,7 @@ public class Response implements HttpServletResponse {
         isCharacterEncodingSet = false;
 
         applicationResponse = null;
-        if (Globals.IS_SECURITY_ENABLED || Connector.RECYCLE_FACADES) {
+        if (getRequest().getDiscardFacades()) {
             if (facade != null) {
                 facade.clear();
                 facade = null;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 940a2ff..d462162 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -54,6 +54,10 @@
   <subsection name="Catalina">
     <changelog>
       <update>
+        Refactor recycle facade system property into a new connector attribute
+        named <code>discardFacades</code> and enable it by default. (remm)
+      </update>
+      <update>
         Update to Jakarta Servlet 5.0, Jakarta Server Pages 3.0. Jakarta
         Expression Language 4.0, Jakarta WebSocket 2.0, Jakarta Authentication
         2.0 and Jakarta Annotations 2.0. (markt)
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index f3c868d..54fbd42 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -94,6 +94,15 @@
       <code>_default_</code> will be used.</p>
     </attribute>
 
+    <attribute name="discardFacades" required="false">
+      <p>A boolean value which can be used to enable or disable the recycling
+      of the facade objects that isolate the container internal request
+      processing objects. If set to <code>true</code> the facades will be
+      set for garbage collection after every request, otherwise they will be
+      reused. This setting has no effect when the security manager is enabled.
+      If not specified, this attribute is set to <code>true</code>.</p>
+    </attribute>
+
     <attribute name="enableLookups" required="false">
       <p>Set to <code>true</code> if you want calls to
       <code>request.getRemoteHost()</code> to perform DNS lookups in
diff --git a/webapps/docs/config/systemprops.xml 
b/webapps/docs/config/systemprops.xml
index 9294817..06623c8 100644
--- a/webapps/docs/config/systemprops.xml
+++ b/webapps/docs/config/systemprops.xml
@@ -262,12 +262,6 @@
 
   <properties>
 
-    <property name="org.apache.catalina.connector. RECYCLE_FACADES">
-      <p>If this is <code>true</code> or if a security manager is in use a new
-      facade object will be created for each request.</p>
-      <p>If not specified, the default value of <code>false</code> will be 
used.</p>
-    </property>
-
     <property
     name="org.apache.catalina.connector. CoyoteAdapter.ALLOW_BACKSLASH">
       <p>If this is <code>true</code> the '\' character will be permitted as a
diff --git a/webapps/docs/security-howto.xml b/webapps/docs/security-howto.xml
index 8b3d14d..b54a7dc 100644
--- a/webapps/docs/security-howto.xml
+++ b/webapps/docs/security-howto.xml
@@ -258,6 +258,11 @@
       handle the response from a TRACE request (which exposes the browser to an
       XSS attack), support for TRACE requests is disabled by default.</p>
 
+      <p>The <strong>discardFacades</strong> attribute set to <code>true</code>
+      will cause a new facade object to be created for each request. This is
+      the default value, and this reduces the chances of a bug in an
+      application exposing data from one request to another.</p>
+
       <p>The <strong>maxPostSize</strong> attribute controls the maximum size
       of a POST request that will be parsed for parameters. The parameters are
       cached for the duration of the request so this is limited to 2MB by
@@ -449,11 +454,6 @@
   </section>
 
   <section name="System Properties">
-    <p>Setting <strong>org.apache.catalina.connector.RECYCLE_FACADES</strong>
-    system property to <code>true</code> will cause a new facade object to be
-    created for each request. This reduces the chances of a bug in an
-    application exposing data from one request to another.</p>
-
     <p>The <strong>
     org.apache.catalina.connector.CoyoteAdapter.ALLOW_BACKSLASH</strong> and
     <strong>org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH</strong>


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

Reply via email to