This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d701009958e1c2f16f10226f15a13268ae314a32
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Aug 16 08:50:56 2023 +0100
Remove Parameters.FailReason and associated plumbing
This includes removing the FailedRequestFilter
---
conf/web.xml | 21 ----
java/org/apache/catalina/Globals.java | 16 ---
java/org/apache/catalina/connector/Request.java | 26 -----
.../catalina/filters/FailedRequestFilter.java | 109 ---------------------
java/org/apache/tomcat/util/http/Parameters.java | 37 -------
.../org/apache/catalina/connector/TestRequest.java | 39 +-------
webapps/docs/config/ajp.xml | 15 ++-
webapps/docs/config/filter.xml | 43 --------
webapps/docs/config/http.xml | 15 ++-
webapps/docs/security-howto.xml | 10 +-
10 files changed, 15 insertions(+), 316 deletions(-)
diff --git a/conf/web.xml b/conf/web.xml
index 9ec69cc66e..89942a9e39 100644
--- a/conf/web.xml
+++ b/conf/web.xml
@@ -517,19 +517,6 @@
</filter>
-->
- <!-- A filter that triggers request parameters parsing and rejects the -->
- <!-- request if some parameters were skipped because of parsing errors or -->
- <!-- request size limitations. -->
-<!--
- <filter>
- <filter-name>failedRequestFilter</filter-name>
- <filter-class>
- org.apache.catalina.filters.FailedRequestFilter
- </filter-class>
- <async-supported>true</async-supported>
- </filter>
--->
-
<!-- NOTE: An SSI Servlet is also available as an alternative SSI -->
<!-- implementation. Use either the Servlet or the Filter but NOT both. -->
@@ -608,14 +595,6 @@
</filter-mapping>
-->
- <!-- The mapping for the Failed Request Filter -->
-<!--
- <filter-mapping>
- <filter-name>failedRequestFilter</filter-name>
- <url-pattern>/*</url-pattern>
- </filter-mapping>
--->
-
<!-- The mapping for the SSI Filter -->
<!--
<filter-mapping>
diff --git a/java/org/apache/catalina/Globals.java
b/java/org/apache/catalina/Globals.java
index 0d112ea6c2..c3d4e45710 100644
--- a/java/org/apache/catalina/Globals.java
+++ b/java/org/apache/catalina/Globals.java
@@ -50,22 +50,6 @@ public final class Globals {
public static final String NAMED_DISPATCHER_ATTR =
"org.apache.catalina.NAMED";
- /**
- * The request attribute that is set to {@code Boolean.TRUE} if some
request
- * parameters have been ignored during request parameters parsing. It can
- * happen, for example, if there is a limit on the total count of parseable
- * parameters, or if parameter cannot be decoded, or any other error
- * happened during parameter parsing.
- */
- public static final String PARAMETER_PARSE_FAILED_ATTR =
"org.apache.catalina.parameter_parse_failed";
-
-
- /**
- * The reason that the parameter parsing failed.
- */
- public static final String PARAMETER_PARSE_FAILED_REASON_ATTR =
"org.apache.catalina.parameter_parse_failed_reason";
-
-
/**
* The request attribute set by the RemoteIpFilter, RemoteIpValve (and may
* be set by other similar components) that identifies for the connector
the
diff --git a/java/org/apache/catalina/connector/Request.java
b/java/org/apache/catalina/connector/Request.java
index 869cae087d..82fea929f8 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -915,7 +915,6 @@ public class Request implements HttpServletRequest {
* <li>{@link Globals#KEY_SIZE_ATTR} (SSL connections only)</li>
* <li>{@link Globals#SSL_SESSION_ID_ATTR} (SSL connections only)</li>
* <li>{@link Globals#SSL_SESSION_MGR_ATTR} (SSL connections only)</li>
- * <li>{@link Globals#PARAMETER_PARSE_FAILED_ATTR}</li>
* </ul>
* The underlying connector may also expose request attributes. These all
have names starting with
* "org.apache.tomcat" and include:
@@ -3287,31 +3286,6 @@ public class Request implements HttpServletRequest {
// NO-OP
}
});
- specialAttributes.put(Globals.PARAMETER_PARSE_FAILED_ATTR, new
SpecialAttributeAdapter() {
- @Override
- public Object get(Request request, String name) {
- if
(request.getCoyoteRequest().getParameters().isParseFailed()) {
- return Boolean.TRUE;
- }
- return null;
- }
-
- @Override
- public void set(Request request, String name, Object value) {
- // NO-OP
- }
- });
- specialAttributes.put(Globals.PARAMETER_PARSE_FAILED_REASON_ATTR, new
SpecialAttributeAdapter() {
- @Override
- public Object get(Request request, String name) {
- return
request.getCoyoteRequest().getParameters().getParseFailedReason();
- }
-
- @Override
- public void set(Request request, String name, Object value) {
- // NO-OP
- }
- });
specialAttributes.put(Globals.SENDFILE_SUPPORTED_ATTR, new
SpecialAttributeAdapter() {
@Override
public Object get(Request request, String name) {
diff --git a/java/org/apache/catalina/filters/FailedRequestFilter.java
b/java/org/apache/catalina/filters/FailedRequestFilter.java
deleted file mode 100644
index 98032f8de9..0000000000
--- a/java/org/apache/catalina/filters/FailedRequestFilter.java
+++ /dev/null
@@ -1,109 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.catalina.filters;
-
-import java.io.IOException;
-
-import jakarta.servlet.FilterChain;
-import jakarta.servlet.ServletException;
-import jakarta.servlet.ServletRequest;
-import jakarta.servlet.ServletResponse;
-import jakarta.servlet.http.HttpServletResponse;
-
-import org.apache.catalina.Globals;
-import org.apache.juli.logging.Log;
-import org.apache.juli.logging.LogFactory;
-import org.apache.tomcat.util.http.Parameters.FailReason;
-
-/**
- * Filter that will reject requests if there was a failure during parameter
parsing. This filter can be used to ensure
- * that none parameter values submitted by client are lost.
- * <p>
- * Note that parameter parsing may consume the body of an HTTP request, so
caution is needed if the servlet protected by
- * this filter uses <code>request.getInputStream()</code> or
<code>request.getReader()</code> calls. In general the risk
- * of breaking a web application by adding this filter is not so high, because
parameter parsing does check content type
- * of the request before consuming the request body.
- */
-public class FailedRequestFilter extends FilterBase {
-
- // Log must be non-static as loggers are created per class-loader and this
- // Filter may be used in multiple class loaders
- private final Log log = LogFactory.getLog(FailedRequestFilter.class); //
must not be static
-
- @Override
- protected Log getLogger() {
- return log;
- }
-
- @Override
- public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain)
- throws IOException, ServletException {
- if (!isGoodRequest(request)) {
- FailReason reason = (FailReason)
request.getAttribute(Globals.PARAMETER_PARSE_FAILED_REASON_ATTR);
-
- int status;
-
- switch (reason) {
- case IO_ERROR:
- // Not the client's fault
- status = HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
- break;
- case POST_TOO_LARGE:
- status = HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE;
- break;
- case TOO_MANY_PARAMETERS:
- // 413/414 aren't really correct here since the request
body
- // and/or URI could be well below any limits set. Use the
- // default.
- case UNKNOWN: // Assume the client is at fault
- // Various things that the client can get wrong that don't
have
- // a specific status code so use the default.
- case INVALID_CONTENT_TYPE:
- case MULTIPART_CONFIG_INVALID:
- case NO_NAME:
- case URL_DECODING:
- case CLIENT_DISCONNECT:
- // Client is never going to see this so this is really just
- // for the access logs. The default is fine.
- default:
- // 400
- status = HttpServletResponse.SC_BAD_REQUEST;
- break;
- }
-
- ((HttpServletResponse) response).sendError(status);
- return;
- }
- chain.doFilter(request, response);
- }
-
- private boolean isGoodRequest(ServletRequest request) {
- // Trigger parsing of parameters
- request.getParameter("none");
- // Detect failure
- if (request.getAttribute(Globals.PARAMETER_PARSE_FAILED_ATTR) != null)
{
- return false;
- }
- return true;
- }
-
- @Override
- protected boolean isConfigProblemFatal() {
- return true;
- }
-
-}
diff --git a/java/org/apache/tomcat/util/http/Parameters.java
b/java/org/apache/tomcat/util/http/Parameters.java
index 4c517e83c4..feb4c03e97 100644
--- a/java/org/apache/tomcat/util/http/Parameters.java
+++ b/java/org/apache/tomcat/util/http/Parameters.java
@@ -54,12 +54,6 @@ public final class Parameters {
private int limit = -1;
private int parameterCount = 0;
- /**
- * Set to the reason for the failure (the first failure if there is more
than one) if there were failures during
- * parameter parsing.
- */
- private FailReason parseFailedReason = null;
-
public Parameters() {
// NO-OP
}
@@ -98,23 +92,6 @@ public final class Parameters {
}
- public boolean isParseFailed() {
- return parseFailedReason != null;
- }
-
-
- public FailReason getParseFailedReason() {
- return parseFailedReason;
- }
-
-
- public void setParseFailedReason(FailReason failReason) {
- if (this.parseFailedReason == null) {
- this.parseFailedReason = failReason;
- }
- }
-
-
public int size() {
return parameterCount;
}
@@ -126,7 +103,6 @@ public final class Parameters {
didQueryParameters = false;
charset = DEFAULT_BODY_CHARSET;
decodedQuery.recycle();
- parseFailedReason = null;
}
@@ -416,17 +392,4 @@ public final class Parameters {
}
return sb.toString();
}
-
-
- public enum FailReason {
- CLIENT_DISCONNECT,
- MULTIPART_CONFIG_INVALID,
- INVALID_CONTENT_TYPE,
- IO_ERROR,
- NO_NAME,
- POST_TOO_LARGE,
- TOO_MANY_PARAMETERS,
- UNKNOWN,
- URL_DECODING
- }
}
diff --git a/test/org/apache/catalina/connector/TestRequest.java
b/test/org/apache/catalina/connector/TestRequest.java
index 4c166d3201..c6b4d4edb5 100644
--- a/test/org/apache/catalina/connector/TestRequest.java
+++ b/test/org/apache/catalina/connector/TestRequest.java
@@ -48,7 +48,6 @@ import org.junit.Test;
import org.apache.catalina.Context;
import org.apache.catalina.authenticator.BasicAuthenticator;
-import org.apache.catalina.filters.FailedRequestFilter;
import org.apache.catalina.startup.SimpleHttpClient;
import org.apache.catalina.startup.TesterMapRealm;
import org.apache.catalina.startup.Tomcat;
@@ -56,8 +55,6 @@ import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.tomcat.unittest.TesterRequest;
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.buf.EncodedSolidusHandling;
-import org.apache.tomcat.util.descriptor.web.FilterDef;
-import org.apache.tomcat.util.descriptor.web.FilterMap;
import org.apache.tomcat.util.descriptor.web.LoginConfig;
/**
@@ -73,7 +70,7 @@ public class TestRequest extends TomcatBaseTest {
*/
@Test
public void testBug37794() {
- Bug37794Client client = new Bug37794Client(true);
+ Bug37794Client client = new Bug37794Client();
// Edge cases around zero
client.doRequest(-1, false); // Unlimited
@@ -112,22 +109,6 @@ public class TestRequest extends TomcatBaseTest {
Assert.assertTrue(client.isResponseBodyOK());
}
- /**
- * Additional test for failed requests handling when no FailedRequestFilter
- * is defined.
- */
- @Test
- public void testBug37794withoutFilter() {
- Bug37794Client client = new Bug37794Client(false);
-
- // Edge cases around actual content length
- client.reset();
- client.doRequest(6, false); // Too small should fail
- // Response code will be OK, but parameters list will be empty
- Assert.assertTrue(client.isResponse200());
- Assert.assertEquals("", client.getResponseBody());
- }
-
private static class Bug37794Servlet extends HttpServlet {
private static final long serialVersionUID = 1L;
@@ -157,14 +138,8 @@ public class TestRequest extends TomcatBaseTest {
*/
private class Bug37794Client extends SimpleHttpClient {
- private final boolean createFilter;
-
private boolean init;
- Bug37794Client(boolean createFilter) {
- this.createFilter = createFilter;
- }
-
private synchronized void init() throws Exception {
if (init) {
return;
@@ -175,18 +150,6 @@ public class TestRequest extends TomcatBaseTest {
Tomcat.addServlet(root, "Bug37794", new Bug37794Servlet());
root.addServletMappingDecoded("/test", "Bug37794");
- if (createFilter) {
- FilterDef failedRequestFilter = new FilterDef();
- failedRequestFilter.setFilterName("failedRequestFilter");
- failedRequestFilter.setFilterClass(
- FailedRequestFilter.class.getName());
- FilterMap failedRequestFilterMap = new FilterMap();
- failedRequestFilterMap.setFilterName("failedRequestFilter");
- failedRequestFilterMap.addURLPatternDecoded("/*");
- root.addFilterDef(failedRequestFilter);
- root.addFilterMap(failedRequestFilterMap);
- }
-
tomcat.start();
setPort(tomcat.getConnector().getLocalPort());
diff --git a/webapps/docs/config/ajp.xml b/webapps/docs/config/ajp.xml
index bbccae65d0..6eda243d89 100644
--- a/webapps/docs/config/ajp.xml
+++ b/webapps/docs/config/ajp.xml
@@ -152,20 +152,17 @@
files) obtained from the query string and, for POST requests, the request
body if the content type is
<code>application/x-www-form-urlencoded</code> or
- <code>multipart/form-data</code>. Request parameters beyond this limit
- will be ignored. A value of less than 0 means no limit. If not specified,
- a default of 1000 is used. Note that <code>FailedRequestFilter</code>
- <a href="filter.html">filter</a> can be used to reject requests that
- exceed the limit.</p>
+ <code>multipart/form-data</code>. Requests that exceed this limit will be
+ rejected. A value of less than 0 means no limit. If not specified, a
+ default of 1000 is used.</p>
</attribute>
<attribute name="maxPostSize" required="false">
<p>The maximum size in bytes of the POST which will be handled by
the container FORM URL parameter parsing. The limit can be disabled by
- setting this attribute to a value less than zero. If not specified, this
- attribute is set to 2097152 (2 MiB). Note that the
- <a
href="filter.html#Failed_Request_Filter"><code>FailedRequestFilter</code></a>
- can be used to reject requests that exceed this limit.</p>
+ setting this attribute to a value less than zero. Requests that exceed
+ this limit will be rejected. If not specified, this attribute is set to
+ 2097152 (2 MiB).</p>
</attribute>
<attribute name="maxSavePostSize" required="false">
diff --git a/webapps/docs/config/filter.xml b/webapps/docs/config/filter.xml
index 4ed22b776e..54ca8d3bfb 100644
--- a/webapps/docs/config/filter.xml
+++ b/webapps/docs/config/filter.xml
@@ -824,49 +824,6 @@ FINE: Request "/docs/config/manager.html" with response
status "200"
</section>
-<section name="Failed Request Filter">
-
- <subsection name="Introduction">
-
- <p>This filter triggers parameters parsing in a request and rejects the
- request if some parameters were skipped during parameter parsing because
- of parsing errors or request size limitations (such as
- <code>maxParameterCount</code> attribute in a
- <a href="http.html">Connector</a>).
- This filter can be used to ensure that none parameter values submitted by
- client are lost.</p>
-
- <p>Note that parameter parsing may consume the body of an HTTP request, so
- caution is needed if the servlet protected by this filter uses
- <code>request.getInputStream()</code> or <code>request.getReader()</code>
- calls. In general the risk of breaking a web application by adding this
- filter is not so high, because parameter parsing does check content type
- of the request before consuming the request body.</p>
-
- <p>Note, that for the POST requests to be parsed correctly, a
- <code>SetCharacterEncodingFilter</code> filter must be configured above
- this one. See CharacterEncoding page in the FAQ for details.</p>
-
- <p>The request is rejected with HTTP status code 400 (Bad Request).</p>
-
- </subsection>
-
- <subsection name="Filter Class Name">
-
- <p>The filter class name for the Failed Request Filter is
- <strong><code>org.apache.catalina.filters.FailedRequestFilter</code>
- </strong>.</p>
-
- </subsection>
-
- <subsection name="Initialisation parameters">
-
- <p>The Failed Request Filter does not support any initialization
parameters.</p>
-
- </subsection>
-
-</section>
-
<section name="HTTP Header Security Filter">
<subsection name="Introduction">
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index 9a6ac3e832..5bc3c4a6e4 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -148,20 +148,17 @@
files) obtained from the query string and, for POST requests, the request
body if the content type is
<code>application/x-www-form-urlencoded</code> or
- <code>multipart/form-data</code>. Request parameters beyond this limit
- will be ignored. A value of less than 0 means no limit. If not specified,
- a default of 1000 is used. Note that <code>FailedRequestFilter</code>
- <a href="filter.html">filter</a> can be used to reject requests that
- exceed the limit.</p>
+ <code>multipart/form-data</code>. Requests that exceed this limit will be
+ rejected. A value of less than 0 means no limit. If not specified, a
+ default of 1000 is used.</p>
</attribute>
<attribute name="maxPostSize" required="false">
<p>The maximum size in bytes of the POST which will be handled by
the container FORM URL parameter parsing. The limit can be disabled by
- setting this attribute to a value less than zero. If not specified, this
- attribute is set to 2097152 (2 MiB). Note that the
- <a
href="filter.html#Failed_Request_Filter"><code>FailedRequestFilter</code></a>
- can be used to reject requests that exceed this limit.</p>
+ setting this attribute to a value less than zero. Requests that exceed
+ this limit will be rejected. If not specified, this attribute is set to
+ 2097152 (2 MiB).</p>
</attribute>
<attribute name="maxSavePostSize" required="false">
diff --git a/webapps/docs/security-howto.xml b/webapps/docs/security-howto.xml
index 5751559455..0ca99031c1 100644
--- a/webapps/docs/security-howto.xml
+++ b/webapps/docs/security-howto.xml
@@ -276,9 +276,8 @@
total number of request parameters (including uploaded files) obtained
from the query string and, for POST requests, the request body if the
content type is <code>application/x-www-form-urlencoded</code> or
- <code>multipart/form-data</code>. Excessive parameters are ignored. If
you
- want to reject such requests, configure a
- <a href="config/filter.html">FailedRequestFilter</a>.</p>
+ <code>multipart/form-data</code>. Requests with excessive parameters are
+ rejected.</p>
<p>The <strong>xpoweredBy</strong> attribute controls whether or not the
X-Powered-By HTTP header is sent with each request. If sent, the value of
@@ -526,11 +525,6 @@
information on the potential risks and mitigations may be found by
following the links in the <a href="cgi-howto.html">CGI How To</a>.</p>
- <p><a href="config/filter.html">FailedRequestFilter</a>
- can be configured and used to reject requests that had errors during
- request parameter parsing. Without the filter the default behaviour is
- to ignore invalid or excessive parameters.</p>
-
<p><a href="config/filter.html">HttpHeaderSecurityFilter</a> can be
used to add headers to responses to improve security. If clients access
Tomcat directly, then you probably want to enable this filter and all the
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
