Author: markt
Date: Wed Jun 24 13:26:06 2015
New Revision: 1687261
URL: http://svn.apache.org/r1687261
Log:
Make the (first) reason parameter parsing failed available as a request
attribute and then use it to provide a better status code via the
FailedRequstFilter (if configured).
Modified:
tomcat/trunk/java/org/apache/catalina/Globals.java
tomcat/trunk/java/org/apache/catalina/connector/Request.java
tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
tomcat/trunk/test/org/apache/catalina/connector/TestRequest.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=1687261&r1=1687260&r2=1687261&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/Globals.java (original)
+++ tomcat/trunk/java/org/apache/catalina/Globals.java Wed Jun 24 13:26:06 2015
@@ -213,6 +213,13 @@ public final class Globals {
/**
+ * The reason that the parameter parsing failed.
+ */
+ public static final String PARAMETER_PARSE_FAILED_REASON_ATTR =
+ "org.apache.catalina.parameter_parse_failed_reason";
+
+
+ /**
* 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=1687261&r1=1687260&r2=1687261&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Wed Jun 24
13:26:06 2015
@@ -88,6 +88,7 @@ import org.apache.tomcat.util.buf.UDecod
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.ServerCookie;
import org.apache.tomcat.util.http.ServerCookies;
import org.apache.tomcat.util.http.fileupload.FileItem;
@@ -2636,6 +2637,7 @@ public class Request
}
if (!location.isDirectory()) {
+
parameters.setParseFailedReason(FailReason.MULTIPART_CONFIG_INVALID);
partsParseException = new IOException(
sm.getString("coyoteRequest.uploadLocationInvalid",
location));
@@ -2648,6 +2650,7 @@ public class Request
try {
factory.setRepository(location.getCanonicalFile());
} catch (IOException ioe) {
+ parameters.setParseFailedReason(FailReason.IO_ERROR);
partsParseException = ioe;
return;
}
@@ -2714,6 +2717,7 @@ public class Request
// Value separator
postSize++;
if (postSize > maxPostSize) {
+
parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
throw new IllegalStateException(sm.getString(
"coyoteRequest.maxPostSizeExceeded"));
}
@@ -2724,19 +2728,23 @@ public class Request
success = true;
} catch (InvalidContentTypeException e) {
+
parameters.setParseFailedReason(FailReason.INVALID_CONTENT_TYPE);
partsParseException = new ServletException(e);
} catch (FileUploadBase.SizeException e) {
+ parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
checkSwallowInput();
partsParseException = new IllegalStateException(e);
} catch (FileUploadException e) {
+ parameters.setParseFailedReason(FailReason.IO_ERROR);
partsParseException = new IOException(e);
} catch (IllegalStateException e) {
+ // addParameters() will set parseFailedReason
checkSwallowInput();
partsParseException = e;
}
} finally {
if (partsParseException != null || !success) {
- parameters.setParseFailed(true);
+ parameters.setParseFailedReason(FailReason.UNKNOWN);
}
}
}
@@ -3015,6 +3023,7 @@ public class Request
sm.getString("coyoteRequest.postTooLarge"));
}
checkSwallowInput();
+ parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
return;
}
byte[] formData = null;
@@ -3028,6 +3037,7 @@ public class Request
}
try {
if (readPostBody(formData, len) != len) {
+
parameters.setParseFailedReason(FailReason.REQUEST_BODY_INCOMPLETE);
return;
}
} catch (IOException e) {
@@ -3038,6 +3048,7 @@ public class Request
sm.getString("coyoteRequest.parseParameters"),
e);
}
+
parameters.setParseFailedReason(FailReason.CLIENT_DISCONNECT);
return;
}
parameters.processParameters(formData, 0, len);
@@ -3048,6 +3059,7 @@ public class Request
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(
@@ -3057,6 +3069,7 @@ public class Request
return;
} catch (IOException e) {
// Client disconnect
+
parameters.setParseFailedReason(FailReason.CLIENT_DISCONNECT);
Context context = getContext();
if (context != null &&
context.getLogger().isDebugEnabled()) {
context.getLogger().debug(
@@ -3072,7 +3085,7 @@ public class Request
success = true;
} finally {
if (!success) {
- parameters.setParseFailed(true);
+ parameters.setParseFailedReason(FailReason.UNKNOWN);
}
}
@@ -3275,6 +3288,18 @@ public class Request
}
@Override
+ public void set(Request request, String name, Object
value) {
+ // NO-OP
+ }
+ });
+ specialAttributes.put(Globals.PARAMETER_PARSE_FAILED_REASON_ATTR,
+ new SpecialAttributeAdapter() {
+ @Override
+ public Object get(Request request, String name) {
+ return
request.getCoyoteRequest().getParameters().getParseFailedReason();
+ }
+
+ @Override
public void set(Request request, String name, Object
value) {
// NO-OP
}
Modified: 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=1687261&r1=1687260&r2=1687261&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java Wed
Jun 24 13:26:06 2015
@@ -27,6 +27,7 @@ import javax.servlet.http.HttpServletRes
import org.apache.catalina.Globals;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.http.Parameters.FailReason;
/**
* Filter that will reject requests if there was a failure during parameter
@@ -53,8 +54,41 @@ public class FailedRequestFilter extends
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException {
if (!isGoodRequest(request)) {
- ((HttpServletResponse) response)
- .sendError(HttpServletResponse.SC_BAD_REQUEST);
+ FailReason reason = (FailReason) request.getAttribute(
+ Globals.PARAMETER_PARSE_FAILED_REASON_ATTR);
+
+ int status;
+
+ switch (reason) {
+ case IO_ERROR:
+ // Not the client's fault
+ status = HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+ break;
+ case POST_TOO_LARGE:
+ status = HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE;
+ break;
+ case TOO_MANY_PARAMETERS:
+ // 413/414 aren't really correct here since the request
body
+ // and/or URI could be well below any limits set. Use the
+ // default.
+ case UNKNOWN: // Assume the client is at fault
+ // Various things that the client can get wrong that don't have
+ // a specific status code so use the default.
+ case INVALID_CONTENT_TYPE:
+ case MULTIPART_CONFIG_INVALID:
+ case NO_NAME:
+ case REQUEST_BODY_INCOMPLETE:
+ case URL_DECODING:
+ case CLIENT_DISCONNECT:
+ // Client is never going to see this so this is really just
+ // for the access logs. The default is fine.
+ default:
+ // 400
+ status = HttpServletResponse.SC_BAD_REQUEST;
+ break;
+ }
+
+ ((HttpServletResponse) response).sendError(status);
return;
}
chain.doFilter(request, response);
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=1687261&r1=1687260&r2=1687261&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java Wed Jun 24
13:26:06 2015
@@ -66,10 +66,10 @@ public final class Parameters {
private int parameterCount = 0;
/**
- * Is set to <code>true</code> if there were failures during parameter
- * parsing.
+ * Set to the reason for the failure (the first failure if there is more
+ * than one) if there were failures during parameter parsing.
*/
- private boolean parseFailed = false;
+ private FailReason parseFailedReason = null;
public Parameters() {
// NO-OP
@@ -101,23 +101,34 @@ public final class Parameters {
}
}
+
public boolean isParseFailed() {
- return parseFailed;
+ return parseFailedReason != null;
+ }
+
+
+ public FailReason getParseFailedReason() {
+ return parseFailedReason;
}
- public void setParseFailed(boolean parseFailed) {
- this.parseFailed = parseFailed;
+
+ public void setParseFailedReason(FailReason failReason) {
+ if (this.parseFailedReason == null) {
+ this.parseFailedReason = failReason;
+ }
}
+
public void recycle() {
parameterCount = 0;
paramHashValues.clear();
didQueryParameters=false;
encoding=null;
decodedQuery.recycle();
- parseFailed = false;
+ parseFailedReason = null;
}
+
// -------------------- Data access --------------------
// Access to the current name/values, no side effect ( processing ).
// You must explicitly call handleQueryParameters and the post methods.
@@ -189,7 +200,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;
+ setParseFailedReason(FailReason.TOO_MANY_PARAMETERS);
throw new IllegalStateException(sm.getString(
"parameters.maxCountFail", Integer.valueOf(limit)));
}
@@ -334,7 +345,7 @@ public final class Parameters {
log.debug(message);
}
}
- parseFailed = true;
+ setParseFailedReason(FailReason.NO_NAME);
continue;
// invalid chunk - it's better to ignore
}
@@ -388,7 +399,6 @@ public final class Parameters {
} catch (IllegalStateException ise) {
// Hitting limit stops processing further params but does
// not cause request to fail.
- parseFailed = true;
UserDataHelper.Mode logMode =
maxParamCountLog.getNextMode();
if (logMode != null) {
String message = ise.getMessage();
@@ -407,7 +417,7 @@ public final class Parameters {
break;
}
} catch (IOException e) {
- parseFailed = true;
+ setParseFailedReason(FailReason.URL_DECODING);
decodeFailCount++;
if (decodeFailCount == 1 || log.isDebugEnabled()) {
if (log.isDebugEnabled()) {
@@ -511,4 +521,18 @@ public final class Parameters {
}
return sb.toString();
}
+
+
+ public enum FailReason {
+ CLIENT_DISCONNECT,
+ MULTIPART_CONFIG_INVALID,
+ INVALID_CONTENT_TYPE,
+ IO_ERROR,
+ NO_NAME,
+ POST_TOO_LARGE,
+ REQUEST_BODY_INCOMPLETE,
+ TOO_MANY_PARAMETERS,
+ UNKNOWN,
+ URL_DECODING
+ }
}
Modified: tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java?rev=1687261&r1=1687260&r2=1687261&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java (original)
+++ tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java Wed Jun 24
13:26:06 2015
@@ -84,17 +84,17 @@ public class TestRequest extends TomcatB
assertTrue(client.isResponseBodyOK());
client.reset();
client.doRequest(0, false); // 0 bytes - too small should fail
- assertTrue(client.isResponse400());
+ assertTrue(client.isResponse413());
client.reset();
client.doRequest(1, false); // 1 byte - too small should fail
- assertTrue(client.isResponse400());
+ assertTrue(client.isResponse413());
client.reset();
// Edge cases around actual content length
client.reset();
client.doRequest(6, false); // Too small should fail
- assertTrue(client.isResponse400());
+ assertTrue(client.isResponse413());
client.reset();
client.doRequest(7, false); // Just enough should pass
assertTrue(client.isResponse200());
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
