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 6f181e1062a472bc5f0234980f66cbde42c1041b
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Aug 15 20:32:15 2023 +0100
Implement parameter error handling changes
---
java/org/apache/catalina/connector/Request.java | 459 ++++++++++-----------
.../apache/catalina/core/StandardWrapperValve.java | 14 +-
.../util/http/InvalidParameterException.java | 98 +++++
java/org/apache/tomcat/util/http/Parameters.java | 132 ++----
.../apache/tomcat/util/http/TestParameters.java | 18 +-
5 files changed, 367 insertions(+), 354 deletions(-)
diff --git a/java/org/apache/catalina/connector/Request.java
b/java/org/apache/catalina/connector/Request.java
index 9a6de071d5..869cae087d 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -103,8 +103,8 @@ import org.apache.tomcat.util.buf.StringUtils;
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.InvalidParameterException;
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;
@@ -303,6 +303,12 @@ public class Request implements HttpServletRequest {
protected ParameterMap<String,String[]> parameterMap = new
ParameterMap<>();
+ /**
+ * The exception thrown, if any when parsing the parameters including
parts.
+ */
+ protected IllegalStateException parametersParseException = null;
+
+
/**
* The parts, if any, uploaded with this request.
*/
@@ -445,6 +451,7 @@ public class Request implements HttpServletRequest {
}
parts = null;
}
+ parametersParseException = null;
partsParseException = null;
locales.clear();
localesParsed = false;
@@ -1061,30 +1068,13 @@ public class Request implements HttpServletRequest {
}
- /**
- * @return the value of the specified request parameter, if any;
otherwise, return <code>null</code>. If there is
- * more than one value defined, return only the first one.
- *
- * @param name Name of the desired request parameter
- */
@Override
public String getParameter(String name) {
-
- if (!parametersParsed) {
- parseParameters();
- }
-
+ parseParameters();
return coyoteRequest.getParameters().getParameter(name);
-
}
- /**
- * Returns a <code>Map</code> 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.
- *
- * @return A <code>Map</code> containing parameter names as keys and
parameter values as map values.
- */
@Override
public Map<String,String[]> getParameterMap() {
@@ -1102,39 +1092,20 @@ public class Request implements HttpServletRequest {
parameterMap.setLocked(true);
return parameterMap;
-
}
- /**
- * @return the names of all defined request parameters for this request.
- */
@Override
public Enumeration<String> getParameterNames() {
-
- if (!parametersParsed) {
- parseParameters();
- }
-
+ parseParameters();
return coyoteRequest.getParameters().getParameterNames();
-
}
- /**
- * @return the defined values for the specified request parameter, if any;
otherwise, return <code>null</code>.
- *
- * @param name Name of the desired request parameter
- */
@Override
public String[] getParameterValues(String name) {
-
- if (!parametersParsed) {
- parseParameters();
- }
-
+ parseParameters();
return coyoteRequest.getParameters().getParameterValues(name);
-
}
@@ -2635,6 +2606,7 @@ public class Request implements HttpServletRequest {
getContext().getAuthenticator().logout(this);
}
+
@Override
public Collection<Part> getParts() throws IOException,
IllegalStateException, ServletException {
@@ -2653,6 +2625,7 @@ public class Request implements HttpServletRequest {
return parts;
}
+
private void parseParts() {
// Return immediately if the parts have already been parsed
@@ -2677,119 +2650,103 @@ public class Request implements HttpServletRequest {
Parameters parameters = coyoteRequest.getParameters();
parameters.setLimit(maxParameterCount);
- boolean success = false;
- try {
- File location;
- String locationStr = mce.getLocation();
- if (locationStr == null || locationStr.length() == 0) {
- location = ((File)
context.getServletContext().getAttribute(ServletContext.TEMPDIR));
- } else {
- // If relative, it is relative to TEMPDIR
- location = new File(locationStr);
- if (!location.isAbsolute()) {
- location = new File((File)
context.getServletContext().getAttribute(ServletContext.TEMPDIR),
- locationStr).getAbsoluteFile();
- }
- }
-
- if (!location.exists() && context.getCreateUploadTargets()) {
- log.warn(sm.getString("coyoteRequest.uploadCreate",
location.getAbsolutePath(),
- getMappingData().wrapper.getName()));
- if (!location.mkdirs()) {
- log.warn(sm.getString("coyoteRequest.uploadCreateFail",
location.getAbsolutePath()));
- }
+ File location;
+ String locationStr = mce.getLocation();
+ if (locationStr == null || locationStr.length() == 0) {
+ location = ((File)
context.getServletContext().getAttribute(ServletContext.TEMPDIR));
+ } else {
+ // If relative, it is relative to TEMPDIR
+ location = new File(locationStr);
+ if (!location.isAbsolute()) {
+ location =
+ new File((File)
context.getServletContext().getAttribute(ServletContext.TEMPDIR), locationStr)
+ .getAbsoluteFile();
}
+ }
- if (!location.isDirectory()) {
-
parameters.setParseFailedReason(FailReason.MULTIPART_CONFIG_INVALID);
- partsParseException = new
IOException(sm.getString("coyoteRequest.uploadLocationInvalid", location));
- return;
+ if (!location.exists() && context.getCreateUploadTargets()) {
+ log.warn(sm.getString("coyoteRequest.uploadCreate",
location.getAbsolutePath(),
+ getMappingData().wrapper.getName()));
+ if (!location.mkdirs()) {
+ log.warn(sm.getString("coyoteRequest.uploadCreateFail",
location.getAbsolutePath()));
}
+ }
+ if (!location.isDirectory()) {
+ partsParseException = new
IOException(sm.getString("coyoteRequest.uploadLocationInvalid", location));
+ return;
+ }
- // Create a new file upload handler
- DiskFileItemFactory factory = new DiskFileItemFactory();
- try {
- factory.setRepository(location.getCanonicalFile());
- } catch (IOException ioe) {
- parameters.setParseFailedReason(FailReason.IO_ERROR);
- partsParseException = ioe;
- return;
- }
- factory.setSizeThreshold(mce.getFileSizeThreshold());
+ // Create a new file upload handler
+ DiskFileItemFactory factory = new DiskFileItemFactory();
+ try {
+ factory.setRepository(location.getCanonicalFile());
+ } catch (IOException ioe) {
+ partsParseException = ioe;
+ return;
+ }
+ factory.setSizeThreshold(mce.getFileSizeThreshold());
- FileUpload upload = new FileUpload();
- upload.setFileItemFactory(factory);
- upload.setFileSizeMax(mce.getMaxFileSize());
- upload.setSizeMax(mce.getMaxRequestSize());
- if (maxParameterCount > -1) {
- // There is a limit. The limit for parts needs to be reduced by
- // the number of parameters we have already parsed.
- // Must be under the limit else parsing parameters would have
- // triggered an exception.
- upload.setFileCountMax(maxParameterCount - parameters.size());
- }
+ FileUpload upload = new FileUpload();
+ upload.setFileItemFactory(factory);
+ upload.setFileSizeMax(mce.getMaxFileSize());
+ upload.setSizeMax(mce.getMaxRequestSize());
+ if (maxParameterCount > -1) {
+ // There is a limit. The limit for parts needs to be reduced by
+ // the number of parameters we have already parsed.
+ // Must be under the limit else parsing parameters would have
+ // triggered an exception.
+ upload.setFileCountMax(maxParameterCount - parameters.size());
+ }
- parts = new ArrayList<>();
- try {
- List<FileItem> items = upload.parseRequest(new
ServletRequestContext(this));
- int maxPostSize = getConnector().getMaxPostSize();
- int postSize = 0;
- Charset charset = getCharset();
- for (FileItem item : items) {
- ApplicationPart part = new ApplicationPart(item, location);
- parts.add(part);
- if (part.getSubmittedFileName() == null) {
- String name = part.getName();
- if (maxPostSize >= 0) {
- // Have to calculate equivalent size. Not
completely
- // accurate but close enough.
- postSize += name.getBytes(charset).length;
- // Equals sign
- postSize++;
- // Value length
- postSize += part.getSize();
- // Value separator
- postSize++;
- if (postSize > maxPostSize) {
-
parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
- throw new
IllegalStateException(sm.getString("coyoteRequest.maxPostSizeExceeded"));
- }
- }
- String value = null;
- try {
- value = part.getString(charset.name());
- } catch (UnsupportedEncodingException uee) {
- // Not possible
+ parts = new ArrayList<>();
+ try {
+ List<FileItem> items = upload.parseRequest(new
ServletRequestContext(this));
+ int maxPostSize = getConnector().getMaxPostSize();
+ int postSize = 0;
+ Charset charset = getCharset();
+ for (FileItem item : items) {
+ ApplicationPart part = new ApplicationPart(item, location);
+ parts.add(part);
+ if (part.getSubmittedFileName() == null) {
+ String name = part.getName();
+ if (maxPostSize >= 0) {
+ // Have to calculate equivalent size. Not completely
+ // accurate but close enough.
+ postSize += name.getBytes(charset).length;
+ // Equals sign
+ postSize++;
+ // Value length
+ postSize += part.getSize();
+ // Value separator
+ postSize++;
+ if (postSize > maxPostSize) {
+ throw new
IllegalStateException(sm.getString("coyoteRequest.maxPostSizeExceeded"));
}
- parameters.addParameter(name, value);
}
+ String value = null;
+ try {
+ value = part.getString(charset.name());
+ } catch (UnsupportedEncodingException uee) {
+ // Not possible
+ }
+ parameters.addParameter(name, value);
}
-
- success = true;
- } catch (InvalidContentTypeException e) {
-
parameters.setParseFailedReason(FailReason.INVALID_CONTENT_TYPE);
- partsParseException = new ServletException(e);
- } catch (SizeException e) {
- parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
- checkSwallowInput();
- partsParseException = new IllegalStateException(e);
- } catch (IOException e) {
- parameters.setParseFailedReason(FailReason.IO_ERROR);
- partsParseException = e;
- } catch (IllegalStateException e) {
- // addParameters() will set parseFailedReason
- checkSwallowInput();
- partsParseException = e;
- }
- } finally {
- // This might look odd but is correct. setParseFailedReason() only
- // sets the failure reason if none is currently set. This code
could
- // 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);
}
+ } catch (InvalidContentTypeException e) {
+ partsParseException = new ServletException(e);
+ return;
+ } catch (SizeException e) {
+ checkSwallowInput();
+ partsParseException = new InvalidParameterException(e,
HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
+ return;
+ } catch (IOException e) {
+ partsParseException = e;
+ return;
+ } catch (IllegalStateException e) {
+ checkSwallowInput();
+ partsParseException = e;
+ return;
}
}
@@ -3012,131 +2969,142 @@ public class Request implements HttpServletRequest {
* Parse request parameters.
*/
protected void parseParameters() {
+ doParseParameters();
+ if (parametersParseException != null) {
+ throw parametersParseException;
+ }
+ }
+
+
+ protected void doParseParameters() {
+ if (parametersParsed) {
+ return;
+ }
parametersParsed = true;
Parameters parameters = coyoteRequest.getParameters();
- boolean success = false;
- try {
- // Set this every time in case limit has been changed via JMX
- int maxParameterCount = getConnector().getMaxParameterCount();
- if (parts != null && maxParameterCount > 0) {
- maxParameterCount -= parts.size();
- }
- parameters.setLimit(maxParameterCount);
- // getCharacterEncoding() may have been overridden to search for
- // hidden form field containing request encoding
- Charset charset = getCharset();
+ // Set this every time in case limit has been changed via JMX
+ int maxParameterCount = getConnector().getMaxParameterCount();
+ if (parts != null && maxParameterCount > 0) {
+ maxParameterCount -= parts.size();
+ }
+ parameters.setLimit(maxParameterCount);
- boolean useBodyEncodingForURI =
connector.getUseBodyEncodingForURI();
- parameters.setCharset(charset);
- if (useBodyEncodingForURI) {
- parameters.setQueryStringCharset(charset);
- }
- // Note: If !useBodyEncodingForURI, the query string encoding is
- // that set towards the start of CoyoteAdapter.service()
+ // getCharacterEncoding() may have been overridden to search for
+ // hidden form field containing request encoding
+ Charset charset = getCharset();
- parameters.handleQueryParameters();
+ boolean useBodyEncodingForURI = connector.getUseBodyEncodingForURI();
+ parameters.setCharset(charset);
+ if (useBodyEncodingForURI) {
+ parameters.setQueryStringCharset(charset);
+ }
+ // Note: If !useBodyEncodingForURI, the query string encoding is
+ // that set towards the start of CoyoteAdapter.service()
- if (usingInputStream || usingReader) {
- success = true;
- return;
- }
+ parameters.handleQueryParameters();
- String contentType = getContentType();
- if (contentType == null) {
- contentType = "";
- }
- int semicolon = contentType.indexOf(';');
- if (semicolon >= 0) {
- contentType = contentType.substring(0, semicolon).trim();
- } else {
- contentType = contentType.trim();
- }
+ if (usingInputStream || usingReader) {
+ return;
+ }
- if ("multipart/form-data".equals(contentType)) {
- parseParts();
- success = true;
- return;
- }
+ String contentType = getContentType();
+ if (contentType == null) {
+ contentType = "";
+ }
+ int semicolon = contentType.indexOf(';');
+ if (semicolon >= 0) {
+ contentType = contentType.substring(0, semicolon).trim();
+ } else {
+ contentType = contentType.trim();
+ }
- if (!getConnector().isParseBodyMethod(getMethod())) {
- success = true;
- return;
+ if ("multipart/form-data".equals(contentType)) {
+ parseParts();
+ if (partsParseException instanceof IllegalStateException) {
+ parametersParseException = (IllegalStateException)
partsParseException;
+ } else if (partsParseException != null) {
+ parametersParseException = new
InvalidParameterException(partsParseException);
}
+ return;
+ }
- if (!("application/x-www-form-urlencoded".equals(contentType))) {
- success = true;
- return;
- }
+ if (!getConnector().isParseBodyMethod(getMethod())) {
+ return;
+ }
- int len = getContentLength();
+ if (!("application/x-www-form-urlencoded".equals(contentType))) {
+ return;
+ }
- if (len > 0) {
- int maxPostSize = connector.getMaxPostSize();
- if ((maxPostSize >= 0) && (len > maxPostSize)) {
- Context context = getContext();
- if (context != null &&
context.getLogger().isDebugEnabled()) {
-
context.getLogger().debug(sm.getString("coyoteRequest.postTooLarge"));
- }
- checkSwallowInput();
- parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
- return;
+ int len = getContentLength();
+
+ if (len > 0) {
+ int maxPostSize = connector.getMaxPostSize();
+ if ((maxPostSize >= 0) && (len > maxPostSize)) {
+ String message = sm.getString("coyoteRequest.postTooLarge");
+ Context context = getContext();
+ if (context != null && context.getLogger().isDebugEnabled()) {
+ context.getLogger().debug(message);
}
- byte[] formData = null;
- if (len < CACHED_POST_LEN) {
- if (postData == null) {
- postData = new byte[CACHED_POST_LEN];
- }
- formData = postData;
- } else {
- formData = new byte[len];
+ checkSwallowInput();
+ parametersParseException =
+ new InvalidParameterException(message,
HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
+ return;
+ }
+ byte[] formData = null;
+ if (len < CACHED_POST_LEN) {
+ if (postData == null) {
+ postData = new byte[CACHED_POST_LEN];
}
- try {
- readPostBodyFully(formData, len);
- } catch (IOException e) {
- // Client disconnect
- Context context = getContext();
- if (context != null &&
context.getLogger().isDebugEnabled()) {
-
context.getLogger().debug(sm.getString("coyoteRequest.parseParameters"), e);
- }
-
parameters.setParseFailedReason(FailReason.CLIENT_DISCONNECT);
- return;
+ formData = postData;
+ } else {
+ formData = new byte[len];
+ }
+ try {
+ readPostBodyFully(formData, len);
+ } catch (IOException e) {
+ // Client disconnect
+ Context context = getContext();
+ if (context != null && context.getLogger().isDebugEnabled()) {
+
context.getLogger().debug(sm.getString("coyoteRequest.parseParameters"), e);
}
- parameters.processParameters(formData, 0, len);
- } else if
("chunked".equalsIgnoreCase(coyoteRequest.getHeader("transfer-encoding"))) {
- byte[] formData = null;
- try {
- formData = readChunkedPostBody();
- } catch (IllegalStateException ise) {
- // chunkedPostTooLarge error
- parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
- Context context = getContext();
- if (context != null &&
context.getLogger().isDebugEnabled()) {
-
context.getLogger().debug(sm.getString("coyoteRequest.parseParameters"), ise);
- }
- return;
- } catch (IOException e) {
- // Client disconnect
-
parameters.setParseFailedReason(FailReason.CLIENT_DISCONNECT);
- Context context = getContext();
- if (context != null &&
context.getLogger().isDebugEnabled()) {
-
context.getLogger().debug(sm.getString("coyoteRequest.parseParameters"), e);
- }
- return;
+ response.getCoyoteResponse().action(ActionCode.CLOSE_NOW,
null);
+ if (e instanceof ClientAbortException) {
+ parametersParseException = new
InvalidParameterException(e);
+ } else {
+ parametersParseException = new
InvalidParameterException(new ClientAbortException(e));
+ }
+ return;
+ }
+ parameters.processParameters(formData, 0, len);
+ } else if
("chunked".equalsIgnoreCase(coyoteRequest.getHeader("transfer-encoding"))) {
+ byte[] formData = null;
+ try {
+ formData = readChunkedPostBody();
+ } catch (IllegalStateException ise) {
+ parametersParseException = ise;
+ return;
+ } catch (IOException e) {
+ // Client disconnect
+ Context context = getContext();
+ if (context != null && context.getLogger().isDebugEnabled()) {
+
context.getLogger().debug(sm.getString("coyoteRequest.parseParameters"), e);
}
- if (formData != null) {
- parameters.processParameters(formData, 0, formData.length);
+ response.getCoyoteResponse().action(ActionCode.CLOSE_NOW,
null);
+ if (e instanceof ClientAbortException) {
+ parametersParseException = new
InvalidParameterException(e);
+ } else {
+ parametersParseException = new
InvalidParameterException(new ClientAbortException(e));
}
+ return;
}
- success = true;
- } finally {
- if (!success) {
- parameters.setParseFailedReason(FailReason.UNKNOWN);
+ if (formData != null) {
+ parameters.processParameters(formData, 0, formData.length);
}
}
-
}
@@ -3178,7 +3146,8 @@ public class Request implements HttpServletRequest {
if (connector.getMaxPostSize() >= 0 && (body.getLength() + len) >
connector.getMaxPostSize()) {
// Too much data
checkSwallowInput();
- throw new
IllegalStateException(sm.getString("coyoteRequest.chunkedPostTooLarge"));
+ throw new
InvalidParameterException(sm.getString("coyoteRequest.chunkedPostTooLarge"),
+ HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
}
if (len > 0) {
body.append(buffer, 0, len);
diff --git a/java/org/apache/catalina/core/StandardWrapperValve.java
b/java/org/apache/catalina/core/StandardWrapperValve.java
index 70226ce3a9..1c3e5c31d0 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;
@@ -195,6 +196,13 @@ final class StandardWrapperValve extends ValveBase {
}
throwable = e;
exception(request, response, e);
+ } catch (InvalidParameterException e) {
+ if (container.getLogger().isDebugEnabled()) {
+ container.getLogger().debug(
+ sm.getString("standardWrapper.serviceException",
wrapper.getName(), context.getName()), e);
+ }
+ throwable = e;
+ exception(request, response, e, e.getErrorCode());
} catch (Throwable e) {
ExceptionUtils.handleThrowable(e);
container.getLogger()
@@ -271,8 +279,12 @@ 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);
+ }
+
+ private void exception(Request request, Response response, Throwable
exception, int errorCode) {
request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, exception);
- response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ response.setStatus(errorCode);
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..f5162e11f6
--- /dev/null
+++ b/java/org/apache/tomcat/util/http/InvalidParameterException.java
@@ -0,0 +1,98 @@
+/*
+ * 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 jakarta.servlet.http.HttpServletResponse;
+
+/**
+ * Extend {@link IllegalStateException} to identify the cause as an invalid
parameter.
+ * <p>
+ * Implementation note: This class extends {@link IllegalStateException}
since that is the class that the Servlet 6.1
+ * onwards Javadocs define is thrown by the various {@code
ServletRequest.getParameterXXX()} methods.
+ */
+public class InvalidParameterException extends IllegalStateException {
+
+ private static final long serialVersionUID = 1L;
+
+ private final int errorCode;
+
+
+ /**
+ * Construct a new exception with the given message.
+ *
+ * @param message The message to use for the exception
+ */
+ public InvalidParameterException(String message) {
+ this(message, HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ /**
+ * Construct a new exception with the given message and error code.
+ *
+ * @param message The message to use for the exception
+ * @param errorCode The HTTP status code to use when reporting this error.
Expected to be >= 400.
+ */
+ public InvalidParameterException(String message, int errorCode) {
+ this(message, null, errorCode);
+ }
+
+
+ /**
+ * Construct a new exception with the given message and cause.
+ *
+ * @param message The message to use for the exception
+ * @param cause The exception to use as the cause of this exception
+ */
+ public InvalidParameterException(String message, Throwable cause) {
+ this(message, cause, HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ /**
+ * Construct a new exception with the given cause. The message for this
exception will be generated by calling
+ * {@code cause.toString()}.
+ *
+ * @param cause The exception to use as the cause of this exception
+ */
+ public InvalidParameterException(Throwable cause) {
+ this(cause.toString(), cause, HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ /**
+ * Construct a new exception with the given cause and error code. The
message for this exception will be generated
+ * by calling {@code cause.toString()}.
+ *
+ * @param cause The exception to use as the cause of this exception
+ * @param errorCode The HTTP status code to use when reporting this error.
Expected to be >= 400.
+ */
+ public InvalidParameterException(Throwable cause, int errorCode) {
+ this(cause.toString(), cause, errorCode);
+ }
+
+
+ private InvalidParameterException(String message, Throwable cause, int
errorCode) {
+ super(message, cause);
+ this.errorCode = errorCode;
+ }
+
+
+ public int getErrorCode() {
+ return errorCode;
+ }
+}
\ No newline at end of file
diff --git a/java/org/apache/tomcat/util/http/Parameters.java
b/java/org/apache/tomcat/util/http/Parameters.java
index b590d379e8..4c517e83c4 100644
--- a/java/org/apache/tomcat/util/http/Parameters.java
+++ b/java/org/apache/tomcat/util/http/Parameters.java
@@ -18,6 +18,7 @@ package org.apache.tomcat.util.http;
import java.io.IOException;
import java.nio.charset.Charset;
+import java.nio.charset.CodingErrorAction;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
@@ -31,17 +32,12 @@ import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.buf.StringUtils;
import org.apache.tomcat.util.buf.UDecoder;
-import org.apache.tomcat.util.log.UserDataHelper;
import org.apache.tomcat.util.res.StringManager;
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 maxParamCountLog = new
UserDataHelper(log);
-
private static final StringManager sm =
StringManager.getManager("org.apache.tomcat.util.http");
private final Map<String,ArrayList<String>> paramHashValues = new
LinkedHashMap<>();
@@ -202,10 +198,8 @@ 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)));
+ // Processing this parameter will push us over the limit.
+ throw new
InvalidParameterException(sm.getString("parameters.maxCountFail",
Integer.valueOf(limit)));
}
parameterCount++;
@@ -237,8 +231,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;
@@ -316,30 +308,15 @@ public final class Parameters {
continue;
}
// &=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 message = sm.getString("parameters.invalidChunk",
Integer.valueOf(nameStart),
+ Integer.valueOf(valueEnd), extract);
+ throw new InvalidParameterException(message);
}
tmpName.setBytes(bytes, nameStart, nameEnd - nameStart);
@@ -374,89 +351,34 @@ public final class Parameters {
urlDecode(tmpName);
}
tmpName.setCharset(charset);
- name = tmpName.toString();
+ name = tmpName.toString(CodingErrorAction.REPORT,
CodingErrorAction.REPORT);
if (valueStart >= 0) {
if (decodeValue) {
urlDecode(tmpValue);
}
tmpValue.setCharset(charset);
- value = tmpValue.toString();
+ value = tmpValue.toString(CodingErrorAction.REPORT,
CodingErrorAction.REPORT);
} 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;
- }
+ addParameter(name, value);
} 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);
- }
- }
- }
+ String message;
+ if (log.isDebugEnabled()) {
+ message = sm.getString("parameters.decodeFail.debug",
origName.toString(), origValue.toString());
+ } else {
+ message = sm.getString("parameters.decodeFail.info",
tmpName.toString(), tmpValue.toString());
}
- }
-
- 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 (logMode != null) {
- String message =
sm.getString("parameters.multipleDecodingFail",
Integer.valueOf(decodeFailCount));
- switch (logMode) {
- case INFO_THEN_DEBUG:
- message += sm.getString("parameters.fallToDebug");
- //$FALL-THROUGH$
- case INFO:
- log.info(message);
- break;
- case DEBUG:
- log.debug(message);
+ throw new InvalidParameterException(message, e);
+ } finally {
+ tmpName.recycle();
+ tmpValue.recycle();
+ // Only recycle copies if we used them
+ if (log.isDebugEnabled()) {
+ origName.recycle();
+ origValue.recycle();
}
}
}
diff --git a/test/org/apache/tomcat/util/http/TestParameters.java
b/test/org/apache/tomcat/util/http/TestParameters.java
index 885b563fe2..db1c073965 100644
--- a/test/org/apache/tomcat/util/http/TestParameters.java
+++ b/test/org/apache/tomcat/util/http/TestParameters.java
@@ -40,7 +40,7 @@ public class TestParameters {
new Parameter("\ufb6b\ufb6a\ufb72", "\uffee\uffeb\uffe2");
@Test
- public void testProcessParametersByteArrayIntInt() {
+ public void testProcessParametersByteArrayIntIntValid() {
doTestProcessParametersByteArrayIntInt(-1, SIMPLE);
doTestProcessParametersByteArrayIntInt(-1, SIMPLE_MULTIPLE);
doTestProcessParametersByteArrayIntInt(-1, NO_VALUE);
@@ -60,14 +60,26 @@ public class TestParameters {
doTestProcessParametersByteArrayIntInt(-1,
UTF8, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY);
+ doTestProcessParametersByteArrayIntInt(4,
+ SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testProcessParametersByteArrayIntIntInvalid01() {
doTestProcessParametersByteArrayIntInt(1,
SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testProcessParametersByteArrayIntIntInvalid02() {
doTestProcessParametersByteArrayIntInt(2,
SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testProcessParametersByteArrayIntIntInvalid03() {
doTestProcessParametersByteArrayIntInt(3,
SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
- doTestProcessParametersByteArrayIntInt(4,
- SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
}
// Make sure the inner Parameter class behaves correctly
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
