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:23 UTC
[tomcat] 03/04: First pass at implementing changes for parameter parsing
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