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