From 45c72e4f37a2f842568b97a2b114757d82c67066 Mon Sep 17 00:00:00 2001 From: rjung Date: Tue, 8 Mar 2011 17:18:16 +0000 Subject: [PATCH] New context attribute "swallowAbortedUploads" allows to make request data swallowing configurable for requests that are too large. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1079444 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/Context.java | 18 + java/org/apache/catalina/connector/Request.java | 16 + java/org/apache/catalina/connector/Response.java | 10 + java/org/apache/catalina/core/StandardContext.java | 32 +- java/org/apache/coyote/ActionCode.java | 7 + .../apache/coyote/ajp/AbstractAjpProcessor.java | 5 + .../coyote/http11/AbstractHttp11Processor.java | 6 + .../catalina/core/TestSwallowAbortedUploads.java | 399 +++++++++++++++++++++ .../apache/catalina/startup/SimpleHttpClient.java | 5 + webapps/docs/changelog.xml | 5 + webapps/docs/config/context.xml | 20 ++ 11 files changed, 522 insertions(+), 1 deletion(-) create mode 100644 test/org/apache/catalina/core/TestSwallowAbortedUploads.java diff --git a/java/org/apache/catalina/Context.java b/java/org/apache/catalina/Context.java index af26716e7..78a0d01f7 100644 --- a/java/org/apache/catalina/Context.java +++ b/java/org/apache/catalina/Context.java @@ -111,6 +111,24 @@ public interface Context extends Container { public boolean getAllowCasualMultipartParsing(); /** + * Set to false to disable request data swallowing + * after an upload was aborted due to size constraints. + * + * @param swallowAbortedUploads false to disable + * swallowing, true otherwise (default). + */ + public void setSwallowAbortedUploads(boolean swallowAbortedUploads); + + /** + * Returns true if remaining request data will be read + * (swallowed) even the request violates a data size constraint. + * + * @return true if data will be swallowed (default), + * false otherwise. + */ + public boolean getSwallowAbortedUploads(); + + /** * Return the set of initialized application event listener objects, * in the order they were specified in the web application deployment * descriptor, for this application. diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java index 231219499..30d55872a 100644 --- a/java/org/apache/catalina/connector/Request.java +++ b/java/org/apache/catalina/connector/Request.java @@ -800,6 +800,9 @@ public class Request */ public void finishRequest() throws IOException { // The reader and input stream don't need to be closed + // TODO: Is this ever called? + // If so, move input swallow disabling from + // Response.finishResponse() to here } @@ -2450,6 +2453,16 @@ public class Request return (inputBuffer.available() > 0); } + /** + * Disable swallowing of remaining input if configured + */ + protected void disableSwallowInput() { + Context context = getContext(); + if (context != null && !context.getSwallowAbortedUploads()) { + coyoteRequest.action(ActionCode.DISABLE_SWALLOW_INPUT, null); + } + } + public void cometClose() { coyoteRequest.action(ActionCode.COMET_CLOSE,getEvent()); } @@ -2620,6 +2633,7 @@ public class Request } catch (InvalidContentTypeException e) { partsParseException = new ServletException(e); } catch (FileUploadBase.SizeException e) { + disableSwallowInput(); partsParseException = new IllegalStateException(e); } catch (FileUploadException e) { partsParseException = new IOException(e); @@ -2845,6 +2859,7 @@ public class Request context.getLogger().debug( sm.getString("coyoteRequest.postTooLarge")); } + disableSwallowInput(); return; } byte[] formData = null; @@ -2922,6 +2937,7 @@ public class Request if (connector.getMaxPostSize() > 0 && (body.getLength() + len) > connector.getMaxPostSize()) { // Too much data + disableSwallowInput(); throw new IllegalArgumentException( sm.getString("coyoteRequest.chunkedPostTooLarge")); } diff --git a/java/org/apache/catalina/connector/Response.java b/java/org/apache/catalina/connector/Response.java index 00eb84406..26cb5ba17 100644 --- a/java/org/apache/catalina/connector/Response.java +++ b/java/org/apache/catalina/connector/Response.java @@ -49,6 +49,7 @@ import org.apache.catalina.core.ApplicationSessionCookieConfig; import org.apache.catalina.security.SecurityUtil; import org.apache.catalina.util.CharsetMapper; import org.apache.catalina.util.DateTool; +import org.apache.coyote.ActionCode; import org.apache.tomcat.util.buf.CharChunk; import org.apache.tomcat.util.buf.UEncoder; import org.apache.tomcat.util.http.FastHttpDateFormat; @@ -497,6 +498,15 @@ public class Response */ public void finishResponse() throws IOException { + // Optionally disable swallowing of additional request data. + // TODO: Should be in Request.finishRequest(), but that method + // seems to get called never. + Context context = getContext(); + if (context != null + && getStatus() == HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE + && !context.getSwallowAbortedUploads()) { + coyoteResponse.action(ActionCode.DISABLE_SWALLOW_INPUT, null); + } // Writing leftover bytes outputBuffer.close(); } diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index cc1e588ed..d5d80e3d7 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -197,6 +197,12 @@ public class StandardContext extends ContainerBase protected boolean allowCasualMultipartParsing = false; /** + * Control whether remaining request data will be read + * (swallowed) even if the request violates a data size constraint. + */ + public boolean swallowAbortedUploads = true; + + /** * The alternate deployment descriptor name. */ private String altDDName = null; @@ -1066,6 +1072,30 @@ public class StandardContext extends ContainerBase } /** + * Set to false to disable request data swallowing + * after an upload was aborted due to size constraints. + * + * @param swallowAbortedUploads false to disable + * swallowing, true otherwise (default). + */ + @Override + public void setSwallowAbortedUploads(boolean swallowAbortedUploads) { + this.swallowAbortedUploads = swallowAbortedUploads; + } + + /** + * Returns true if remaining request data will be read + * (swallowed) even the request violates a data size constraint. + * + * @return true if data will be swallowed (default), + * false otherwise. + */ + @Override + public boolean getSwallowAbortedUploads() { + return this.swallowAbortedUploads; + } + + /** * Set cache TTL. */ public void setCacheTTL(int cacheTTL) { @@ -6440,4 +6470,4 @@ public class StandardContext extends ContainerBase return false; } -} +} \ No newline at end of file diff --git a/java/org/apache/coyote/ActionCode.java b/java/org/apache/coyote/ActionCode.java index 31ee255b7..c268658f1 100644 --- a/java/org/apache/coyote/ActionCode.java +++ b/java/org/apache/coyote/ActionCode.java @@ -49,6 +49,13 @@ public enum ActionCode { POST_REQUEST, /** + * Hook called if swallowing request input should be disabled. + * Example: Cancel a large file upload. + * + */ + DISABLE_SWALLOW_INPUT, + + /** * Callback for lazy evaluation - extract the remote host address. */ REQ_HOST_ATTRIBUTE, diff --git a/java/org/apache/coyote/ajp/AbstractAjpProcessor.java b/java/org/apache/coyote/ajp/AbstractAjpProcessor.java index 44b00e13b..7c3fdc9f7 100644 --- a/java/org/apache/coyote/ajp/AbstractAjpProcessor.java +++ b/java/org/apache/coyote/ajp/AbstractAjpProcessor.java @@ -266,6 +266,11 @@ public abstract class AbstractAjpProcessor implements ActionHook, Processor { error = true; } + } else if (actionCode == ActionCode.DISABLE_SWALLOW_INPUT) { + // TODO: Do not swallow request input but + // make sure we are closing the connection + error = true; + } else if (actionCode == ActionCode.CLOSE) { // Close // End the processing of the current request, and stop any further diff --git a/java/org/apache/coyote/http11/AbstractHttp11Processor.java b/java/org/apache/coyote/http11/AbstractHttp11Processor.java index 765c79b7b..b212b51a3 100644 --- a/java/org/apache/coyote/http11/AbstractHttp11Processor.java +++ b/java/org/apache/coyote/http11/AbstractHttp11Processor.java @@ -767,6 +767,12 @@ public abstract class AbstractHttp11Processor implements ActionHook, Processor { response.setErrorException(e); } + } else if (actionCode == ActionCode.DISABLE_SWALLOW_INPUT) { + // Do not swallow request input but + // make sure we are closing the connection + error = true; + getInputBuffer().setSwallowInput(false); + } else if (actionCode == ActionCode.RESET) { // Reset response // Note: This must be called before the response is committed diff --git a/test/org/apache/catalina/core/TestSwallowAbortedUploads.java b/test/org/apache/catalina/core/TestSwallowAbortedUploads.java new file mode 100644 index 000000000..8ea33fbc4 --- /dev/null +++ b/test/org/apache/catalina/core/TestSwallowAbortedUploads.java @@ -0,0 +1,399 @@ +/* + * 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.core; + +import java.io.IOException; +import java.io.PrintWriter; +import java.util.Arrays; +import java.util.Collection; + +import javax.servlet.MultipartConfigElement; +import javax.servlet.ServletException; +import javax.servlet.annotation.MultipartConfig; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.Part; + +import org.apache.catalina.Context; +import org.apache.catalina.Wrapper; +import org.apache.catalina.startup.SimpleHttpClient; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; + +public class TestSwallowAbortedUploads extends TomcatBaseTest { + + private static Log log = LogFactory.getLog(TestSwallowAbortedUploads.class); + + /** + * Test whether size limited uploads correctly handle connection draining. + */ + public Exception doAbortedUploadTest(AbortedUploadClient client, boolean limited, + boolean swallow) { + client.setPort(getPort()); + Exception ex = client.doRequest(limited, swallow); + if (log.isDebugEnabled()) { + log.debug("Response line: " + client.getResponseLine()); + log.debug("Response headers: " + client.getResponseHeaders()); + log.debug("Response body: " + client.getResponseBody()); + if (ex != null) { + log.debug("Exception in client: ", ex); + } + + } + return ex; + } + + /** + * Test whether aborted POST correctly handle connection draining. + */ + public Exception doAbortedPOSTTest(AbortedPOSTClient client, int status, + boolean swallow) { + client.setPort(getPort()); + Exception ex = client.doRequest(status, swallow); + if (log.isDebugEnabled()) { + log.debug("Response line: " + client.getResponseLine()); + log.debug("Response headers: " + client.getResponseHeaders()); + log.debug("Response body: " + client.getResponseBody()); + if (ex != null) { + log.info("Exception in client: ", ex); + } + + } + return ex; + } + + public void testAbortedUploadUnlimitedSwallow() { + log.info("Unlimited, swallow enabled"); + AbortedUploadClient client = new AbortedUploadClient(); + Exception ex = doAbortedUploadTest(client, false, true); + assertNull("Unlimited upload with swallow enabled generates client exception", + ex); + assertTrue("Unlimited upload with swallow enabled returns error status code", + client.isResponse200()); + client.reset(); + } + + public void testAbortedUploadUnlimitedNoSwallow() { + log.info("Unlimited, swallow disabled"); + AbortedUploadClient client = new AbortedUploadClient(); + Exception ex = doAbortedUploadTest(client, false, false); + assertNull("Unlimited upload with swallow disabled generates client exception", + ex); + assertTrue("Unlimited upload with swallow disabled returns error status code", + client.isResponse200()); + client.reset(); + } + + public void testAbortedUploadLimitedSwallow() { + log.info("Limited, swallow enabled"); + AbortedUploadClient client = new AbortedUploadClient(); + Exception ex = doAbortedUploadTest(client, true, true); + assertNull("Limited upload with swallow enabled generates client exception", + ex); + assertTrue("Limited upload with swallow enabled returns error status code", + client.isResponse500()); + client.reset(); + } + + public void testAbortedUploadLimitedNoSwallow() { + log.info("Limited, swallow disabled"); + AbortedUploadClient client = new AbortedUploadClient(); + Exception ex = doAbortedUploadTest(client, true, false); + assertTrue("Limited upload with swallow disabled does not generate client exception", + ex != null && ex instanceof java.net.SocketException); + client.reset(); + } + + public void testAbortedPOSTOKSwallow() { + log.info("Aborted (OK), swallow enabled"); + AbortedPOSTClient client = new AbortedPOSTClient(); + Exception ex = doAbortedPOSTTest(client, HttpServletResponse.SC_OK, true); + assertNull("Unlimited upload with swallow enabled generates client exception", + ex); + assertTrue("Unlimited upload with swallow enabled returns error status code", + client.isResponse200()); + client.reset(); + } + + public void testAbortedPOSTOKNoSwallow() { + log.info("Aborted (OK), swallow disabled"); + AbortedPOSTClient client = new AbortedPOSTClient(); + Exception ex = doAbortedPOSTTest(client, HttpServletResponse.SC_OK, false); + assertNull("Unlimited upload with swallow disabled generates client exception", + ex); + assertTrue("Unlimited upload with swallow disabled returns error status code", + client.isResponse200()); + client.reset(); + } + + public void testAbortedPOST413Swallow() { + log.info("Aborted (413), swallow enabled"); + AbortedPOSTClient client = new AbortedPOSTClient(); + Exception ex = doAbortedPOSTTest(client, HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, true); + assertNull("Limited upload with swallow enabled generates client exception", + ex); + assertTrue("Limited upload with swallow enabled returns error status code", + client.isResponse413()); + client.reset(); + } + + public void testAbortedPOST413NoSwallow() { + log.info("Aborted (413), swallow disabled"); + AbortedPOSTClient client = new AbortedPOSTClient(); + Exception ex = doAbortedPOSTTest(client, HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, false); + assertTrue("Limited upload with swallow disabled does not generate client exception", + ex != null && ex instanceof java.net.SocketException); + client.reset(); + } + + @MultipartConfig + private static class AbortedUploadServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + PrintWriter out = resp.getWriter(); + resp.setContentType("text/plain"); + resp.setCharacterEncoding("UTF-8"); + StringBuilder sb = new StringBuilder(); + try { + Collection c = req.getParts(); + if (c == null) { + log.debug("Count: -1"); + sb.append("Count: -1\n"); + } else { + log.debug("Count: " + c.size()); + sb.append("Count: " + c.size() + "\n"); + for (Part p : c) { + log.debug("Name: " + p.getName() + ", Size: " + + p.getSize()); + sb.append("Name: " + p.getName() + ", Size: " + + p.getSize() + "\n"); + } + } + } catch (IllegalStateException ex) { + log.debug("IllegalStateException during getParts()"); + sb.append("IllegalStateException during getParts()\n"); + resp.setStatus(500); + } catch (Throwable ex) { + log.error("Exception during getParts()", ex); + sb.append(ex); + resp.setStatus(500); + } + out.print(sb.toString()); + resp.flushBuffer(); + } + + } + + /** + * Test no connection draining when upload too large + */ + private class AbortedUploadClient extends SimpleHttpClient { + + private static final String URI = "/uploadAborted"; + private static final String servletName = "uploadAborted"; + private static final int limitSize = 100; + private static final int hugeSize = 200000; + + private boolean init; + private Context context; + + private synchronized void init(boolean limited, boolean swallow) + throws Exception { + if (init) + return; + + Tomcat tomcat = getTomcatInstance(); + context = tomcat.addContext("", TEMP_DIR); + Wrapper w; + w = Tomcat.addServlet(context, servletName, + new AbortedUploadServlet()); + // Tomcat.addServlet does not respect annotations, so we have + // to set our own MultipartConfigElement. + // Choose upload file size limit. + if (limited) { + w.setMultipartConfigElement(new MultipartConfigElement("", + limitSize, -1, -1)); + } else { + w.setMultipartConfigElement(new MultipartConfigElement("")); + } + context.addServletMapping(URI, servletName); + context.setSwallowAbortedUploads(swallow); + + tomcat.start(); + + init = true; + } + + private Exception doRequest(boolean limited, boolean swallow) { + char body[] = new char[hugeSize]; + Arrays.fill(body, 'X'); + + try { + init(limited, swallow); + + // Open connection + connect(); + + // Send specified request body using method + String[] request; + + String boundary = "--simpleboundary"; + StringBuilder sb = new StringBuilder(); + + sb.append("--"); + sb.append(boundary); + sb.append(CRLF); + sb.append("Content-Disposition: form-data; name=\"part\""); + sb.append(CRLF); + sb.append(CRLF); + sb.append(body); + sb.append(CRLF); + sb.append("--"); + sb.append(boundary); + sb.append("--"); + sb.append(CRLF); + + // Re-encode the content so that bytes = characters + String content = new String(sb.toString().getBytes("UTF-8"), + "ASCII"); + + request = new String[] { "POST http://localhost:" + getPort() + URI + " HTTP/1.1" + CRLF + + "Host: localhost" + CRLF + + "Connection: close" + CRLF + + "Content-Type: multipart/form-data; boundary=" + boundary + CRLF + + "Content-Length: " + content.length() + CRLF + + CRLF + + content + CRLF }; + + setRequest(request); + processRequest(); // blocks until response has been read + + // Close the connection + disconnect(); + } catch (Exception e) { + return e; + } + return null; + } + + @Override + public boolean isResponseBodyOK() { + return false; // Don't care + } + } + + private static class AbortedPOSTServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + private int status = 200; + + public void setStatus(int status) { + this.status = status; + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + resp.setContentType("text/plain"); + resp.setCharacterEncoding("UTF-8"); + resp.setStatus(status); + PrintWriter out = resp.getWriter(); + out.print("OK"); + resp.flushBuffer(); + } + + } + + /** + * Test no connection draining when upload too large + */ + private class AbortedPOSTClient extends SimpleHttpClient { + + private static final String URI = "/uploadAborted"; + private static final String servletName = "uploadAborted"; + private static final int hugeSize = 200000; + + private boolean init; + private Context context; + + private synchronized void init(int status, boolean swallow) + throws Exception { + if (init) + return; + + Tomcat tomcat = getTomcatInstance(); + context = tomcat.addContext("", TEMP_DIR); + AbortedPOSTServlet servlet = new AbortedPOSTServlet(); + servlet.setStatus(status); + Tomcat.addServlet(context, servletName, + servlet); + context.addServletMapping(URI, servletName); + context.setSwallowAbortedUploads(swallow); + + tomcat.start(); + + init = true; + } + + private Exception doRequest(int status, boolean swallow) { + char body[] = new char[hugeSize]; + Arrays.fill(body, 'X'); + + try { + init(status, swallow); + + // Open connection + connect(); + + // Send specified request body using method + String[] request; + + String content = new String(body); + + request = new String[] { "POST http://localhost:" + getPort() + URI + " HTTP/1.1" + CRLF + + "Host: localhost" + CRLF + + "Connection: close" + CRLF + + "Content-Length: " + content.length() + CRLF + + CRLF + + content + CRLF }; + + setRequest(request); + processRequest(); // blocks until response has been read + + // Close the connection + disconnect(); + } catch (Exception e) { + return e; + } + return null; + } + + @Override + public boolean isResponseBodyOK() { + return false; // Don't care + } + } + +} diff --git a/test/org/apache/catalina/startup/SimpleHttpClient.java b/test/org/apache/catalina/startup/SimpleHttpClient.java index 933badd72..119b6958a 100644 --- a/test/org/apache/catalina/startup/SimpleHttpClient.java +++ b/test/org/apache/catalina/startup/SimpleHttpClient.java @@ -48,6 +48,7 @@ public abstract class SimpleHttpClient { public static final String OK_200 = "HTTP/1.1 200"; public static final String REDIRECT_302 = "HTTP/1.1 302"; public static final String FAIL_404 = "HTTP/1.1 404"; + public static final String FAIL_413 = "HTTP/1.1 413"; public static final String FAIL_50X = "HTTP/1.1 50"; public static final String FAIL_500 = "HTTP/1.1 500"; public static final String FAIL_501 = "HTTP/1.1 501"; @@ -218,6 +219,10 @@ public abstract class SimpleHttpClient { return getResponseLine().startsWith(FAIL_404); } + public boolean isResponse413() { + return getResponseLine().startsWith(FAIL_413); + } + public boolean isResponse50x() { return getResponseLine().startsWith(FAIL_50X); } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 42f26fab6..3c6d734c0 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -53,6 +53,11 @@ 50855: Fix NPE on HttpServletRequest.logout() when debug logging is enabled. (markt) + + New context attribute "swallowAbortedUploads" allows + to make request data swallowing configurable for requests + that are too large. (rjung) + diff --git a/webapps/docs/config/context.xml b/webapps/docs/config/context.xml index 3b185c032..a14bb671a 100644 --- a/webapps/docs/config/context.xml +++ b/webapps/docs/config/context.xml @@ -569,6 +569,26 @@ default value of true will be used.

+ +

Set to false if Tomcat should not read any additional + request body data for aborted uploads and instead abort the client + connection. This setting is used in the following situations: +

    +
  • the size of the request body is larger than the + maxPostSize configured in the connector +
    • +
  • the size limit of a MultiPart upload is reached +
    • +
  • the servlet sets the response status to 413 + (Request Entity Too Large) +
    • +
+ Not reading the additional data will free the request processing thread + more quickly. Unfortunately most HTTP clients will not read the response + in case they can not write the full request.

+

The default is true, so additional data is being read.

+ +

If the value of this flag is true, the bytes output to System.out and System.err by the web application will be redirected to -- 2.11.0