From ee9dd59eb0c286ed387626362456a6249ada1816 Mon Sep 17 00:00:00 2001 From: schultz Date: Fri, 21 Jan 2011 17:46:03 +0000 Subject: [PATCH] Fixed bug #49711: HttpServletRequest#getParts() does not work in a Filter - Added attribute allowCasualMultipartParsing (default false) - Requests that contain multipart/form-data will be parsed in the absence of @MultipartConfig when the above attribute is set to "true" git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1061929 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/catalina/ant/AbstractCatalinaTask.java | 1 + java/org/apache/catalina/connector/Connector.java | 33 +++++ java/org/apache/catalina/connector/Request.java | 22 ++- java/org/apache/tomcat/util/net/AprEndpoint.java | 3 + .../org/apache/catalina/connector/TestRequest.java | 164 +++++++++++++++++++++ webapps/docs/changelog.xml | 6 + webapps/docs/config/ajp.xml | 13 +- webapps/docs/config/http.xml | 13 +- 8 files changed, 247 insertions(+), 8 deletions(-) diff --git a/java/org/apache/catalina/ant/AbstractCatalinaTask.java b/java/org/apache/catalina/ant/AbstractCatalinaTask.java index c248d42e6..b89ee6ed5 100644 --- a/java/org/apache/catalina/ant/AbstractCatalinaTask.java +++ b/java/org/apache/catalina/ant/AbstractCatalinaTask.java @@ -170,6 +170,7 @@ public abstract class AbstractCatalinaTask extends BaseRedirectorHelperTask { URLConnection conn = null; InputStreamReader reader = null; try { + openRedirector(); // Create a connection for this command conn = (new URL(url + command)).openConnection(); diff --git a/java/org/apache/catalina/connector/Connector.java b/java/org/apache/catalina/connector/Connector.java index f5fb1e5e6..6f54c66d6 100644 --- a/java/org/apache/catalina/connector/Connector.java +++ b/java/org/apache/catalina/connector/Connector.java @@ -246,6 +246,13 @@ public class Connector extends LifecycleMBeanBase { */ protected boolean useBodyEncodingForURI = false; + + /** + * Allow multipart/form-data requests to be parsed even when the + * target servlet doesn't specify @MultipartConfig or have a + * <multipart-config> element. + */ + protected boolean allowCasualMultipartParsing = false; protected static HashMap replacements = new HashMap(); @@ -767,6 +774,32 @@ public class Connector extends LifecycleMBeanBase { } + /** + * Set to true to allow requests mapped to servlets that + * do not explicitly declare @MultipartConfig or have + * <multipart-config> specified in web.xml to parse + * multipart/form-data requests. + * + * @param allowCasualMultipartParsing true to allow such + * casual parsing, false otherwise. + */ + public void setAllowCasualMultipartParsing(boolean allowCasualMultipartParsing) + { + this.allowCasualMultipartParsing = allowCasualMultipartParsing; + } + + /** + * Returns true if requests mapped to servlets without + * "multipart config" to parse multipart/form-data requests anyway. + * + * @return true if requests mapped to servlets without + * "multipart config" to parse multipart/form-data requests, + * false otherwise. + */ + protected boolean getAllowCasualMultipartParsing() + { + return this.allowCasualMultipartParsing; + } /** * Indicates whether the generation of an X-Powered-By response header for diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java index a3302b7e2..f65607878 100644 --- a/java/org/apache/catalina/connector/Request.java +++ b/java/org/apache/catalina/connector/Request.java @@ -2537,7 +2537,7 @@ public class Request return parts; } - + private void parseParts() { // Return immediately if the parts have already been parsed @@ -2545,13 +2545,22 @@ public class Request return; MultipartConfigElement mce = getWrapper().getMultipartConfigElement(); + if (mce == null) { - parts = Collections.emptyList(); - return; + Connector connector = getConnector(); + if(connector.getAllowCasualMultipartParsing()) { + mce = new MultipartConfigElement(null, + connector.getMaxPostSize(), + connector.getMaxPostSize(), + connector.getMaxPostSize()); + } else { + parts = Collections.emptyList(); + return; + } } Parameters parameters = coyoteRequest.getParameters(); - + File location; String locationStr = mce.getLocation(); if (locationStr == null || locationStr.length() == 0) { @@ -2582,7 +2591,7 @@ public class Request upload.setFileItemFactory(factory); upload.setFileSizeMax(mce.getMaxFileSize()); upload.setSizeMax(mce.getMaxRequestSize()); - + parts = new ArrayList(); try { List items = upload.parseRequest(this); @@ -2604,11 +2613,12 @@ public class Request Parameters.DEFAULT_ENCODING)}); } catch (UnsupportedEncodingException e) { // Should not be possible + e.printStackTrace(); } } } } - + } catch (InvalidContentTypeException e) { partsParseException = new ServletException(e); } catch (FileUploadBase.SizeException e) { diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index dcb912eec..c89d7512a 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -242,6 +242,9 @@ public class AprEndpoint extends AbstractEndpoint { public String getSSLCipherSuite() { return SSLCipherSuite; } public void setSSLCipherSuite(String SSLCipherSuite) { this.SSLCipherSuite = SSLCipherSuite; } + protected boolean SSLFIPSMode = false; + public boolean getSSLFIPSMode() { return SSLFIPSMode; } + public void setSSLFIPSMode(boolean SSLFIPSMode) { this.SSLFIPSMode = SSLFIPSMode; } /** * SSL certificate file. diff --git a/test/org/apache/catalina/connector/TestRequest.java b/test/org/apache/catalina/connector/TestRequest.java index 4c5beed17..8195543d0 100644 --- a/test/org/apache/catalina/connector/TestRequest.java +++ b/test/org/apache/catalina/connector/TestRequest.java @@ -25,12 +25,15 @@ import java.net.URL; import java.util.Enumeration; import java.util.TreeMap; +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 org.apache.catalina.Context; +import org.apache.catalina.Wrapper; import org.apache.catalina.authenticator.BasicAuthenticator; import org.apache.catalina.deploy.LoginConfig; import org.apache.catalina.startup.SimpleHttpClient; @@ -524,6 +527,167 @@ public class TestRequest extends TomcatBaseTest { } } + /** + * Test case for bug 49711: HttpServletRequest.getParts does not work + * in a filter. + */ + public void testBug49711() { + Bug49711Client client = new Bug49711Client(); + client.setPort(getPort()); + + // Make sure non-multipart works properly + client.doRequest("/regular", false, false); + + assertEquals("Incorrect response for GET request", + "parts=0", + client.getResponseBody()); + + client.reset(); + + // Make sure regular multipart works properly + client.doRequest("/multipart", false, true); // send multipart request + + assertEquals("Regular multipart doesn't work", + "parts=1", + client.getResponseBody()); + + client.reset(); + + // Make casual multipart request to "regular" servlet w/o config + // We expect that no parts will be available + client.doRequest("/regular", false, true); // send multipart request + + assertEquals("Incorrect response for non-configured casual multipart request", + "parts=0", // multipart request should be ignored + client.getResponseBody()); + + client.reset(); + + // Make casual multipart request to "regular" servlet w/config + // We expect that the server /will/ parse the parts, even though + // there is no @MultipartConfig + client.doRequest("/regular", true, true); // send multipart request + + assertEquals("Incorrect response for configured casual multipart request", + "parts=1", + client.getResponseBody()); + + client.reset(); + } + + private static class Bug49711Servlet extends HttpServlet { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + // Just echo the parameters and values back as plain text + resp.setContentType("text/plain"); + resp.setCharacterEncoding("UTF-8"); + + PrintWriter out = resp.getWriter(); + + out.println("parts=" + (null == req.getParts() + ? "null" + : req.getParts().size())); + } + } + + @MultipartConfig + private static class Bug49711Servlet_multipart extends Bug49711Servlet { + } + + /** + * Bug 49711 test client: test for casual getParts calls. + */ + private class Bug49711Client extends SimpleHttpClient { + + private boolean init; + + private synchronized void init() throws Exception { + if (init) return; + + Tomcat tomcat = getTomcatInstance(); + Context root = tomcat.addContext("", TEMP_DIR); + Tomcat.addServlet(root, "regular", new Bug49711Servlet()); + Wrapper w = Tomcat.addServlet(root, "multipart", new Bug49711Servlet_multipart()); + + // Tomcat.addServlet does not respect annotations, so we have + // to set our own MultipartConfigElement. + w.setMultipartConfigElement(new MultipartConfigElement("")); + + root.addServletMapping("/regular", "regular"); + root.addServletMapping("/multipart", "multipart"); + tomcat.start(); + + init = true; + } + + private Exception doRequest(String uri, + boolean allowCasualMultipart, + boolean makeMultipartRequest) { + Tomcat tomcat = getTomcatInstance(); + + tomcat.getConnector().setAllowCasualMultipartParsing(allowCasualMultipart); + + try { + init(); + + // Open connection + connect(); + + // Send specified request body using method + String[] request; + + if(makeMultipartRequest) { + String boundary = "--simpleboundary"; + + String content = "--" + boundary + CRLF + + "Content-Disposition: form-data; name=\"name\"" + CRLF + CRLF + + "value" + CRLF + + "--" + boundary + "--" + CRLF + ; + + // Re-encode the content so that bytes = characters + if(null != content) + content = new String(content.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 + }; + } + else + { + request = new String[] { + "GET http://localhost:" + getPort() + uri + " HTTP/1.1" + CRLF + + "Host: localhost" + CRLF + + "Connection: close" + CRLF + + 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 HttpURLConnection getConnection() throws IOException { final String query = "http://localhost:" + getPort() + "/"; URL postURL; diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index e3644dd29..7a7d89603 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -66,6 +66,12 @@ Fix NPE in RemoteAddrFilter, RemoteHostFilter. (kkolinko) + 49711: HttpServletRequest#getParts will work in a filter + or servlet without an @MultipartConfig annotation or + MultipartConfigElement if the new "allowCasualMultipartParsing" + connector attribute is set to "true". (schultz) + + 50582: Refactor access logging so chunked encoding is not forced for all requests if bytes sent is logged. (markt) diff --git a/webapps/docs/config/ajp.xml b/webapps/docs/config/ajp.xml index 9c71e5cb3..0e56e0a8e 100644 --- a/webapps/docs/config/ajp.xml +++ b/webapps/docs/config/ajp.xml @@ -74,6 +74,17 @@ + +

Set to true if Tomcat should automatically parse + multipart/form-data request bodies when HttpServletRequest.getPart* + or HttpServletRequest.getParameter* is called, even when the + target servlet isn't marked with the @MultipartConfig annotation + (See Servlet Specification 3.0, Section 3.2 for details). + Note that any setting other than true causes Tomcat + to behave in a way that is not technically spec-compliant. + The default is false

+ +

A boolean value which can be used to enable or disable the TRACE HTTP method. If not specified, this attribute is set to false.

@@ -121,7 +132,7 @@ to POST. This is useful in RESTful applications that want to support POST-style semantics for PUT requests. Note that any setting other than POST causes Tomcat - to behave in a way that does against the intent of the servlet + to behave in a way that goes against the intent of the servlet specification. The HTTP method TRACE is specifically forbidden here in accordance with the HTTP specification. diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml index 55ea85a96..ce51cd82c 100644 --- a/webapps/docs/config/http.xml +++ b/webapps/docs/config/http.xml @@ -74,6 +74,17 @@ + +

Set to true if Tomcat should automatically parse + multipart/form-data request bodies when HttpServletRequest.getPart* + or HttpServletRequest.getParameter* is called, even when the + target servlet isn't marked with the @MultipartConfig annotation + (See Servlet Specification 3.0, Section 3.2 for details). + Note that any setting other than true causes Tomcat + to behave in a way that is not technically spec-compliant. + The default is false

+ +

A boolean value which can be used to enable or disable the TRACE HTTP method. If not specified, this attribute is set to false.

@@ -121,7 +132,7 @@ to POST. This is useful in RESTful applications that want to support POST-style semantics for PUT requests. Note that any setting other than POST causes Tomcat - to behave in a way that does against the intent of the servlet + to behave in a way that goes against the intent of the servlet specification. The HTTP method TRACE is specifically forbidden here in accordance with the HTTP specification. -- 2.11.0