You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/11/01 13:49:54 UTC

svn commit: r1195968 - in /tomcat/trunk: java/org/apache/catalina/connector/LocalStrings.properties java/org/apache/catalina/connector/Request.java java/org/apache/tomcat/util/http/Parameters.java test/org/apache/tomcat/util/http/TestParameters.java

Author: markt
Date: Tue Nov  1 12:49:53 2011
New Revision: 1195968

URL: http://svn.apache.org/viewvc?rev=1195968&view=rev
Log:
Extend the parameter limits to multi-part processing

Modified:
    tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
    tomcat/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
    tomcat/trunk/test/org/apache/tomcat/util/http/TestParameters.java

Modified: tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties?rev=1195968&r1=1195967&r2=1195968&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties Tue Nov  1 12:49:53 2011
@@ -62,6 +62,7 @@ coyoteRequest.authenticate.ise=Cannot ca
 coyoteRequest.uploadLocationInvalid=The temporary upload location [{0}] is not valid
 coyoteRequest.sessionEndAccessFail=Exception triggered ending access to session while recycling request
 coyoteRequest.sendfileNotCanonical=Unable to determine canonical name of file [{0}] specified for use with sendfile
+coyoteRequest.maxPostSizeExceeded=The multi-part request contained parameter data (excluding uploaded files) that exceeded the limit for maxPostSize set on the associated connector
 
 requestFacade.nullRequest=The request object has been recycled and is no longer associated with this facade
 

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=1195968&r1=1195967&r2=1195968&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Tue Nov  1 12:49:53 2011
@@ -22,6 +22,7 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
 import java.security.Principal;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -2520,26 +2521,59 @@ public class Request
         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
+                }
+            }
             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;
                         }
-                        parameters.addParameter(part.getName(),
-                                part.getString(encoding));
+                        value = part.getString(encoding);
                     } catch (UnsupportedEncodingException uee) {
                         try {
-                            parameters.addParameter(part.getName(),
-                                    part.getString(
-                                            Parameters.DEFAULT_ENCODING));
+                            value = part.getString(Parameters.DEFAULT_ENCODING);
                         } catch (UnsupportedEncodingException e) {
                             // Should not be possible
                         }
                     }
+                    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++;
+                        if (postSize > maxPostSize) {
+                            throw new IllegalStateException(sm.getString(
+                                    "coyoteRequest.maxPostSizeExceeded"));
+                        }
+                    }
+                    parameters.addParameter(name, value);
                 }
             }
 

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=1195968&r1=1195967&r2=1195968&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java Tue Nov  1 12:49:53 2011
@@ -157,10 +157,21 @@ public final class Parameters {
     }
 
 
-    public void addParameter( String key, String value ) {
+    public void addParameter( String key, String value )
+            throws IllegalStateException {
+
         if( key==null ) {
             return;
         }
+
+        parameterCount ++;
+        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
+            throw new IllegalStateException(sm.getString(
+                    "parameters.maxCountFail", Integer.valueOf(limit)));
+        }
+
         ArrayList<String> values = paramHashValues.get(key);
         if (values == null) {
             values = new ArrayList<String>(1);
@@ -204,13 +215,6 @@ public final class Parameters {
         int end = start + len;
 
         while(pos < end) {
-            parameterCount ++;
-
-            if (limit > -1 && parameterCount >= limit) {
-                log.warn(sm.getString("parameters.maxCountFail",
-                        Integer.valueOf(limit)));
-                break;
-            }
             int nameStart = pos;
             int nameEnd = -1;
             int valueStart = -1;
@@ -328,7 +332,14 @@ public final class Parameters {
                 tmpValue.setCharset(charset);
                 value = tmpValue.toString();
 
-                addParameter(name, value);
+                try {
+                    addParameter(name, value);
+                } catch (IllegalStateException ise) {
+                    // Hitting limit stops processing further params but does
+                    // not cause request to fail.
+                    log.warn(ise.getMessage());
+                    break;
+                }
             } catch (IOException e) {
                 decodeFailCount++;
                 if (decodeFailCount == 1 || log.isDebugEnabled()) {

Modified: tomcat/trunk/test/org/apache/tomcat/util/http/TestParameters.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/http/TestParameters.java?rev=1195968&r1=1195967&r2=1195968&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/http/TestParameters.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/http/TestParameters.java Tue Nov  1 12:49:53 2011
@@ -21,6 +21,7 @@ import java.util.Enumeration;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import org.junit.Test;
@@ -44,24 +45,33 @@ public class TestParameters {
 
     @Test
     public void testProcessParametersByteArrayIntInt() {
-        doTestProcessParametersByteArrayIntInt(SIMPLE);
-        doTestProcessParametersByteArrayIntInt(SIMPLE_MULTIPLE);
-        doTestProcessParametersByteArrayIntInt(NO_VALUE);
-        doTestProcessParametersByteArrayIntInt(EMPTY_VALUE);
-        doTestProcessParametersByteArrayIntInt(EMPTY);
-        doTestProcessParametersByteArrayIntInt(UTF8);
-        doTestProcessParametersByteArrayIntInt(
+        doTestProcessParametersByteArrayIntInt(-1, SIMPLE);
+        doTestProcessParametersByteArrayIntInt(-1, SIMPLE_MULTIPLE);
+        doTestProcessParametersByteArrayIntInt(-1, NO_VALUE);
+        doTestProcessParametersByteArrayIntInt(-1, EMPTY_VALUE);
+        doTestProcessParametersByteArrayIntInt(-1, EMPTY);
+        doTestProcessParametersByteArrayIntInt(-1, UTF8);
+        doTestProcessParametersByteArrayIntInt(-1,
                 SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY, UTF8);
-        doTestProcessParametersByteArrayIntInt(
+        doTestProcessParametersByteArrayIntInt(-1,
                 SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY, UTF8, SIMPLE);
-        doTestProcessParametersByteArrayIntInt(
+        doTestProcessParametersByteArrayIntInt(-1,
                 NO_VALUE, EMPTY_VALUE, EMPTY, UTF8, SIMPLE, SIMPLE_MULTIPLE);
-        doTestProcessParametersByteArrayIntInt(
+        doTestProcessParametersByteArrayIntInt(-1,
                 EMPTY_VALUE, EMPTY, UTF8, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE);
-        doTestProcessParametersByteArrayIntInt(
+        doTestProcessParametersByteArrayIntInt(-1,
                 EMPTY, UTF8, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE);
-        doTestProcessParametersByteArrayIntInt(
+        doTestProcessParametersByteArrayIntInt(-1,
                 UTF8, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY);
+
+        doTestProcessParametersByteArrayIntInt(1,
+                SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
+        doTestProcessParametersByteArrayIntInt(2,
+                SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
+        doTestProcessParametersByteArrayIntInt(3,
+                SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
+        doTestProcessParametersByteArrayIntInt(4,
+                SIMPLE, NO_VALUE, EMPTY_VALUE, UTF8);
     }
 
     // Make sure the inner Parameter class behaves correctly
@@ -73,7 +83,7 @@ public class TestParameters {
         assertEquals("foo4=", EMPTY_VALUE.toString());
     }
 
-    private long doTestProcessParametersByteArrayIntInt(
+    private long doTestProcessParametersByteArrayIntInt(int limit,
             Parameter... parameters) {
 
         // Build the byte array
@@ -92,12 +102,19 @@ public class TestParameters {
 
         Parameters p = new Parameters();
         p.setEncoding("UTF-8");
+        p.setLimit(limit);
 
         long start = System.nanoTime();
         p.processParameters(data, 0, data.length);
         long end = System.nanoTime();
 
-        validateParameters(parameters, p);
+        if (limit == -1) {
+            validateParameters(parameters, p);
+        } else {
+            Parameter[] limitParameters = new Parameter[limit];
+            System.arraycopy(parameters, 0, limitParameters, 0, limit);
+            validateParameters(limitParameters, p);
+        }
         return end - start;
     }
 
@@ -157,6 +174,73 @@ public class TestParameters {
         assertEquals("value4", values[3]);
     }
 
+    @Test
+    public void testAddParametersLimit() {
+        Parameters p = new Parameters();
+
+        p.setLimit(2);
+
+        // Empty at this point
+        Enumeration<String> names = p.getParameterNames();
+        assertFalse(names.hasMoreElements());
+        String[] values = p.getParameterValues("foo1");
+        assertNull(values);
+
+        // Add a parameter
+        p.addParameter("foo1", "value1");
+
+        names = p.getParameterNames();
+        assertTrue(names.hasMoreElements());
+        assertEquals("foo1", names.nextElement());
+        assertFalse(names.hasMoreElements());
+
+        values = p.getParameterValues("foo1");
+        assertEquals(1, values.length);
+        assertEquals("value1", values[0]);
+
+        // Add another parameter
+        p.addParameter("foo2", "value2");
+
+        names = p.getParameterNames();
+        assertTrue(names.hasMoreElements());
+        assertEquals("foo2", names.nextElement());
+        assertEquals("foo1", names.nextElement());
+        assertFalse(names.hasMoreElements());
+
+        values = p.getParameterValues("foo1");
+        assertEquals(1, values.length);
+        assertEquals("value1", values[0]);
+
+        values = p.getParameterValues("foo2");
+        assertEquals(1, values.length);
+        assertEquals("value2", values[0]);
+
+        // Add another parameter
+        IllegalStateException e = null;
+        try {
+            p.addParameter("foo3", "value3");
+        } catch (IllegalStateException ise) {
+            e = ise;
+        }
+        assertNotNull(e);
+
+        // Check current parameters remain unaffected
+        names = p.getParameterNames();
+        assertTrue(names.hasMoreElements());
+        assertEquals("foo2", names.nextElement());
+        assertEquals("foo1", names.nextElement());
+        assertFalse(names.hasMoreElements());
+
+        values = p.getParameterValues("foo1");
+        assertEquals(1, values.length);
+        assertEquals("value1", values[0]);
+
+        values = p.getParameterValues("foo2");
+        assertEquals(1, values.length);
+        assertEquals("value2", values[0]);
+
+    }
+
     private void validateParameters(Parameter[] parameters, Parameters p) {
         Enumeration<String> names = p.getParameterNames();
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org