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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org