tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: svn commit: r1061929 - in /tomcat/trunk: java/org/apache/catalina/ant/ java/org/apache/catalina/connector/ java/org/apache/tomcat/util/net/ test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Date Fri, 21 Jan 2011 17:55:58 GMT
All,

Oops. I over-committed files. Evidently, svn commit doesn't work like
cvs commit.

I'll back-out my changes to:

tomcat/trunk/java/org/apache/catalina/ant/AbstractCatalinaTask.java
tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java

Sorry for the noise.

Thanks,
-chris

On 1/21/2011 12:46 PM, schultz@apache.org wrote:
> Author: schultz
> Date: Fri Jan 21 17:46:03 2011
> New Revision: 1061929
> 
> URL: http://svn.apache.org/viewvc?rev=1061929&view=rev
> Log:
> Fixed bug #49711: HttpServletRequest#getParts() does not work in a Filter
> - Added <Connector> 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"
> 
> 
> Modified:
>     tomcat/trunk/java/org/apache/catalina/ant/AbstractCatalinaTask.java
>     tomcat/trunk/java/org/apache/catalina/connector/Connector.java
>     tomcat/trunk/java/org/apache/catalina/connector/Request.java
>     tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
>     tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java
>     tomcat/trunk/webapps/docs/changelog.xml
>     tomcat/trunk/webapps/docs/config/ajp.xml
>     tomcat/trunk/webapps/docs/config/http.xml
> 
> Modified: tomcat/trunk/java/org/apache/catalina/ant/AbstractCatalinaTask.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ant/AbstractCatalinaTask.java?rev=1061929&r1=1061928&r2=1061929&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/ant/AbstractCatalinaTask.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/ant/AbstractCatalinaTask.java Fri Jan 21 17:46:03
2011
> @@ -170,6 +170,7 @@ public abstract class AbstractCatalinaTa
>          URLConnection conn = null;
>          InputStreamReader reader = null;
>          try {
> +            openRedirector();
>  
>              // Create a connection for this command
>              conn = (new URL(url + command)).openConnection();
> 
> Modified: tomcat/trunk/java/org/apache/catalina/connector/Connector.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Connector.java?rev=1061929&r1=1061928&r2=1061929&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/connector/Connector.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/connector/Connector.java Fri Jan 21 17:46:03
2011
> @@ -246,6 +246,13 @@ public class Connector extends Lifecycle
>        */
>       protected boolean useBodyEncodingForURI = false;
>  
> +
> +    /**
> +     * Allow multipart/form-data requests to be parsed even when the
> +     * target servlet doesn't specify @MultipartConfig or have a
> +     * &lt;multipart-config&gt; element.
> +     */
> +    protected boolean allowCasualMultipartParsing = false;
>       
>       protected static HashMap<String,String> replacements =
>           new HashMap<String,String>();
> @@ -767,6 +774,32 @@ public class Connector extends Lifecycle
>  
>       }
>  
> +    /**
> +     * Set to <code>true</code> to allow requests mapped to servlets that
> +     * do not explicitly declare @MultipartConfig or have
> +     * &lt;multipart-config&gt; specified in web.xml to parse
> +     * multipart/form-data requests.
> +     *
> +     * @param allowCasualMultipartParsing <code>true</code> to allow such
> +     *        casual parsing, <code>false</code> otherwise.
> +     */
> +    public void setAllowCasualMultipartParsing(boolean allowCasualMultipartParsing)
> +    {
> +        this.allowCasualMultipartParsing = allowCasualMultipartParsing;
> +    }
> +
> +    /**
> +     * Returns <code>true</code> if requests mapped to servlets without
> +     * "multipart config" to parse multipart/form-data requests anyway.
> +     *
> +     * @return <code>true</code> if requests mapped to servlets without
> +     *    "multipart config" to parse multipart/form-data requests,
> +     *    <code>false</code> otherwise.
> +     */
> +    protected boolean getAllowCasualMultipartParsing()
> +    {
> +        return this.allowCasualMultipartParsing;
> +    }
>  
>      /**
>       * Indicates whether the generation of an X-Powered-By response header for
> 
> 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=1061929&r1=1061928&r2=1061929&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Fri Jan 21 17:46:03
2011
> @@ -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<Part>();
>          try {
>              List<FileItem> 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) {
> 
> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1061929&r1=1061928&r2=1061929&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Fri Jan 21 17:46:03
2011
> @@ -242,6 +242,9 @@ public class AprEndpoint extends Abstrac
>      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.
> 
> 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=1061929&r1=1061928&r2=1061929&view=diff
> ==============================================================================
> --- tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java (original)
> +++ tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java Fri Jan 21 17:46:03
2011
> @@ -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 TomcatB
>          }
>      }
>  
> +    /**
> +     * 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;
> 
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1061929&r1=1061928&r2=1061929&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Fri Jan 21 17:46:03 2011
> @@ -66,6 +66,12 @@
>          Fix NPE in RemoteAddrFilter, RemoteHostFilter. (kkolinko)
>        </fix>
>        <fix>
> +        <bug>49711</bug>: 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)
> +      </fix>
> +      <fix>
>          <bug>50582</bug>: Refactor access logging so chunked encoding is
not
>          forced for all requests if bytes sent is logged. (markt)
>        </fix>
> 
> Modified: tomcat/trunk/webapps/docs/config/ajp.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/ajp.xml?rev=1061929&r1=1061928&r2=1061929&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/config/ajp.xml (original)
> +++ tomcat/trunk/webapps/docs/config/ajp.xml Fri Jan 21 17:46:03 2011
> @@ -74,6 +74,17 @@
>  
>    <attributes>
>  
> +    <attribute name="allowCasualMultipartParsing" required="false">
> +      <p>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 <code>true</code> causes Tomcat
> +      to behave in a way that is not technically spec-compliant.
> +      The default is <code>false</code></p>
> +    </attribute>
> +
>      <attribute name="allowTrace" required="false">
>        <p>A boolean value which can be used to enable or disable the TRACE
>        HTTP method. If not specified, this attribute is set to false.</p>
> @@ -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 <code>POST</code> 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.
> 
> Modified: tomcat/trunk/webapps/docs/config/http.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/http.xml?rev=1061929&r1=1061928&r2=1061929&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/config/http.xml (original)
> +++ tomcat/trunk/webapps/docs/config/http.xml Fri Jan 21 17:46:03 2011
> @@ -74,6 +74,17 @@
>  
>    <attributes>
>   
> +    <attribute name="allowCasualMultipartParsing" required="false">
> +      <p>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 <code>true</code> causes Tomcat
> +      to behave in a way that is not technically spec-compliant.
> +      The default is <code>false</code></p>
> +    </attribute>
> +
>      <attribute name="allowTrace" required="false">
>        <p>A boolean value which can be used to enable or disable the TRACE
>        HTTP method. If not specified, this attribute is set to false.</p>
> @@ -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 <code>POST</code> 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.
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 


Mime
View raw message