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/01/22 14:35:57 UTC
[tomcat] branch master updated: Add new connector attribute to
control facade recycling
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 <re...@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: Add new connector attribute to
control facade recycling
Posted by Mark Thomas <ma...@apache.org>.
On 22/01/2020 15:12, Rémy Maucherat wrote:
> On Wed, Jan 22, 2020 at 3:35 PM <remm@apache.org
> <ma...@apache.org>> wrote:
>
> 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 <remm@apache.org <ma...@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.
>
>
> If acceptable, should I backport this ? [the default used will be the
> value of the system property, or false if not present]
+1 to back-port.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: Add new connector attribute to
control facade recycling
Posted by Rémy Maucherat <re...@apache.org>.
On Wed, Jan 22, 2020 at 3:35 PM <re...@apache.org> wrote:
> 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 <re...@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.
>
If acceptable, should I backport this ? [the default used will be the value
of the system property, or false if not present]
Rémy
> ---
> 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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>