On Fri, Jan 24, 2020 at 8:11 PM Christopher Schultz < ch...@christopherschultz.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > Rémy, > > On 1/23/20 4:28 AM, r...@apache.org wrote: > > 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 d5dce75 Add new connector attribute to control facade > > recycling d5dce75 is described below > > > > commit d5dce75b5c55194065f585c9a0b9cf606eb3a165 Author: remm > > <r...@apache.org> AuthorDate: Wed Jan 22 15:35:37 2020 +0100 > > > > Add new connector attribute to control facade recycling > > > > Use the same default as before, using the system property and false > > if not set. > > Did you intend to change the public interface of this class? > > RECYCLE_FACADES -> (gone) > > IMO it's too late in 9.0.x / 8.5.x / 7.0.x to remove this. Tomcat 10 > is fine, but this constant should remain in the other versions. > Ooops, I never thought of that constant that way but I agree. I'll add it back undocumented. Rémy > > - -chris > > > --- java/org/apache/catalina/connector/Connector.java | 38 > > ++++++++++++++++++----- > > 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 | 11 +++++++ > > webapps/docs/security-howto.xml | 10 +++--- 6 > > files changed, 64 insertions(+), 16 deletions(-) > > > > diff --git a/java/org/apache/catalina/connector/Connector.java > > b/java/org/apache/catalina/connector/Connector.java index > > 515d2af..d54927c 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,17 @@ 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 = + > > Boolean.parseBoolean(System.getProperty("org.apache.catalina.connector > .RECYCLE_FACADES", > > "false")); + + + /** * The redirect port for non-SSL to SSL > > redirects. */ protected int redirectPort = 443; @@ -373,6 +378,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 > > 841049a..7f27bdd 100644 --- > > a/java/org/apache/catalina/connector/Request.java +++ > > b/java/org/apache/catalina/connector/Request.java @@ -495,7 +495,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); @@ -506,7 +506,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; @@ -586,6 > > +586,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 > > d06a46c..37515ed 100644 --- > > a/java/org/apache/catalina/connector/Response.java +++ > > b/java/org/apache/catalina/connector/Response.java @@ -44,7 +44,6 > > @@ import javax.servlet.http.HttpServletResponse; import > > javax.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; @@ -229,7 +228,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 > > ae9d505..9473e86 100644 --- a/webapps/docs/changelog.xml +++ > > b/webapps/docs/changelog.xml @@ -119,6 +119,10 @@ > > <code>CachedResourceURLConnection</code>, it expands the feature > > to include packedWARs and to take account of resource JARs. > > (markt) </fix> + <update> + Refactor recycle facade > > system property into a new connector attribute + named > > <code>discardFacades</code>. (remm) + </update> </changelog> > > </subsection> <subsection name="Coyote"> diff --git > > a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml index > > 057ee47..7a92820 100644 --- a/webapps/docs/config/http.xml +++ > > b/webapps/docs/config/http.xml @@ -94,6 +94,17 @@ > > <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 the value of the + > > <code>org.apache.catalina.connector.RECYCLE_FACADES</code> system + > > property, or <code>false</code> if not set.</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/security-howto.xml > > b/webapps/docs/security-howto.xml index 8b3d14d..4b21e01 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 + 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</stron > g> > > > > > > > > --------------------------------------------------------------------- > > > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > > For additional commands, e-mail: dev-h...@tomcat.apache.org > > > -----BEGIN PGP SIGNATURE----- > Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/ > > iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl4rQT4ACgkQHPApP6U8 > pFhfgQ/+Ikx1U2RVG5PFpOQOv+lOtN45wcC8/rc/RkbPamBMp9pzxjI8uQbMXWti > yod4SkMonmDQJuVSENTKmA5vvMLzxeWpwWgMQbNje9uc5Pja0JhnfRwO26xXAkxw > IwnyPQohGDFVqGDXE4MCQ7KVJae4a8MBCHkpvcgH7rWDMXxD6CGAzJG4MOLDAzJe > QyxVANYWA8tBAgqUxr/UF0SjH+nXTL+Ymc6jup3bAtPmalUzsFpp2PqhgJK4GKr3 > s/0hlegUq7YZwpUncN/qN3uwox+vehswgx25nMfr5yjzI831R+cNZN1aR2EGFs7X > WKVC5CPwxZo4Bfey503GTZXCB4HOBc4UkAA+t0guotkLMJd8Ir813cyhAG20BVa6 > fam627I4X9THdqr6cAuZKvFvWGuFIyGeI2Zv15rYvGqbf4iCT6+DvORmhZvmTBtx > xJQhd39dKO+0hqHkAq/6K6Gf1hTr56xYmFdkFvNG1LdoxttwzoIU0HAPsLlfOtJh > m7GQpnkXtMZPaEr5B4muw7ktaQypS4siiNy1auhgZro+GhWLPzi671tCOYh6XfuU > f+5XEHBKbHC/r2MYj2LqRIGhhBiTttv7rt2znfm3YoFvPpcfYvc9lm5BJF+CWMzZ > 8GUNlM8tg4Ra2152hV1gSHqVqM0ZOU6aHo3PkDd59vXslseNQEY= > =QjKh > -----END PGP SIGNATURE----- > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
