Author: kkolinko
Date: Mon Nov 7 10:46:14 2011
New Revision: 1198696
URL: http://svn.apache.org/viewvc?rev=1198696&view=rev
Log:
Introduce new request attribute to be used to mark request if there was a
failure during parameter parsing,
and a Filter that triggers parameter parsing and rejects requests marked with
that attribute.
Added:
tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
(with props)
Modified:
tomcat/trunk/java/org/apache/catalina/Globals.java
tomcat/trunk/java/org/apache/catalina/connector/Request.java
tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
Modified: tomcat/trunk/java/org/apache/catalina/Globals.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Globals.java?rev=1198696&r1=1198695&r2=1198696&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/Globals.java (original)
+++ tomcat/trunk/java/org/apache/catalina/Globals.java Mon Nov 7 10:46:14 2011
@@ -226,6 +226,17 @@ public final class Globals {
/**
+ * The request attribute that is set to {@code Boolean.TRUE} if some
request
+ * parameters have been ignored during request parameters parsing. It can
+ * happen, for example, if there is a limit on the total count of parseable
+ * parameters, or if parameter cannot be decoded, or any other error
+ * happened during parameter parsing.
+ */
+ public static final String PARAMETER_PARSE_FAILED_ATTR =
+ "org.apache.catalina.parameter_parse_failed";
+
+
+ /**
* The master flag which controls strict servlet specification
* compliance.
*/
Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1198696&r1=1198695&r2=1198696&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Mon Nov 7
10:46:14 2011
@@ -2383,6 +2383,12 @@ public class Request
}
}
+ private void checkParameterParseFailed() {
+ if (getCoyoteRequest().getParameters().isParseFailed()) {
+ setAttribute(Globals.PARAMETER_PARSE_FAILED_ATTR, Boolean.TRUE);
+ }
+ }
+
public void cometClose() {
coyoteRequest.action(ActionCode.COMET_CLOSE,getEvent());
setComet(false);
@@ -2487,109 +2493,117 @@ public class Request
Parameters parameters = coyoteRequest.getParameters();
- File location;
- String locationStr = mce.getLocation();
- if (locationStr == null || locationStr.length() == 0) {
- location = ((File) context.getServletContext().getAttribute(
- ServletContext.TEMPDIR));
- } else {
- location = new File(locationStr);
- }
-
- if (!location.isAbsolute() || !location.isDirectory()) {
- partsParseException = new IOException(
- sm.getString("coyoteRequest.uploadLocationInvalid",
- location));
- return;
- }
-
- // Create a new file upload handler
- DiskFileItemFactory factory = new DiskFileItemFactory();
+ boolean success = false;
try {
- factory.setRepository(location.getCanonicalFile());
- } catch (IOException ioe) {
- partsParseException = ioe;
- return;
- }
- factory.setSizeThreshold(mce.getFileSizeThreshold());
+ File location;
+ String locationStr = mce.getLocation();
+ if (locationStr == null || locationStr.length() == 0) {
+ location = ((File) context.getServletContext().getAttribute(
+ ServletContext.TEMPDIR));
+ } else {
+ location = new File(locationStr);
+ }
- ServletFileUpload upload = new ServletFileUpload();
- upload.setFileItemFactory(factory);
- upload.setFileSizeMax(mce.getMaxFileSize());
- upload.setSizeMax(mce.getMaxRequestSize());
+ if (!location.isAbsolute() || !location.isDirectory()) {
+ partsParseException = new IOException(
+ sm.getString("coyoteRequest.uploadLocationInvalid",
+ location));
+ return;
+ }
- parts = new ArrayList<Part>();
- try {
- List<FileItem> items = upload.parseRequest(this);
- int maxPostSize = getConnector().getMaxPostSize();
- int postSize = 0;
- String enc = getCharacterEncoding();
- Charset charset = null;
- if (enc != null) {
- try {
- charset = B2CConverter.getCharset(enc);
- } catch (UnsupportedEncodingException e) {
- // Ignore
- }
+
+ // Create a new file upload handler
+ DiskFileItemFactory factory = new DiskFileItemFactory();
+ try {
+ factory.setRepository(location.getCanonicalFile());
+ } catch (IOException ioe) {
+ partsParseException = ioe;
+ return;
}
- for (FileItem item : items) {
- ApplicationPart part = new ApplicationPart(item, mce);
- parts.add(part);
- if (part.getFilename() == null) {
- String name = part.getName();
- String value = null;
+ factory.setSizeThreshold(mce.getFileSizeThreshold());
+
+ ServletFileUpload upload = new ServletFileUpload();
+ upload.setFileItemFactory(factory);
+ upload.setFileSizeMax(mce.getMaxFileSize());
+ upload.setSizeMax(mce.getMaxRequestSize());
+
+ parts = new ArrayList<Part>();
+ try {
+ List<FileItem> items = upload.parseRequest(this);
+ int maxPostSize = getConnector().getMaxPostSize();
+ int postSize = 0;
+ String enc = getCharacterEncoding();
+ Charset charset = null;
+ if (enc != null) {
try {
- String encoding = parameters.getEncoding();
- if (encoding == null) {
- encoding = Parameters.DEFAULT_ENCODING;
- }
- value = part.getString(encoding);
- } catch (UnsupportedEncodingException uee) {
- try {
- value =
part.getString(Parameters.DEFAULT_ENCODING);
- } catch (UnsupportedEncodingException e) {
- // Should not be possible
- }
+ charset = B2CConverter.getCharset(enc);
+ } catch (UnsupportedEncodingException e) {
+ // Ignore
}
- if (maxPostSize > 0) {
- // Have to calculate equivalent size. Not completely
- // accurate but close enough.
- if (charset == null) {
- // Name length
- postSize += name.getBytes().length;
- } else {
- postSize += name.getBytes(charset).length;
+ }
+ for (FileItem item : items) {
+ ApplicationPart part = new ApplicationPart(item, mce);
+ parts.add(part);
+ if (part.getFilename() == null) {
+ String name = part.getName();
+ String value = null;
+ try {
+ String encoding = parameters.getEncoding();
+ if (encoding == null) {
+ encoding = Parameters.DEFAULT_ENCODING;
+ }
+ value = part.getString(encoding);
+ } catch (UnsupportedEncodingException uee) {
+ try {
+ value =
part.getString(Parameters.DEFAULT_ENCODING);
+ } catch (UnsupportedEncodingException e) {
+ // Should not be possible
+ }
}
- if (value != null) {
- // Equals sign
+ if (maxPostSize > 0) {
+ // Have to calculate equivalent size. Not
completely
+ // accurate but close enough.
+ if (charset == null) {
+ // Name length
+ postSize += name.getBytes().length;
+ } else {
+ postSize += name.getBytes(charset).length;
+ }
+ if (value != null) {
+ // Equals sign
+ postSize++;
+ // Value length
+ postSize += part.getSize();
+ }
+ // Value separator
postSize++;
- // Value length
- postSize += part.getSize();
- }
- // Value separator
- postSize++;
- if (postSize > maxPostSize) {
- throw new IllegalStateException(sm.getString(
- "coyoteRequest.maxPostSizeExceeded"));
+ if (postSize > maxPostSize) {
+ throw new IllegalStateException(sm.getString(
+ "coyoteRequest.maxPostSizeExceeded"));
+ }
}
+ parameters.addParameter(name, value);
}
- parameters.addParameter(name, value);
}
- }
- } catch (InvalidContentTypeException e) {
- partsParseException = new ServletException(e);
- } catch (FileUploadBase.SizeException e) {
- checkSwallowInput();
- partsParseException = new IllegalStateException(e);
- } catch (FileUploadException e) {
- partsParseException = new IOException(e);
- } catch (IllegalStateException e) {
- checkSwallowInput();
- partsParseException = e;
+ success = true;
+ } catch (InvalidContentTypeException e) {
+ partsParseException = new ServletException(e);
+ } catch (FileUploadBase.SizeException e) {
+ checkSwallowInput();
+ partsParseException = new IllegalStateException(e);
+ } catch (FileUploadException e) {
+ partsParseException = new IOException(e);
+ } catch (IllegalStateException e) {
+ checkSwallowInput();
+ partsParseException = e;
+ }
+ } finally {
+ if (partsParseException != null || !success) {
+ parameters.setParseFailed(true);
+ }
+ checkParameterParseFailed();
}
-
- return;
}
@@ -2774,108 +2788,120 @@ public class Request
parametersParsed = true;
Parameters parameters = coyoteRequest.getParameters();
- // Set this every time in case limit has been changed via JMX
- parameters.setLimit(getConnector().getMaxParameterCount());
+ boolean success = false;
+ try {
+ // Set this every time in case limit has been changed via JMX
+ parameters.setLimit(getConnector().getMaxParameterCount());
- // getCharacterEncoding() may have been overridden to search for
- // hidden form field containing request encoding
- String enc = getCharacterEncoding();
-
- boolean useBodyEncodingForURI = connector.getUseBodyEncodingForURI();
- if (enc != null) {
- parameters.setEncoding(enc);
- if (useBodyEncodingForURI) {
- parameters.setQueryStringEncoding(enc);
- }
- } else {
- parameters.setEncoding
- (org.apache.coyote.Constants.DEFAULT_CHARACTER_ENCODING);
- if (useBodyEncodingForURI) {
- parameters.setQueryStringEncoding
+ // getCharacterEncoding() may have been overridden to search for
+ // hidden form field containing request encoding
+ String enc = getCharacterEncoding();
+
+ boolean useBodyEncodingForURI =
connector.getUseBodyEncodingForURI();
+ if (enc != null) {
+ parameters.setEncoding(enc);
+ if (useBodyEncodingForURI) {
+ parameters.setQueryStringEncoding(enc);
+ }
+ } else {
+ parameters.setEncoding
(org.apache.coyote.Constants.DEFAULT_CHARACTER_ENCODING);
+ if (useBodyEncodingForURI) {
+ parameters.setQueryStringEncoding
+
(org.apache.coyote.Constants.DEFAULT_CHARACTER_ENCODING);
+ }
}
- }
- parameters.handleQueryParameters();
+ parameters.handleQueryParameters();
- if (usingInputStream || usingReader) {
- return;
- }
+ if (usingInputStream || usingReader) {
+ success = true;
+ return;
+ }
- if( !getConnector().isParseBodyMethod(getMethod()) ) {
- return;
- }
+ if( !getConnector().isParseBodyMethod(getMethod()) ) {
+ 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();
- }
+ 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 ("multipart/form-data".equals(contentType)) {
- parseParts();
- return;
- }
+ if ("multipart/form-data".equals(contentType)) {
+ parseParts();
+ success = true;
+ return;
+ }
- if (!("application/x-www-form-urlencoded".equals(contentType))) {
- return;
- }
+ if (!("application/x-www-form-urlencoded".equals(contentType))) {
+ success = true;
+ return;
+ }
- int len = getContentLength();
+ int len = getContentLength();
- if (len > 0) {
- int maxPostSize = connector.getMaxPostSize();
- if ((maxPostSize > 0) && (len > maxPostSize)) {
- if (context.getLogger().isDebugEnabled()) {
- context.getLogger().debug(
- sm.getString("coyoteRequest.postTooLarge"));
+ if (len > 0) {
+ int maxPostSize = connector.getMaxPostSize();
+ if ((maxPostSize > 0) && (len > maxPostSize)) {
+ if (context.getLogger().isDebugEnabled()) {
+ context.getLogger().debug(
+ sm.getString("coyoteRequest.postTooLarge"));
+ }
+ checkSwallowInput();
+ return;
}
- checkSwallowInput();
- return;
- }
- byte[] formData = null;
- if (len < CACHED_POST_LEN) {
- if (postData == null) {
- postData = new byte[CACHED_POST_LEN];
+ byte[] formData = null;
+ if (len < CACHED_POST_LEN) {
+ if (postData == null) {
+ postData = new byte[CACHED_POST_LEN];
+ }
+ formData = postData;
+ } else {
+ formData = new byte[len];
}
- formData = postData;
- } else {
- formData = new byte[len];
- }
- try {
- if (readPostBody(formData, len) != len) {
+ try {
+ if (readPostBody(formData, len) != len) {
+ return;
+ }
+ } catch (IOException e) {
+ // Client disconnect
+ if (context.getLogger().isDebugEnabled()) {
+ context.getLogger().debug(
+ sm.getString("coyoteRequest.parseParameters"),
e);
+ }
return;
}
- } catch (IOException e) {
- // Client disconnect
- if (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 (IOException e) {
+ // Client disconnect
+ if (context.getLogger().isDebugEnabled()) {
+ context.getLogger().debug(
+ sm.getString("coyoteRequest.parseParameters"),
e);
+ }
+ return;
}
- return;
- }
- parameters.processParameters(formData, 0, len);
- } else if ("chunked".equalsIgnoreCase(
- coyoteRequest.getHeader("transfer-encoding"))) {
- byte[] formData = null;
- try {
- formData = readChunkedPostBody();
- } catch (IOException e) {
- // Client disconnect
- if (context.getLogger().isDebugEnabled()) {
- context.getLogger().debug(
- sm.getString("coyoteRequest.parseParameters"), e);
+ if (formData != null) {
+ parameters.processParameters(formData, 0, formData.length);
}
- return;
}
- if (formData != null) {
- parameters.processParameters(formData, 0, formData.length);
+ } finally {
+ if (!success) {
+ parameters.setParseFailed(true);
}
+ checkParameterParseFailed();
}
}
Added: tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java?rev=1198696&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
(added)
+++ tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java Mon
Nov 7 10:46:14 2011
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.filters;
+
+import java.io.IOException;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.Globals;
+import org.apache.catalina.comet.CometEvent;
+import org.apache.catalina.comet.CometFilter;
+import org.apache.catalina.comet.CometFilterChain;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+
+/**
+ * Filter that will reject requests if there was a failure during parameter
+ * parsing. This filter can be used to ensure that none parameter values
+ * submitted by client are lost.
+ *
+ * <p>
+ * Note that it has side effect that it triggers parameter parsing and thus
+ * consumes the body for POST requests, so use it only with addresses that do
not
+ * use <code>request.getInputStream()</code> and
+ * <code>request.getReader()</code>.
+ */
+public class FailedRequestFilter extends FilterBase implements CometFilter {
+
+ private static final Log log =
LogFactory.getLog(FailedRequestFilter.class);
+
+ @Override
+ protected Log getLogger() {
+ return log;
+ }
+
+ @Override
+ public void doFilter(ServletRequest request, ServletResponse response,
+ FilterChain chain) throws IOException, ServletException {
+ if (!isGoodRequest(request)) {
+ ((HttpServletResponse) response)
+ .sendError(HttpServletResponse.SC_BAD_REQUEST);
+ return;
+ }
+ chain.doFilter(request, response);
+ }
+
+ @Override
+ public void doFilterEvent(CometEvent event, CometFilterChain chain)
+ throws IOException, ServletException {
+ if (event.getEventType() == CometEvent.EventType.BEGIN
+ && !isGoodRequest(event.getHttpServletRequest())) {
+ event.getHttpServletResponse().sendError(
+ HttpServletResponse.SC_BAD_REQUEST);
+ event.close();
+ return;
+ }
+ chain.doFilterEvent(event);
+ }
+
+ private boolean isGoodRequest(ServletRequest request) {
+ // Trigger parsing of parameters
+ request.getParameter("none");
+ // Detect failure
+ if (request.getAttribute(Globals.PARAMETER_PARSE_FAILED_ATTR) != null)
{
+ return false;
+ }
+ return true;
+ }
+
+ @Override
+ protected boolean isConfigProblemFatal() {
+ return true;
+ }
+
+}
Propchange:
tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java?rev=1198696&r1=1198695&r2=1198696&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java Mon Nov 7
10:46:14 2011
@@ -59,6 +59,12 @@ public final class Parameters {
private int limit = -1;
private int parameterCount = 0;
+ /**
+ * Is set to <code>true</code> if there were failures during parameter
+ * parsing.
+ */
+ private boolean parseFailed = false;
+
public Parameters() {
// NO-OP
}
@@ -89,12 +95,21 @@ public final class Parameters {
}
}
+ public boolean isParseFailed() {
+ return parseFailed;
+ }
+
+ public void setParseFailed(boolean parseFailed) {
+ this.parseFailed = parseFailed;
+ }
+
public void recycle() {
parameterCount = 0;
paramHashValues.clear();
didQueryParameters=false;
encoding=null;
decodedQuery.recycle();
+ parseFailed = false;
}
// -------------------- Data access --------------------
@@ -168,6 +183,7 @@ 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
+ parseFailed = true;
throw new IllegalStateException(sm.getString(
"parameters.maxCountFail", Integer.valueOf(limit)));
}
@@ -296,6 +312,7 @@ public final class Parameters {
null));
}
}
+ parseFailed = true;
continue;
// invalid chunk - it's better to ignore
}
@@ -337,10 +354,12 @@ public final class Parameters {
} catch (IllegalStateException ise) {
// Hitting limit stops processing further params but does
// not cause request to fail.
+ parseFailed = true;
log.warn(ise.getMessage());
break;
}
} catch (IOException e) {
+ parseFailed = true;
decodeFailCount++;
if (decodeFailCount == 1 || log.isDebugEnabled()) {
if (log.isDebugEnabled()) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
