You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2023/06/22 18:32:20 UTC
[tomcat] branch tmp created (now e1875c1cae)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch tmp
in repository https://gitbox.apache.org/repos/asf/tomcat.git
at e1875c1cae Add tests for parsing query string parameters
This branch includes the following new commits:
new 944951302e Add control of byte decoding errors to ByteChunk and StringCache
new 34d4ed59f5 Update Servlet API for parameter error handling changes
new 61009deff1 First pass at implementing changes for parameter parsing
new e1875c1cae Add tests for parsing query string parameters
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch tmp created (now e1875c1cae)
Posted by Mark Thomas <ma...@apache.org>.
On 22/06/2023 19:32, markt@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a change to branch tmp
Sorry for the noise. Pressed the wrong button in my IDE.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/04: Update Servlet API for parameter error handling changes
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch tmp
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 34d4ed59f50fa1eb4b373730e20bdb821f30a353
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jun 15 12:15:08 2023 +0100
Update Servlet API for parameter error handling changes
---
java/jakarta/servlet/ServletRequest.java | 56 ++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/java/jakarta/servlet/ServletRequest.java b/java/jakarta/servlet/ServletRequest.java
index 795502f3b4..1e700dd900 100644
--- a/java/jakarta/servlet/ServletRequest.java
+++ b/java/jakarta/servlet/ServletRequest.java
@@ -162,11 +162,25 @@ public interface ServletRequest {
* <p>
* If the parameter data was sent in the request body, such as occurs with an HTTP POST request, then reading the
* body directly via {@link #getInputStream} or {@link #getReader} can interfere with the execution of this method.
+ * <p>
+ * If not already parsed, calling this method will trigger the parsing of the query string, POSTed form data where
+ * the request body has content type is <code>application/x-www-form-urlencoded</code> and POSTed form data where
+ * the request body has content-type <code>multipart/form-data</code> and the Servlet is configured with a
+ * {@link jakarta.servlet.annotation.MultipartConfig} annotation or a <code>multipart-config</code> element in the
+ * deployment descriptor.
*
* @param name a <code>String</code> specifying the name of the parameter
*
* @return a <code>String</code> representing the single value of the parameter
*
+ * @throws IllegalStateException if parameter parsing is triggered and a problem is encountered parsing the
+ * parameters including, but not limited to: invalid percent (%nn) encoding;
+ * invalid byte sequence for the specified character set; I/O errors reading the
+ * request body; and triggering a container defined limit related to parameter
+ * parsing. Containers may provide container specific options to handle some or
+ * all of these errors in an alternative manner that may include not throwing an
+ * exception.
+ *
* @see #getParameterValues
*/
String getParameter(String name);
@@ -175,9 +189,23 @@ public interface ServletRequest {
* Returns an <code>Enumeration</code> of <code>String</code> objects containing the names of the parameters
* contained in this request. If the request has no parameters, the method returns an empty
* <code>Enumeration</code>.
+ * <p>
+ * If not already parsed, calling this method will trigger the parsing of the query string, POSTed form data where
+ * the request body has content type is <code>application/x-www-form-urlencoded</code> and POSTed form data where
+ * the request body has content-type <code>multipart/form-data</code> and the Servlet is configured with a
+ * {@link jakarta.servlet.annotation.MultipartConfig} annotation or a <code>multipart-config</code> element in the
+ * deployment descriptor.
*
* @return an <code>Enumeration</code> of <code>String</code> objects, each <code>String</code> containing the name
* of a request parameter; or an empty <code>Enumeration</code> if the request has no parameters
+ *
+ * @throws IllegalStateException if parameter parsing is triggered and a problem is encountered parsing the
+ * parameters including, but not limited to: invalid percent (%nn) encoding;
+ * invalid byte sequence for the specified character set; I/O errors reading the
+ * request body; and triggering a container defined limit related to parameter
+ * parsing. Containers may provide container specific options to handle some or
+ * all of these errors in an alternative manner that may include not throwing an
+ * exception.
*/
Enumeration<String> getParameterNames();
@@ -186,11 +214,25 @@ public interface ServletRequest {
* <code>null</code> if the parameter does not exist.
* <p>
* If the parameter has a single value, the array has a length of 1.
+ * <p>
+ * If not already parsed, calling this method will trigger the parsing of the query string, POSTed form data where
+ * the request body has content type is <code>application/x-www-form-urlencoded</code> and POSTed form data where
+ * the request body has content-type <code>multipart/form-data</code> and the Servlet is configured with a
+ * {@link jakarta.servlet.annotation.MultipartConfig} annotation or a <code>multipart-config</code> element in the
+ * deployment descriptor.
*
* @param name a <code>String</code> containing the name of the parameter whose value is requested
*
* @return an array of <code>String</code> objects containing the parameter's values
*
+ * @throws IllegalStateException if parameter parsing is triggered and a problem is encountered parsing the
+ * parameters including, but not limited to: invalid percent (%nn) encoding;
+ * invalid byte sequence for the specified character set; I/O errors reading the
+ * request body; and triggering a container defined limit related to parameter
+ * parsing. Containers may provide container specific options to handle some or
+ * all of these errors in an alternative manner that may include not throwing an
+ * exception.
+ *
* @see #getParameter
*/
String[] getParameterValues(String name);
@@ -198,10 +240,24 @@ public interface ServletRequest {
/**
* Returns a java.util.Map of the parameters of this request. Request parameters are extra information sent with the
* request. For HTTP servlets, parameters are contained in the query string or posted form data.
+ * <p>
+ * If not already parsed, calling this method will trigger the parsing of the query string, POSTed form data where
+ * the request body has content type is <code>application/x-www-form-urlencoded</code> and POSTed form data where
+ * the request body has content-type <code>multipart/form-data</code> and the Servlet is configured with a
+ * {@link jakarta.servlet.annotation.MultipartConfig} annotation or a <code>multipart-config</code> element in the
+ * deployment descriptor.
*
* @return an immutable java.util.Map containing parameter names as keys and parameter values as map values. The
* keys in the parameter map are of type String. The values in the parameter map are of type String
* array.
+ *
+ * @throws IllegalStateException if parameter parsing is triggered and a problem is encountered parsing the
+ * parameters including, but not limited to: invalid percent (%nn) encoding;
+ * invalid byte sequence for the specified character set; I/O errors reading the
+ * request body; and triggering a container defined limit related to parameter
+ * parsing. Containers may provide container specific options to handle some or
+ * all of these errors in an alternative manner that may include not throwing an
+ * exception.
*/
Map<String,String[]> getParameterMap();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/04: First pass at implementing changes for parameter parsing
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch tmp
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 61009deff1cf542b6c555121e6af4a6baf7851b6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jun 15 12:15:46 2023 +0100
First pass at implementing changes for parameter parsing
---
java/org/apache/catalina/connector/Request.java | 31 +--
.../apache/catalina/core/StandardHostValve.java | 3 +-
.../apache/catalina/core/StandardWrapperValve.java | 18 +-
.../util/http/InvalidParameterException.java | 56 +++++
.../tomcat/util/http/LocalStrings.properties | 7 +-
.../tomcat/util/http/ParameterErrorHandling.java | 100 +++++++++
java/org/apache/tomcat/util/http/Parameters.java | 241 ++++++++++-----------
7 files changed, 313 insertions(+), 143 deletions(-)
diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java
index 312b3f4e81..c0afb7553c 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -103,7 +103,6 @@ import org.apache.tomcat.util.buf.UDecoder;
import org.apache.tomcat.util.http.CookieProcessor;
import org.apache.tomcat.util.http.FastHttpDateFormat;
import org.apache.tomcat.util.http.Parameters;
-import org.apache.tomcat.util.http.Parameters.FailReason;
import org.apache.tomcat.util.http.Rfc6265CookieProcessor;
import org.apache.tomcat.util.http.ServerCookie;
import org.apache.tomcat.util.http.ServerCookies;
@@ -2723,7 +2722,7 @@ public class Request implements HttpServletRequest {
}
if (!location.isDirectory()) {
- parameters.setParseFailedReason(FailReason.MULTIPART_CONFIG_INVALID);
+ // TODO parameters.setParseFailedReason(FailReason.MULTIPART_CONFIG_INVALID);
partsParseException = new IOException(sm.getString("coyoteRequest.uploadLocationInvalid", location));
return;
}
@@ -2734,7 +2733,7 @@ public class Request implements HttpServletRequest {
try {
factory.setRepository(location.getCanonicalFile());
} catch (IOException ioe) {
- parameters.setParseFailedReason(FailReason.IO_ERROR);
+ // TODO parameters.setParseFailedReason(FailReason.IO_ERROR);
partsParseException = ioe;
return;
}
@@ -2774,7 +2773,7 @@ public class Request implements HttpServletRequest {
// Value separator
postSize++;
if (postSize > maxPostSize) {
- parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
+ // TODO parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
throw new IllegalStateException(sm.getString("coyoteRequest.maxPostSizeExceeded"));
}
}
@@ -2790,14 +2789,14 @@ public class Request implements HttpServletRequest {
success = true;
} catch (InvalidContentTypeException e) {
- parameters.setParseFailedReason(FailReason.INVALID_CONTENT_TYPE);
+ // TODO parameters.setParseFailedReason(FailReason.INVALID_CONTENT_TYPE);
partsParseException = new ServletException(e);
} catch (SizeException e) {
- parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
+ // TODO parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
checkSwallowInput();
partsParseException = new IllegalStateException(e);
} catch (IOException e) {
- parameters.setParseFailedReason(FailReason.IO_ERROR);
+ // TODO parameters.setParseFailedReason(FailReason.IO_ERROR);
partsParseException = new IOException(e);
} catch (IllegalStateException e) {
// addParameters() will set parseFailedReason
@@ -2810,7 +2809,7 @@ public class Request implements HttpServletRequest {
// be more efficient but it is written this way to be robust with
// respect to changes in the remainder of the method.
if (partsParseException != null || !success) {
- parameters.setParseFailedReason(FailReason.UNKNOWN);
+ // TODO parameters.setParseFailedReason(FailReason.UNKNOWN);
}
}
}
@@ -3102,7 +3101,7 @@ public class Request implements HttpServletRequest {
context.getLogger().debug(sm.getString("coyoteRequest.postTooLarge"));
}
checkSwallowInput();
- parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
+ // TODO parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
return;
}
byte[] formData = null;
@@ -3116,7 +3115,7 @@ public class Request implements HttpServletRequest {
}
try {
if (readPostBody(formData, len) != len) {
- parameters.setParseFailedReason(FailReason.REQUEST_BODY_INCOMPLETE);
+ // TODO parameters.setParseFailedReason(FailReason.REQUEST_BODY_INCOMPLETE);
return;
}
} catch (IOException e) {
@@ -3125,7 +3124,7 @@ public class Request implements HttpServletRequest {
if (context != null && context.getLogger().isDebugEnabled()) {
context.getLogger().debug(sm.getString("coyoteRequest.parseParameters"), e);
}
- parameters.setParseFailedReason(FailReason.CLIENT_DISCONNECT);
+ // TODO parameters.setParseFailedReason(FailReason.CLIENT_DISCONNECT);
return;
}
parameters.processParameters(formData, 0, len);
@@ -3135,7 +3134,7 @@ public class Request implements HttpServletRequest {
formData = readChunkedPostBody();
} catch (IllegalStateException ise) {
// chunkedPostTooLarge error
- parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
+ // TODO parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
Context context = getContext();
if (context != null && context.getLogger().isDebugEnabled()) {
context.getLogger().debug(sm.getString("coyoteRequest.parseParameters"), ise);
@@ -3143,7 +3142,7 @@ public class Request implements HttpServletRequest {
return;
} catch (IOException e) {
// Client disconnect
- parameters.setParseFailedReason(FailReason.CLIENT_DISCONNECT);
+ // TODO parameters.setParseFailedReason(FailReason.CLIENT_DISCONNECT);
Context context = getContext();
if (context != null && context.getLogger().isDebugEnabled()) {
context.getLogger().debug(sm.getString("coyoteRequest.parseParameters"), e);
@@ -3157,7 +3156,7 @@ public class Request implements HttpServletRequest {
success = true;
} finally {
if (!success) {
- parameters.setParseFailedReason(FailReason.UNKNOWN);
+ // TODO parameters.setParseFailedReason(FailReason.UNKNOWN);
}
}
@@ -3347,6 +3346,7 @@ public class Request implements HttpServletRequest {
// NO-OP
}
});
+ /*
specialAttributes.put(Globals.PARAMETER_PARSE_FAILED_ATTR, new SpecialAttributeAdapter() {
@Override
public Object get(Request request, String name) {
@@ -3361,6 +3361,8 @@ public class Request implements HttpServletRequest {
// NO-OP
}
});
+ */
+ /*
specialAttributes.put(Globals.PARAMETER_PARSE_FAILED_REASON_ATTR, new SpecialAttributeAdapter() {
@Override
public Object get(Request request, String name) {
@@ -3372,6 +3374,7 @@ public class Request implements HttpServletRequest {
// 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/core/StandardHostValve.java b/java/org/apache/catalina/core/StandardHostValve.java
index 9814a3cc28..98b8e92a2d 100644
--- a/java/org/apache/catalina/core/StandardHostValve.java
+++ b/java/org/apache/catalina/core/StandardHostValve.java
@@ -37,6 +37,7 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.descriptor.web.ErrorPage;
+import org.apache.tomcat.util.http.InvalidParameterException;
import org.apache.tomcat.util.res.StringManager;
/**
@@ -294,7 +295,7 @@ final class StandardHostValve extends ValveBase {
}
}
}
- } else {
+ } else if (!(throwable instanceof InvalidParameterException)){
// A custom error-page has not been defined for the exception
// that was thrown during request processing. Check if an
// error-page for error code 500 was specified and if so,
diff --git a/java/org/apache/catalina/core/StandardWrapperValve.java b/java/org/apache/catalina/core/StandardWrapperValve.java
index 5426004930..4fd4279125 100644
--- a/java/org/apache/catalina/core/StandardWrapperValve.java
+++ b/java/org/apache/catalina/core/StandardWrapperValve.java
@@ -38,6 +38,7 @@ import org.apache.catalina.valves.ValveBase;
import org.apache.coyote.CloseNowException;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.MessageBytes;
+import org.apache.tomcat.util.http.InvalidParameterException;
import org.apache.tomcat.util.log.SystemLogHandler;
import org.apache.tomcat.util.res.StringManager;
@@ -168,6 +169,8 @@ final class StandardWrapperValve extends ValveBase {
}
}
+ } catch (InvalidParameterException e) {
+ exception(request, response, e, HttpServletResponse.SC_BAD_REQUEST);
} catch (ClientAbortException | CloseNowException e) {
if (container.getLogger().isDebugEnabled()) {
container.getLogger().debug(
@@ -271,8 +274,21 @@ final class StandardWrapperValve extends ValveBase {
* @param exception The exception that occurred (which possibly wraps a root cause exception
*/
private void exception(Request request, Response response, Throwable exception) {
+ exception(request, response, exception, HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ }
+
+ /**
+ * Handle the specified ServletException encountered while processing the specified Request to produce the specified
+ * Response. Any exceptions that occur during generation of the exception report are logged and swallowed.
+ *
+ * @param request The request being processed
+ * @param response The response being generated
+ * @param exception The exception that occurred (which possibly wraps a root cause exception
+ * @param status The HTTP status code to use for the response
+ */
+ private void exception(Request request, Response response, Throwable exception, int status) {
request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, exception);
- response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ response.setStatus(status);
response.setError();
}
diff --git a/java/org/apache/tomcat/util/http/InvalidParameterException.java b/java/org/apache/tomcat/util/http/InvalidParameterException.java
new file mode 100644
index 0000000000..95c5806be6
--- /dev/null
+++ b/java/org/apache/tomcat/util/http/InvalidParameterException.java
@@ -0,0 +1,56 @@
+/*
+ * 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.tomcat.util.http;
+
+/**
+ * Extend {@link IllegalStateException} to identify the cause as an invalid parameter.
+ */
+public class InvalidParameterException extends IllegalStateException {
+
+ private static final long serialVersionUID = 1L;
+
+
+ public InvalidParameterException() {
+ super();
+ }
+
+
+ public InvalidParameterException(String message, Throwable cause) {
+ super(message, cause);
+ }
+
+
+ public InvalidParameterException(String s) {
+ super(s);
+ }
+
+
+ public InvalidParameterException(Throwable cause) {
+ super(cause);
+ }
+
+ @Override
+ public synchronized Throwable fillInStackTrace() {
+ /*
+ * This exception is triggered by user input and therefore, since generating stack traces is relatively
+ * expensive, stack traces have been disabled for this class. There should be enough information in the
+ * exception message to identify the problematic parameter. If not, it is very likely that fixing the message is
+ * a better fix than enabling stack traces.
+ */
+ return this;
+ }
+}
diff --git a/java/org/apache/tomcat/util/http/LocalStrings.properties b/java/org/apache/tomcat/util/http/LocalStrings.properties
index 43307a8893..368adf136a 100644
--- a/java/org/apache/tomcat/util/http/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/http/LocalStrings.properties
@@ -24,17 +24,16 @@ headers.maxCountFail=More than the maximum allowed number of headers, [{0}], wer
parameters.bytes=Start processing with input [{0}]
parameters.copyFail=Failed to create copy of original parameter values for debug logging purposes
-parameters.decodeFail.debug=Character decoding failed. Parameter [{0}] with value [{1}] has been ignored.
-parameters.decodeFail.info=Character decoding failed. Parameter [{0}] with value [{1}] has been ignored. Note that the name and value quoted here may be corrupted due to the failed decoding. Use debug level logging to see the original, non-corrupted values.
+parameters.corrupted=Note that the name and/or value quoted here may be corrupted due to the failed decoding. Use debug level logging to see the original, non-corrupted values.
+parameters.decodeFail=Character decoding failed for parameter [{0}] with value [{1}].
parameters.emptyChunk=Empty parameter chunk ignored
parameters.fallToDebug=\n\
\ Note: further occurrences of Parameter errors will be logged at DEBUG level.
parameters.invalidChunk=Invalid chunk starting at byte [{0}] and ending at byte [{1}] with a value of [{2}] ignored
parameters.maxCountFail=More than the maximum number of request parameters (GET plus POST) for a single request ([{0}]) were detected. Any parameters beyond this limit have been ignored. To change this limit, set the maxParameterCount attribute on the Connector.
-parameters.maxCountFail.fallToDebug=\n\
-\ Note: further occurrences of this error will be logged at DEBUG level.
parameters.multipleDecodingFail=Character decoding failed. A total of [{0}] failures were detected but only the first was logged. Enable debug level logging for this logger to log all failures.
parameters.noequal=Parameter starting at position [{0}] and ending at position [{1}] with a value of [{2}] was not followed by an ''='' character
+parameters.urlDecodeFail=URL (%nn) decoding failed for parameter [{0}] with value [{1}].
rfc6265CookieProcessor.invalidAttributeName=An invalid attribute name [{0}] was specified for this cookie
rfc6265CookieProcessor.invalidAttributeValue=An invalid attribute value [{1}] was specified for this cookie attribute [{0}]
diff --git a/java/org/apache/tomcat/util/http/ParameterErrorHandling.java b/java/org/apache/tomcat/util/http/ParameterErrorHandling.java
new file mode 100644
index 0000000000..3d139cfe6b
--- /dev/null
+++ b/java/org/apache/tomcat/util/http/ParameterErrorHandling.java
@@ -0,0 +1,100 @@
+/*
+ * 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.tomcat.util.http;
+
+import java.nio.charset.CodingErrorAction;
+
+public class ParameterErrorHandling {
+
+ private boolean skipEmptyParameter = false;
+ private boolean skipInvalidParameter = false;
+ private boolean skipUrlDecodingError = false;
+ private CodingErrorAction malformedInputAction = CodingErrorAction.REPORT;
+ private CodingErrorAction unmappableCharacterAction = CodingErrorAction.REPORT;
+ private boolean skipDecodingError = false;
+ private boolean skipMaxParameterCountError = false;
+
+
+ public boolean getSkipEmptyParameter() {
+ return skipEmptyParameter;
+ }
+
+
+ public void setSkipEmptyParameter(boolean skipEmptyParameter) {
+ this.skipEmptyParameter = skipEmptyParameter;
+ }
+
+
+ public boolean getSkipInvalidParameter() {
+ return skipInvalidParameter;
+ }
+
+
+ public void setSkipInvalidParameter(boolean skipInvalidParameter) {
+ this.skipInvalidParameter = skipInvalidParameter;
+ }
+
+
+ public boolean getSkipUrlDecodingError() {
+ return skipUrlDecodingError;
+ }
+
+
+ public void setIgnoreUrlDecodingError(boolean skipUrlDecodingError) {
+ this.skipUrlDecodingError = skipUrlDecodingError;
+ }
+
+
+ public CodingErrorAction malformedInputAction() {
+ return malformedInputAction;
+ }
+
+
+ public void onMalformedInput(CodingErrorAction action) {
+ this.malformedInputAction = action;
+ }
+
+
+ public CodingErrorAction unmappableCharacterAction() {
+ return unmappableCharacterAction;
+ }
+
+
+ public void onUnmappableCharacter(CodingErrorAction action) {
+ this.unmappableCharacterAction = action;
+ }
+
+
+ public boolean getSkipDecodingError() {
+ return skipDecodingError;
+ }
+
+
+ public void setSkipDecodingError(boolean skipDecodingError) {
+ this.skipDecodingError = skipDecodingError;
+ }
+
+
+ public boolean getSkipMaxParameterCountError() {
+ return skipMaxParameterCountError;
+ }
+
+
+ public void setSkipMaxParameterCountError(boolean skipMaxParameterCountError) {
+ this.skipMaxParameterCountError = skipMaxParameterCountError;
+ }
+}
diff --git a/java/org/apache/tomcat/util/http/Parameters.java b/java/org/apache/tomcat/util/http/Parameters.java
index e56793b3de..bb12f771af 100644
--- a/java/org/apache/tomcat/util/http/Parameters.java
+++ b/java/org/apache/tomcat/util/http/Parameters.java
@@ -17,6 +17,7 @@
package org.apache.tomcat.util.http;
import java.io.IOException;
+import java.nio.charset.CharacterCodingException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
@@ -24,6 +25,7 @@ import java.util.Collections;
import java.util.Enumeration;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.function.BooleanSupplier;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
@@ -38,7 +40,7 @@ public final class Parameters {
private static final Log log = LogFactory.getLog(Parameters.class);
- private static final UserDataHelper userDataLog = new UserDataHelper(log);
+ private static final UserDataHelper paramParsingLog = new UserDataHelper(log);
private static final UserDataHelper maxParamCountLog = new UserDataHelper(log);
@@ -58,11 +60,8 @@ 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;
+ private final ParameterErrorHandling errorHandler = new ParameterErrorHandling();
+ private int parseFailureCount = 0;
public Parameters() {
// NO-OP
@@ -102,23 +101,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;
}
@@ -130,7 +112,7 @@ public final class Parameters {
didQueryParameters = false;
charset = DEFAULT_BODY_CHARSET;
decodedQuery.recycle();
- parseFailedReason = null;
+ parseFailureCount = 0;
}
@@ -196,7 +178,6 @@ public final class Parameters {
public void addParameter(String key, String value) throws IllegalStateException {
-
if (key == null) {
return;
}
@@ -204,18 +185,20 @@ public final class Parameters {
if (limit > -1 && parameterCount >= limit) {
// Processing this parameter will push us over the limit. ISE is
// what Request.parseParts() uses for requests that are too big
- setParseFailedReason(FailReason.TOO_MANY_PARAMETERS);
- throw new IllegalStateException(sm.getString("parameters.maxCountFail", Integer.valueOf(limit)));
+ String msg = sm.getString("parameters.maxCountFail", Integer.valueOf(limit));
+ handleParameterProcessingError(msg, maxParamCountLog, () -> errorHandler.getSkipMaxParameterCountError(),
+ null);
}
parameterCount++;
-
paramHashValues.computeIfAbsent(key, k -> new ArrayList<>(1)).add(value);
}
+
public void setURLDecoder(UDecoder u) {
urlDec = u;
}
+
// -------------------- Parameter parsing --------------------
// we are called from a single thread - we can do it the hard way
// if needed
@@ -237,8 +220,6 @@ public final class Parameters {
log.debug(sm.getString("parameters.bytes", new String(bytes, start, len, DEFAULT_BODY_CHARSET)));
}
- int decodeFailCount = 0;
-
int pos = start;
int end = start + len;
@@ -309,37 +290,21 @@ public final class Parameters {
if (nameEnd <= nameStart) {
if (valueStart == -1) {
// &&
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("parameters.emptyChunk"));
- }
- // Do not flag as error
- continue;
+ String msg = sm.getString("parameters.emptyChunk");
+ handleParameterProcessingError(msg, paramParsingLog, () -> errorHandler.getSkipEmptyParameter(),
+ null);
}
// &=foo&
- UserDataHelper.Mode logMode = userDataLog.getNextMode();
- if (logMode != null) {
- String extract;
- if (valueEnd > nameStart) {
- extract = new String(bytes, nameStart, valueEnd - nameStart, DEFAULT_BODY_CHARSET);
- } else {
- extract = "";
- }
- String message = sm.getString("parameters.invalidChunk", Integer.valueOf(nameStart),
- Integer.valueOf(valueEnd), extract);
- switch (logMode) {
- case INFO_THEN_DEBUG:
- message += sm.getString("parameters.fallToDebug");
- //$FALL-THROUGH$
- case INFO:
- log.info(message);
- break;
- case DEBUG:
- log.debug(message);
- }
+ String extract;
+ if (valueEnd > nameStart) {
+ extract = new String(bytes, nameStart, valueEnd - nameStart, DEFAULT_BODY_CHARSET);
+ } else {
+ extract = "";
}
- setParseFailedReason(FailReason.NO_NAME);
- continue;
- // invalid chunk - it's better to ignore
+ String msg = sm.getString("parameters.invalidChunk", Integer.valueOf(nameStart),
+ Integer.valueOf(valueEnd), extract);
+ handleParameterProcessingError(msg, paramParsingLog, () -> errorHandler.getSkipInvalidParameter(),
+ null);
}
tmpName.setBytes(bytes, nameStart, nameEnd - nameStart);
@@ -366,88 +331,73 @@ public final class Parameters {
}
}
- try {
- String name;
- String value;
+ String name = null;
+ String value = null;
+ try {
if (decodeName) {
- urlDecode(tmpName);
+ try {
+ urlDecode(tmpName);
+ } catch (IOException e) {
+ // Invalid %nn sequence
+ String msg = getParameterMessage("parameters.urlDecodeFail");
+ handleParameterProcessingError(msg, paramParsingLog,
+ () -> errorHandler.getSkipUrlDecodingError(), null);
+ }
}
+
tmpName.setCharset(charset);
- name = tmpName.toString();
+ try {
+ name = tmpName.toString(errorHandler.malformedInputAction(),
+ errorHandler.unmappableCharacterAction());
+ } catch (CharacterCodingException e) {
+ // Invalid byte sequence for character set
+ String msg = getParameterMessage("parameters.decodeFail");
+ handleParameterProcessingError(msg, paramParsingLog, () -> errorHandler.getSkipDecodingError(),
+ null);
+ }
if (valueStart >= 0) {
if (decodeValue) {
- urlDecode(tmpValue);
+ try {
+ urlDecode(tmpValue);
+ } catch (IOException e) {
+ // Invalid %nn sequence
+ String msg = getParameterMessage("parameters.urlDecodeFail");
+ handleParameterProcessingError(msg, paramParsingLog,
+ () -> errorHandler.getSkipUrlDecodingError(), null);
+ }
}
tmpValue.setCharset(charset);
- value = tmpValue.toString();
+ try {
+ value = tmpValue.toString(errorHandler.malformedInputAction(),
+ errorHandler.unmappableCharacterAction());
+ } catch (CharacterCodingException e) {
+ // Invalid byte sequence for character set
+ String msg = getParameterMessage("parameters.decodeFail");
+ handleParameterProcessingError(msg, paramParsingLog, () -> errorHandler.getSkipDecodingError(),
+ null);
+ }
} else {
value = "";
}
- try {
- addParameter(name, value);
- } catch (IllegalStateException ise) {
- // Hitting limit stops processing further params but does
- // not cause request to fail.
- UserDataHelper.Mode logMode = maxParamCountLog.getNextMode();
- if (logMode != null) {
- String message = ise.getMessage();
- switch (logMode) {
- case INFO_THEN_DEBUG:
- message += sm.getString("parameters.maxCountFail.fallToDebug");
- //$FALL-THROUGH$
- case INFO:
- log.info(message);
- break;
- case DEBUG:
- log.debug(message);
- }
- }
- break;
- }
- } catch (IOException e) {
- setParseFailedReason(FailReason.URL_DECODING);
- decodeFailCount++;
- if (decodeFailCount == 1 || log.isDebugEnabled()) {
- if (log.isDebugEnabled()) {
- log.debug(
- sm.getString("parameters.decodeFail.debug", origName.toString(), origValue.toString()),
- e);
- } else if (log.isInfoEnabled()) {
- UserDataHelper.Mode logMode = userDataLog.getNextMode();
- if (logMode != null) {
- String message =
- sm.getString("parameters.decodeFail.info", tmpName.toString(), tmpValue.toString());
- switch (logMode) {
- case INFO_THEN_DEBUG:
- message += sm.getString("parameters.fallToDebug");
- //$FALL-THROUGH$
- case INFO:
- log.info(message);
- break;
- case DEBUG:
- log.debug(message);
- }
- }
- }
+ addParameter(name, value);
+ } finally {
+ tmpName.recycle();
+ tmpValue.recycle();
+ // Only recycle copies if we used them
+ if (log.isDebugEnabled()) {
+ origName.recycle();
+ origValue.recycle();
}
}
-
- tmpName.recycle();
- tmpValue.recycle();
- // Only recycle copies if we used them
- if (log.isDebugEnabled()) {
- origName.recycle();
- origValue.recycle();
- }
}
- if (decodeFailCount > 1 && !log.isDebugEnabled()) {
- UserDataHelper.Mode logMode = userDataLog.getNextMode();
+ if (parseFailureCount > 1 && !log.isDebugEnabled()) {
+ UserDataHelper.Mode logMode = paramParsingLog.getNextMode();
if (logMode != null) {
- String message = sm.getString("parameters.multipleDecodingFail", Integer.valueOf(decodeFailCount));
+ String message = sm.getString("parameters.multipleDecodingFail", Integer.valueOf(parseFailureCount));
switch (logMode) {
case INFO_THEN_DEBUG:
message += sm.getString("parameters.fallToDebug");
@@ -456,12 +406,57 @@ public final class Parameters {
log.info(message);
break;
case DEBUG:
- log.debug(message);
+ // NO-OP: If debug is enabled all failures will have been logged.
}
}
}
}
+
+ private String getParameterMessage(String messageKey) {
+ // Note: The conversions here won't fail because toString() always uses CodingErrorAction.REPLACE
+ if (log.isDebugEnabled()) {
+ return sm.getString(messageKey, origName.toString(), origValue.toString());
+ } else {
+ return sm.getString(messageKey, tmpName.toString(), tmpValue.toString()) + " " +
+ sm.getString("parameters.corrupted");
+ }
+ }
+
+
+ private void handleParameterProcessingError(String message, UserDataHelper userDataHelper,
+ BooleanSupplier skipError, Throwable cause) {
+ parseFailureCount++;
+ if (log.isDebugEnabled()) {
+ log.debug(message);
+ } else {
+ if (parseFailureCount == 1) {
+ UserDataHelper.Mode logMode = userDataHelper.getNextMode();
+ if (logMode != null) {
+ switch (logMode) {
+ case INFO_THEN_DEBUG:
+ log.info(message + sm.getString("parameters.fallToDebug"));
+ break;
+ case INFO:
+ log.info(message);
+ break;
+ case DEBUG:
+ // NO-OP: If debug is enabled the message will be logged above
+ }
+ }
+ }
+ }
+ if (skipError.getAsBoolean()) {
+ return;
+ }
+ if (cause == null) {
+ throw new InvalidParameterException(message);
+ } else {
+ throw new InvalidParameterException(message, cause);
+ }
+ }
+
+
private void urlDecode(ByteChunk bc) throws IOException {
if (urlDec == null) {
urlDec = new UDecoder();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/04: Add control of byte decoding errors to ByteChunk and StringCache
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch tmp
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 944951302e2f478879411dbff353f5818ad44121
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jun 14 12:25:21 2023 +0100
Add control of byte decoding errors to ByteChunk and StringCache
---
java/org/apache/tomcat/util/buf/ByteChunk.java | 47 +++++++++-
java/org/apache/tomcat/util/buf/StringCache.java | 66 +++++++++++---
test/org/apache/jasper/compiler/TestGenerator.java | 3 +-
.../apache/tomcat/util/buf/TestStringCache.java | 100 +++++++++++++++++++++
4 files changed, 203 insertions(+), 13 deletions(-)
diff --git a/java/org/apache/tomcat/util/buf/ByteChunk.java b/java/org/apache/tomcat/util/buf/ByteChunk.java
index 101f9c0eaa..ff00f55774 100644
--- a/java/org/apache/tomcat/util/buf/ByteChunk.java
+++ b/java/org/apache/tomcat/util/buf/ByteChunk.java
@@ -21,7 +21,9 @@ import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
+import java.nio.charset.CharacterCodingException;
import java.nio.charset.Charset;
+import java.nio.charset.CodingErrorAction;
import java.nio.charset.StandardCharsets;
/*
@@ -521,23 +523,64 @@ public final class ByteChunk extends AbstractChunk {
@Override
public String toString() {
+ try {
+ return toString(CodingErrorAction.REPLACE, CodingErrorAction.REPLACE);
+ } catch (CharacterCodingException e) {
+ // Unreachable code. Use of REPLACE above means the exception will never be thrown.
+ throw new IllegalStateException(e);
+ }
+ }
+
+
+ public String toString(CodingErrorAction malformedInputAction, CodingErrorAction unmappableCharacterAction)
+ throws CharacterCodingException {
if (isNull()) {
return null;
} else if (end - start == 0) {
return "";
}
- return StringCache.toString(this);
+ return StringCache.toString(this, malformedInputAction, unmappableCharacterAction);
}
+ /**
+ * Converts the current content of the byte buffer to a String using the configured character set.
+ *
+ * @return The result of converting the bytes to a String
+ *
+ * @deprecated Unused. This method will be removed in Tomcat 11 onwards.
+ */
+ @Deprecated
public String toStringInternal() {
+ try {
+ return toStringInternal(CodingErrorAction.REPLACE, CodingErrorAction.REPLACE);
+ } catch (CharacterCodingException e) {
+ // Unreachable code. Use of REPLACE above means the exception will never be thrown.
+ throw new IllegalStateException(e);
+ }
+ }
+
+
+ /**
+ * Converts the current content of the byte buffer to a String using the configured character set.
+ *
+ * @param malformedInputAction Action to take if the input is malformed
+ * @param unmappableCharacterAction Action to take if a byte sequence can't be mapped to a character
+ *
+ * @return The result of converting the bytes to a String
+ *
+ * @throws CharacterCodingException If an error occurs during the conversion
+ */
+ public String toStringInternal(CodingErrorAction malformedInputAction, CodingErrorAction unmappableCharacterAction)
+ throws CharacterCodingException {
if (charset == null) {
charset = DEFAULT_CHARSET;
}
// new String(byte[], int, int, Charset) takes a defensive copy of the
// entire byte array. This is expensive if only a small subset of the
// bytes will be used. The code below is from Apache Harmony.
- CharBuffer cb = charset.decode(ByteBuffer.wrap(buff, start, end - start));
+ CharBuffer cb = charset.newDecoder().onMalformedInput(malformedInputAction)
+ .onUnmappableCharacter(unmappableCharacterAction).decode(ByteBuffer.wrap(buff, start, end - start));
return new String(cb.array(), cb.arrayOffset(), cb.length());
}
diff --git a/java/org/apache/tomcat/util/buf/StringCache.java b/java/org/apache/tomcat/util/buf/StringCache.java
index d39de93cfa..5b82e44f74 100644
--- a/java/org/apache/tomcat/util/buf/StringCache.java
+++ b/java/org/apache/tomcat/util/buf/StringCache.java
@@ -16,10 +16,13 @@
*/
package org.apache.tomcat.util.buf;
+import java.nio.charset.CharacterCodingException;
import java.nio.charset.Charset;
+import java.nio.charset.CodingErrorAction;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map.Entry;
+import java.util.Objects;
import java.util.TreeMap;
import org.apache.juli.logging.Log;
@@ -208,11 +211,22 @@ public class StringCache {
public static String toString(ByteChunk bc) {
+ try {
+ return toString(bc, CodingErrorAction.REPLACE, CodingErrorAction.REPLACE);
+ } catch (CharacterCodingException e) {
+ // Unreachable code. Use of REPLACE above means the exception will never be thrown.
+ throw new IllegalStateException(e);
+ }
+ }
+
+
+ public static String toString(ByteChunk bc, CodingErrorAction malformedInputAction,
+ CodingErrorAction unmappableCharacterAction) throws CharacterCodingException {
// If the cache is null, then either caching is disabled, or we're
// still training
if (bcCache == null) {
- String value = bc.toStringInternal();
+ String value = bc.toStringInternal(malformedInputAction, unmappableCharacterAction);
if (byteEnabled && (value.length() < maxStringSize)) {
// If training, everything is synced
synchronized (bcStats) {
@@ -285,6 +299,8 @@ public class StringCache {
System.arraycopy(bc.getBuffer(), start, entry.name, 0, end - start);
// Set encoding
entry.charset = bc.getCharset();
+ entry.malformedInputAction = malformedInputAction;
+ entry.unmappableCharacterAction = unmappableCharacterAction;
// Initialize occurrence count to one
count = new int[1];
count[0] = 1;
@@ -300,9 +316,9 @@ public class StringCache {
} else {
accessCount++;
// Find the corresponding String
- String result = find(bc);
+ String result = find(bc, malformedInputAction, unmappableCharacterAction);
if (result == null) {
- return bc.toStringInternal();
+ return bc.toStringInternal(malformedInputAction, unmappableCharacterAction);
}
// Note: We don't care about safety for the stats
hitCount++;
@@ -462,10 +478,31 @@ public class StringCache {
* @param name The name to find
*
* @return the corresponding value
+ *
+ * @deprecated Unused. Will be removed in Tomcat 11.
+ * Use {@link #find(ByteChunk, CodingErrorAction, CodingErrorAction)}
*/
+ @Deprecated
protected static final String find(ByteChunk name) {
+ return find(name, CodingErrorAction.REPLACE, CodingErrorAction.REPLACE);
+ }
+
+
+ /**
+ * Find an entry given its name in the cache and return the associated String.
+ *
+ * @param name The name to find
+ * @param malformedInputAction Action to take if an malformed input is encountered
+ * @param unmappableCharacterAction Action to take if an unmappable character is encountered
+ *
+ * @return the corresponding value
+ */
+ protected static final String find(ByteChunk name, CodingErrorAction malformedInputAction,
+ CodingErrorAction unmappableCharacterAction) {
int pos = findClosest(name, bcCache, bcCache.length);
- if ((pos < 0) || (compare(name, bcCache[pos].name) != 0) || !(name.getCharset().equals(bcCache[pos].charset))) {
+ if ((pos < 0) || (compare(name, bcCache[pos].name) != 0) || !(name.getCharset().equals(bcCache[pos].charset)) ||
+ !malformedInputAction.equals(bcCache[pos].malformedInputAction) ||
+ !unmappableCharacterAction.equals(bcCache[pos].unmappableCharacterAction)) {
return null;
} else {
return bcCache[pos].value;
@@ -631,11 +668,12 @@ public class StringCache {
// -------------------------------------------------- ByteEntry Inner Class
-
private static class ByteEntry {
private byte[] name = null;
private Charset charset = null;
+ private CodingErrorAction malformedInputAction = null;
+ private CodingErrorAction unmappableCharacterAction = null;
private String value = null;
@Override
@@ -645,17 +683,25 @@ public class StringCache {
@Override
public int hashCode() {
- return value.hashCode();
+ return Objects.hash(malformedInputAction, unmappableCharacterAction, value);
}
@Override
public boolean equals(Object obj) {
- if (obj instanceof ByteEntry) {
- return value.equals(((ByteEntry) obj).value);
+ if (this == obj) {
+ return true;
}
- return false;
+ if (obj == null) {
+ return false;
+ }
+ if (getClass() != obj.getClass()) {
+ return false;
+ }
+ ByteEntry other = (ByteEntry) obj;
+ return Objects.equals(malformedInputAction, other.malformedInputAction) &&
+ Objects.equals(unmappableCharacterAction, other.unmappableCharacterAction) &&
+ Objects.equals(value, other.value);
}
-
}
diff --git a/test/org/apache/jasper/compiler/TestGenerator.java b/test/org/apache/jasper/compiler/TestGenerator.java
index 8cfe73f54c..b854a6010b 100644
--- a/test/org/apache/jasper/compiler/TestGenerator.java
+++ b/test/org/apache/jasper/compiler/TestGenerator.java
@@ -22,6 +22,7 @@ import java.beans.PropertyDescriptor;
import java.beans.PropertyEditorSupport;
import java.io.File;
import java.io.IOException;
+import java.nio.charset.CodingErrorAction;
import java.util.Date;
import java.util.Scanner;
@@ -211,7 +212,7 @@ public class TestGenerator extends TomcatBaseTest {
ByteChunk bc = new ByteChunk();
int rc = getUrl("http://localhost:" + getPort() + "/test/bug5nnnn/bug56529.jsp", bc, null);
Assert.assertEquals(HttpServletResponse.SC_OK, rc);
- String response = bc.toStringInternal();
+ String response = bc.toStringInternal(CodingErrorAction.REPORT, CodingErrorAction.REPORT);
Assert.assertTrue(response, response.contains("[1:attribute1: '', attribute2: '']"));
Assert.assertTrue(response, response.contains("[2:attribute1: '', attribute2: '']"));
}
diff --git a/test/org/apache/tomcat/util/buf/TestStringCache.java b/test/org/apache/tomcat/util/buf/TestStringCache.java
new file mode 100644
index 0000000000..2345d2de76
--- /dev/null
+++ b/test/org/apache/tomcat/util/buf/TestStringCache.java
@@ -0,0 +1,100 @@
+/*
+ * 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.tomcat.util.buf;
+
+import java.nio.charset.CharacterCodingException;
+import java.nio.charset.CodingErrorAction;
+import java.nio.charset.StandardCharsets;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStringCache {
+
+ private static final byte[] BYTES_VALID = new byte[] { 65, 66, 67, 68};
+ private static final byte[] BYTES_INVALID = new byte[] {65, 66, -1, 67, 68};
+
+ private static final ByteChunk INPUT_VALID = new ByteChunk();
+ private static final ByteChunk INPUT_INVALID = new ByteChunk();
+
+ private static final CodingErrorAction[] actions =
+ new CodingErrorAction[] { CodingErrorAction.IGNORE, CodingErrorAction.REPLACE, CodingErrorAction.REPORT };
+
+ static {
+ INPUT_VALID.setBytes(BYTES_VALID, 0, BYTES_VALID.length);
+ INPUT_VALID.setCharset(StandardCharsets.UTF_8);
+ INPUT_INVALID.setBytes(BYTES_INVALID, 0, BYTES_INVALID.length);
+ INPUT_INVALID.setCharset(StandardCharsets.UTF_8);
+ }
+
+
+ @Test
+ public void testCodingErrorLookup() {
+
+ System.setProperty("tomcat.util.buf.StringCache.byte.enabled", "true");
+
+ Assert.assertTrue(StringCache.byteEnabled);
+ StringCache sc = new StringCache();
+ sc.reset();
+
+ for (int i = 0; i < StringCache.trainThreshold * 2; i++) {
+ for (CodingErrorAction malformedInputAction : actions) {
+ try {
+ // UTF-8 doesn't have any unmappable characters
+ INPUT_VALID.toString(malformedInputAction, CodingErrorAction.IGNORE);
+ INPUT_INVALID.toString(malformedInputAction, CodingErrorAction.IGNORE);
+ } catch (CharacterCodingException e) {
+ // Ignore
+ }
+ }
+ }
+
+ Assert.assertNotNull(StringCache.bcCache);
+
+ // Check the valid input is cached correctly
+ for (CodingErrorAction malformedInputAction : actions) {
+ try {
+ Assert.assertEquals("ABCD", INPUT_VALID.toString(malformedInputAction, CodingErrorAction.IGNORE));
+ } catch (CharacterCodingException e) {
+ // Should not happen for valid input
+ Assert.fail();
+ }
+ }
+
+ // Check the valid input is cached correctly
+ try {
+ Assert.assertEquals("ABCD", INPUT_INVALID.toString(CodingErrorAction.IGNORE, CodingErrorAction.IGNORE));
+ } catch (CharacterCodingException e) {
+ // Should not happen for invalid input with IGNORE
+ Assert.fail();
+ }
+ try {
+ Assert.assertEquals("AB\ufffdCD", INPUT_INVALID.toString(CodingErrorAction.REPLACE, CodingErrorAction.IGNORE));
+ } catch (CharacterCodingException e) {
+ // Should not happen for invalid input with REPLACE
+ Assert.fail();
+ }
+ try {
+ Assert.assertEquals("ABCD", INPUT_INVALID.toString(CodingErrorAction.REPORT, CodingErrorAction.IGNORE));
+ // Should throw exception
+ Assert.fail();
+ } catch (CharacterCodingException e) {
+ // Ignore
+ }
+
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 04/04: Add tests for parsing query string parameters
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch tmp
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e1875c1cae1c800855d738be574fa5a47d46f5d4
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jun 22 19:32:15 2023 +0100
Add tests for parsing query string parameters
---
.../servlet/TestServletRequestQueryString.java | 205 +++++++++++++++++++++
1 file changed, 205 insertions(+)
diff --git a/test/jakarta/servlet/TestServletRequestQueryString.java b/test/jakarta/servlet/TestServletRequestQueryString.java
new file mode 100644
index 0000000000..70f42d12bd
--- /dev/null
+++ b/test/jakarta/servlet/TestServletRequestQueryString.java
@@ -0,0 +1,205 @@
+/*
+ * 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 jakarta.servlet;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.nio.charset.CodingErrorAction;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Enumeration;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import jakarta.servlet.http.HttpServlet;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import static org.apache.catalina.startup.SimpleHttpClient.CRLF;
+import org.apache.catalina.core.StandardContext;
+import org.apache.catalina.startup.SimpleHttpClient;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+
+@RunWith(Parameterized.class)
+public class TestServletRequestQueryString extends TomcatBaseTest {
+
+ private static final Integer SC_OK = Integer.valueOf(HttpServletResponse.SC_OK);
+ private static final Integer SC_BAD_REQUEST = Integer.valueOf(HttpServletResponse.SC_BAD_REQUEST);
+ private static final Integer ZERO = Integer.valueOf(0);
+ private static final Integer TWO = Integer.valueOf(2);
+ private static final Integer THREE = Integer.valueOf(3);
+
+ @Parameterized.Parameters(name = "{index}: queryString[{1}], expectedStatusCode[{2}]")
+ public static Collection<Object[]> parameters() {
+ List<Object[]> parameterSets = new ArrayList<>();
+ ParameterErrorHandlingConfiguration defaultConfig = new ParameterErrorHandlingConfiguration();
+
+ // Empty parameter
+ parameterSets.add(new Object[] { defaultConfig, "before=aaa&&after=zzz", SC_BAD_REQUEST, ZERO, null} );
+ ParameterErrorHandlingConfiguration config = new ParameterErrorHandlingConfiguration();
+ config.setSkipEmptyParameter(true);
+ parameterSets.add(new Object[] { config, "before=aaa&&after=zzz", SC_OK, TWO, null} );
+
+ // Invalid parameter
+ parameterSets.add(new Object[] { defaultConfig, "before=aaa&=value&after=zzz", SC_BAD_REQUEST, ZERO, null} );
+ config = new ParameterErrorHandlingConfiguration();
+ config.setSkipInvalidParameter(true);
+ parameterSets.add(new Object[] { config, "before=aaa&=value&after=zzz", SC_OK, TWO, null} );
+
+ // Invalid %nn encoding
+ parameterSets.add(new Object[] { defaultConfig, "before=aaa&test=val%GGue&after=zzz", SC_BAD_REQUEST, ZERO, null} );
+ config = new ParameterErrorHandlingConfiguration();
+ config.setSkipUrlDecodingError(true);
+ parameterSets.add(new Object[] { config, "before=aaa&test=val%GGue&after=zzz", SC_OK, TWO, null} );
+
+ // Invalid UTF-8 byte
+ parameterSets.add(new Object[] { defaultConfig, "before=aaa&test=val%FFue&after=zzz", SC_BAD_REQUEST, ZERO, null} );
+ config = new ParameterErrorHandlingConfiguration();
+ config.setSkipDecodingError(true);
+ config.onMalformedInput(CodingErrorAction.IGNORE);
+ parameterSets.add(new Object[] { config, "before=aaa&test=val%FFue&after=zzz", SC_OK, THREE, "value"} );
+ config = new ParameterErrorHandlingConfiguration();
+ config.setSkipDecodingError(true);
+ config.onMalformedInput(CodingErrorAction.REPLACE);
+ parameterSets.add(new Object[] { config, "before=aaa&test=val%FFue&after=zzz", SC_OK, THREE, "val\ufffdue"} );
+ config = new ParameterErrorHandlingConfiguration();
+ config.setSkipDecodingError(true);
+ config.onMalformedInput(CodingErrorAction.REPORT);
+ parameterSets.add(new Object[] { config, "before=aaa&test=val%FFue&after=zzz", SC_OK, TWO, null} );
+
+ // There are no unmappable UTF-8 code points
+
+ // Too many parameters
+ parameterSets.add(new Object[] { defaultConfig, "before=aaa&test=value&after=zzz&extra=yyy", SC_BAD_REQUEST, ZERO, null} );
+ config = new ParameterErrorHandlingConfiguration();
+ config.setSkipMaxParameterCountError(true);
+ parameterSets.add(new Object[] { config, "before=aaa&test=value&after=zzz&extra=yyy", SC_OK, THREE, null} );
+
+ return parameterSets;
+ }
+
+ @Parameter(0)
+ public ParameterErrorHandlingConfiguration parameterErrorHandlingConfiguration;
+
+ @Parameter(1)
+ public String queryString;
+
+ @Parameter(2)
+ public int expectedStatusCode;
+
+ @Parameter(3)
+ public int expectedValidParameterCount;
+
+ @Parameter(4)
+ public String expectedTestParameterValue;
+
+
+ @Test
+ public void testParameterParsing() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ tomcat.getConnector().setMaxParameterCount(3);
+
+ // No file system docBase required
+ StandardContext ctx = (StandardContext) tomcat.addContext("", null);
+
+ // Map the test Servlet
+ ParameterParsingServlet parameterParsingServlet = new ParameterParsingServlet();
+ Tomcat.addServlet(ctx, "parameterParsingServlet", parameterParsingServlet);
+ ctx.addServletMappingDecoded("/", "parameterParsingServlet");
+ ctx.setParameterErrorHandlingConfiguration(parameterErrorHandlingConfiguration);
+
+ tomcat.start();
+
+ TestParameterClient client = new TestParameterClient();
+ client.setPort(getPort());
+ client.setRequest(new String[] {
+ "GET /?" + queryString +" HTTP/1.1" + CRLF +
+ "Host: localhost:" + getPort() + CRLF +
+ "Connection: close" + CRLF +
+ CRLF });
+ client.setResponseBodyEncoding(StandardCharsets.UTF_8);
+ client.connect(600000, 600000);
+ client.processRequest();
+
+ Assert.assertEquals(expectedStatusCode, client.getStatusCode());
+
+ System.out.println(client.getResponseBody());
+ Map<String,List<String>> parameters = new LinkedHashMap<>();
+ if (client.isResponse200()) {
+ String[] lines = client.getResponseBody().split(System.lineSeparator());
+ for (String line : lines) {
+ // Every line should be name=value
+ int equalsPos = line.indexOf('=');
+ String name = line.substring(0, equalsPos);
+ String value = line.substring(equalsPos + 1);
+
+ List<String> values = parameters.computeIfAbsent(name, k -> new ArrayList<>());
+ values.add(value);
+ }
+ }
+
+ Assert.assertEquals(expectedValidParameterCount, parameters.size());
+
+ if (expectedTestParameterValue != null) {
+ List<String> values = parameters.get("test");
+ Assert.assertNotNull(values);
+ Assert.assertEquals(1, values.size());
+ Assert.assertEquals(expectedTestParameterValue, values.getFirst());
+ }
+ }
+
+
+ private static class ParameterParsingServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding(StandardCharsets.UTF_8);
+ PrintWriter pw = resp.getWriter();
+
+ Enumeration<String> names = req.getParameterNames();
+ while (names.hasMoreElements()) {
+ String name = names.nextElement();
+ for (String value : req.getParameterValues(name)) {
+ pw.print(name + "=" + value + '\n');
+ }
+ }
+ }
+ }
+
+
+ private static class TestParameterClient extends SimpleHttpClient {
+
+ @Override
+ public boolean isResponseBodyOK() {
+ return true;
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org