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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org