You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2011/11/10 23:24:18 UTC

svn commit: r1200601 - in /tomcat/tc6.0.x/trunk: ./ conf/ java/org/apache/catalina/ java/org/apache/catalina/connector/ java/org/apache/catalina/filters/ java/org/apache/coyote/ java/org/apache/tomcat/util/buf/ java/org/apache/tomcat/util/http/ webapps...

Author: kkolinko
Date: Thu Nov 10 22:24:17 2011
New Revision: 1200601

URL: http://svn.apache.org/viewvc?rev=1200601&view=rev
Log:
- Improve performance of parameter processing.
- Implement FailedRequestFilter to reject broken requests

Added:
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
    tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/LocalStrings.properties
Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/conf/web.xml
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/Globals.java
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Connector.java
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/mbeans-descriptors.xml
    tomcat/tc6.0.x/trunk/java/org/apache/coyote/Request.java
    tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java
    tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
    tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/MessageBytes.java
    tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/StringCache.java
    tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
    tomcat/tc6.0.x/trunk/webapps/docs/config/ajp.xml
    tomcat/tc6.0.x/trunk/webapps/docs/config/filter.xml
    tomcat/tc6.0.x/trunk/webapps/docs/config/http.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Nov 10 22:24:17 2011
@@ -64,22 +64,6 @@ PATCHES PROPOSED TO BACKPORT:
       - getStuckThreadIds() returns a list of ids. It might be useful to
         have a similar method that returns Thread.getName() names.
 
-* Improve performance of parameter processing.
-  http://people.apache.org/~kkolinko/patches/2011-11-10_tc6_parameters-v4.patch
-      <add>
-        Improve performance of parameter processing for GET and POST requests.
-        Also add an option to limit the maximum number of parameters processed
-        per request. This defaults to 10000. Excessive parameters are ignored.
-        Note that <code>FailedRequestFilter</code> can be used to reject the
-        request if some parameters were ignored. (markt/kkolinko)
-      </add>
-      <add>
-        New filter <code>FailedRequestFilter</code> that will reject a request
-        if there were errors during HTTP parameter parsing. (kkolinko)
-      </add>
-  +1: kkolinko, markt, rjung
-  -1:
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52121
   Fix possible output corruption when compression is
   enabled for a connector and the response is flushed.

Modified: tomcat/tc6.0.x/trunk/conf/web.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/conf/web.xml?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/conf/web.xml (original)
+++ tomcat/tc6.0.x/trunk/conf/web.xml Thu Nov 10 22:24:17 2011
@@ -421,6 +421,19 @@
 
   <!-- ================== Built In Filter Definitions ===================== -->
 
+  <!-- A filter that triggers request parameters parsing and rejects the    -->
+  <!-- request if some parameters were skipped because of parsing errors or -->
+  <!-- request size limitations.                                            -->
+<!--
+    <filter>
+        <filter-name>failedRequestFilter</filter-name>
+        <filter-class>
+          org.apache.catalina.filters.FailedRequestFilter
+        </filter-class>
+    </filter>
+-->
+
+
   <!-- NOTE: An SSI Servlet is also available as an alternative SSI         -->
   <!-- implementation. Use either the Servlet or the Filter but NOT both.   -->
   <!--                                                                      -->
@@ -481,6 +494,14 @@
 
   <!-- ==================== Built In Filter Mappings ====================== -->
 
+  <!-- The mapping for the Failed Request Filter -->
+<!--
+    <filter-mapping>
+        <filter-name>failedRequestFilter</filter-name>
+        <url-pattern>/*</url-pattern>
+    </filter-mapping>
+-->
+
   <!-- The mapping for the SSI Filter -->
 <!--
     <filter-mapping>

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/Globals.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/Globals.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/Globals.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/Globals.java Thu Nov 10 22:24:17 2011
@@ -327,6 +327,17 @@ public final class Globals {
 
 
     /**
+     * The request attribute that is set to <code>Boolean.TRUE</code> if some request
+     * parameters have been ignored during request parameters parsing. It can
+     * happen, for example, if there is a limit on the total count of parseable
+     * parameters, or if parameter cannot be decoded, or any other error
+     * happened during parameter parsing.
+     */
+    public static final String PARAMETER_PARSE_FAILED_ATTR =
+        "org.apache.catalina.parameter_parse_failed";
+
+
+    /**
      * The master flag which controls strict servlet specification 
      * compliance.
      */

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Connector.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Connector.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Connector.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Connector.java Thu Nov 10 22:24:17 2011
@@ -192,6 +192,13 @@ public class Connector
 
 
     /**
+     * The maximum number of parameters (GET plus POST) which will be
+     * automatically parsed by the container. 10000 by default. A value of less
+     * than 0 means no limit.
+     */
+    protected int maxParameterCount = 10000;
+
+    /**
      * Maximum size of a POST which will be automatically parsed by the
      * container. 2MB by default.
      */
@@ -502,14 +509,34 @@ public class Connector
     }
 
 
-     /**
-      * Return the mapper.
-      */
-     public Mapper getMapper() {
+    /**
+     * Return the mapper.
+     */
+    public Mapper getMapper() {
+        return (mapper);
+    }
 
-         return (mapper);
 
-     }
+    /**
+     * Return the maximum number of parameters (GET plus POST) that will be
+     * automatically parsed by the container. A value of less than 0 means no
+     * limit.
+     */
+    public int getMaxParameterCount() {
+        return maxParameterCount;
+    }
+
+
+    /**
+     * Set the maximum number of parameters (GET plus POST) that will be
+     * automatically parsed by the container. A value of less than 0 means no
+     * limit.
+     *
+     * @param maxParameterCount The new setting
+     */
+    public void setMaxParameterCount(int maxParameterCount) {
+        this.maxParameterCount = maxParameterCount;
+    }
 
 
     /**

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java Thu Nov 10 22:24:17 2011
@@ -920,6 +920,11 @@ public class Request
             return (requestDispatcherPath == null) 
                 ? getRequestPathMB().toString()
                 : requestDispatcherPath.toString();
+        } else if (name.equals(Globals.PARAMETER_PARSE_FAILED_ATTR)) {
+            if (coyoteRequest.getParameters().isParseFailed()) {
+                return Boolean.TRUE;
+            }
+            return null;
         }
 
         Object attr=attributes.get(name);
@@ -976,12 +981,12 @@ public class Request
      * <ul>
      * <li>{@link Globals.DISPATCHER_TYPE_ATTR}</li>
      * <li>{@link Globals.DISPATCHER_REQUEST_PATH_ATTR}</li>
-     * <li>{@link Globals.ASYNC_SUPPORTED_ATTR}</li>
      * <li>{@link Globals.CERTIFICATES_ATTR} (SSL connections only)</li>
      * <li>{@link Globals.CIPHER_SUITE_ATTR} (SSL connections only)</li>
      * <li>{@link Globals.KEY_SIZE_ATTR} (SSL connections only)</li>
      * <li>{@link Globals.SSL_SESSION_ID_ATTR} (SSL connections only)</li>
      * <li>{@link Globals.SSL_SESSION_MGR_ATTR} (SSL connections only)</li>
+     * <li>{@link Globals#PARAMETER_PARSE_FAILED_ATTR}</li>
      * </ul>
      * The underlying connector may also expose request attributes. These all
      * have names starting with "org.apache.tomcat" and include:
@@ -2565,6 +2570,8 @@ public class Request
         parametersParsed = true;
 
         Parameters parameters = coyoteRequest.getParameters();
+        // Set this every time in case limit has been changed via JMX
+        parameters.setLimit(getConnector().getMaxParameterCount());
 
         // getCharacterEncoding() may have been overridden to search for
         // hidden form field containing request encoding
@@ -2605,53 +2612,61 @@ public class Request
         if (!("application/x-www-form-urlencoded".equals(contentType)))
             return;
 
+        boolean success = false;
         int len = getContentLength();
 
-        if (len > 0) {
-            int maxPostSize = connector.getMaxPostSize();
-            if ((maxPostSize > 0) && (len > maxPostSize)) {
-                if (context.getLogger().isDebugEnabled()) {
-                    context.getLogger().debug(
-                            sm.getString("coyoteRequest.postTooLarge"));
+        try {
+            if (len > 0) {
+                int maxPostSize = connector.getMaxPostSize();
+                if ((maxPostSize > 0) && (len > maxPostSize)) {
+                    if (context.getLogger().isDebugEnabled()) {
+                        context.getLogger().debug(
+                                sm.getString("coyoteRequest.postTooLarge"));
+                    }
+                    return;
                 }
-                return;
-            }
-            byte[] formData = null;
-            if (len < CACHED_POST_LEN) {
-                if (postData == null)
-                    postData = new byte[CACHED_POST_LEN];
-                formData = postData;
-            } else {
-                formData = new byte[len];
-            }
-            try {
-                if (readPostBody(formData, len) != len) {
+                byte[] formData = null;
+                if (len < CACHED_POST_LEN) {
+                    if (postData == null)
+                        postData = new byte[CACHED_POST_LEN];
+                    formData = postData;
+                } else {
+                    formData = new byte[len];
+                }
+                try {
+                    if (readPostBody(formData, len) != len) {
+                        return;
+                    }
+                } catch (IOException e) {
+                    // Client disconnect
+                    if (context.getLogger().isDebugEnabled()) {
+                        context.getLogger().debug(
+                                sm.getString("coyoteRequest.parseParameters"), e);
+                    }
                     return;
                 }
-            } catch (IOException e) {
-                // Client disconnect
-                if (context.getLogger().isDebugEnabled()) {
-                    context.getLogger().debug(
-                            sm.getString("coyoteRequest.parseParameters"), e);
+                parameters.processParameters(formData, 0, len);
+            } else if ("chunked".equalsIgnoreCase(
+                    coyoteRequest.getHeader("transfer-encoding"))) {
+                byte[] formData = null;
+                try {
+                    formData = readChunkedPostBody();
+                } catch (IOException e) {
+                    // Client disconnect
+                    if (context.getLogger().isDebugEnabled()) {
+                        context.getLogger().debug(
+                                sm.getString("coyoteRequest.parseParameters"), e);
+                    }
+                    return;
                 }
-                return;
-            }
-            parameters.processParameters(formData, 0, len);
-        } else if ("chunked".equalsIgnoreCase(
-                coyoteRequest.getHeader("transfer-encoding"))) {
-            byte[] formData = null;
-            try {
-                formData = readChunkedPostBody();
-            } catch (IOException e) {
-                // Client disconnect
-                if (context.getLogger().isDebugEnabled()) {
-                    context.getLogger().debug(
-                            sm.getString("coyoteRequest.parseParameters"), e);
+                if (formData != null) {
+                    parameters.processParameters(formData, 0, formData.length);
                 }
-                return;
             }
-            if (formData != null) {
-                parameters.processParameters(formData, 0, formData.length);
+            success = true;
+        } finally {
+            if (!success) {
+                parameters.setParseFailed(true);
             }
         }
 

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/mbeans-descriptors.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/mbeans-descriptors.xml?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/mbeans-descriptors.xml (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/mbeans-descriptors.xml Thu Nov 10 22:24:17 2011
@@ -118,6 +118,10 @@
           description="Maximum number of Keep-Alive requests to honor per connection"
                  type="int"/>
 
+    <attribute   name="maxParameterCount"
+          description="The maximum number of parameters (GET plus POST) which will be automatically parsed by the container. 10000 by default. A value of less than 0 means no limit."
+                 type="int"/>
+
     <attribute   name="maxPostSize"
           description="Maximum size in bytes of a POST which will be handled by the servlet API provided features"
                  type="int"/>

Added: tomcat/tc6.0.x/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java?rev=1200601&view=auto
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java (added)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java Thu Nov 10 22:24:17 2011
@@ -0,0 +1,92 @@
+/*
+ * 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.filters;
+
+import java.io.IOException;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.Globals;
+import org.apache.catalina.CometEvent;
+import org.apache.catalina.CometFilter;
+import org.apache.catalina.CometFilterChain;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+
+/**
+ * Filter that will reject requests if there was a failure during parameter
+ * parsing. This filter can be used to ensure that none parameter values
+ * submitted by client are lost.
+ *
+ * <p>
+ * Note that it has side effect that it triggers parameter parsing and thus
+ * consumes the body for POST requests. Parameter parsing does check content
+ * type of the request, so there should not be problems with addresses that use
+ * <code>request.getInputStream()</code> and <code>request.getReader()</code>,
+ * if requests parsed by them do not use standard value for content mime-type.
+ */
+public class FailedRequestFilter extends FilterBase implements CometFilter {
+
+    private static final Log log = LogFactory.getLog(FailedRequestFilter.class);
+
+    @Override
+    protected Log getLogger() {
+        return log;
+    }
+
+    public void doFilter(ServletRequest request, ServletResponse response,
+            FilterChain chain) throws IOException, ServletException {
+        if (!isGoodRequest(request)) {
+            ((HttpServletResponse) response)
+                    .sendError(HttpServletResponse.SC_BAD_REQUEST);
+            return;
+        }
+        chain.doFilter(request, response);
+    }
+
+    public void doFilterEvent(CometEvent event, CometFilterChain chain)
+            throws IOException, ServletException {
+        if (event.getEventType() == CometEvent.EventType.BEGIN
+                && !isGoodRequest(event.getHttpServletRequest())) {
+            event.getHttpServletResponse().sendError(
+                    HttpServletResponse.SC_BAD_REQUEST);
+            event.close();
+            return;
+        }
+        chain.doFilterEvent(event);
+    }
+
+    private boolean isGoodRequest(ServletRequest request) {
+        // Trigger parsing of parameters
+        request.getParameter("none");
+        // Detect failure
+        if (request.getAttribute(Globals.PARAMETER_PARSE_FAILED_ATTR) != null) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    protected boolean isConfigProblemFatal() {
+        return true;
+    }
+
+}

Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/Request.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/Request.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/Request.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/Request.java Thu Nov 10 22:24:17 2011
@@ -73,7 +73,6 @@ public final class Request {
 
         parameters.setQuery(queryMB);
         parameters.setURLDecoder(urlDecoder);
-        parameters.setHeaders(headers);
 
     }
 

Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java Thu Nov 10 22:24:17 2011
@@ -110,7 +110,14 @@ public class B2CConverter {
         convert(bb, cb, cb.getBuffer().length - cb.getEnd());
     }
 
-    
+    /**
+     * Convert a buffer of bytes into a chars.
+     *
+     * @param bb    Input byte buffer
+     * @param cb    Output char buffer
+     * @param limit Number of bytes to convert
+     * @throws IOException
+     */    
     public void convert( ByteChunk bb, CharChunk cb, int limit) 
         throws IOException
     {
@@ -118,7 +125,7 @@ public class B2CConverter {
         try {
             // read from the reader
             int bbLengthBeforeRead  = 0;
-            while( limit > 0 ) { // conv.ready() ) {
+            while( limit > 0 ) {
                 int size = limit < BUFFER_SIZE ? limit : BUFFER_SIZE;
                 bbLengthBeforeRead = bb.getLength();
                 int cnt=conv.read( result, 0, size );

Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java Thu Nov 10 22:24:17 2011
@@ -19,7 +19,9 @@ package org.apache.tomcat.util.buf;
 
 import java.io.IOException;
 import java.io.Serializable;
+import java.io.UnsupportedEncodingException;
 import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
 import java.nio.charset.Charset;
 
 /*
@@ -96,20 +98,25 @@ public final class ByteChunk implements 
         as most standards seem to converge, but the servlet API requires
         8859_1, and this object is used mostly for servlets. 
     */
-    public static final String DEFAULT_CHARACTER_ENCODING="ISO-8859-1";
+    public static final Charset DEFAULT_CHARSET;
+
+    static {
+        Charset c = null;
+        try {
+            c = B2CConverter.getCharset("ISO-8859-1");
+        } catch (UnsupportedEncodingException e) {
+            // Should never happen since all JVMs must support ISO-8859-1
+        }
+        DEFAULT_CHARSET = c;
+    }
 
-    /** Default Charset to use for interpreting byte[] as as String
-    */
-    public static final Charset DEFAULT_CHARSET =
-        Charset.forName(DEFAULT_CHARACTER_ENCODING);
-    
     // byte[]
     private byte[] buff;
 
     private int start=0;
     private int end;
 
-    private String enc;
+    private Charset charset;
 
     private boolean isSet=false; // XXX
 
@@ -149,7 +156,7 @@ public final class ByteChunk implements 
      */
     public void recycle() {
         //        buff = null;
-        enc=null;
+        charset=null;
         start=0;
         end=0;
         isSet=false;
@@ -189,13 +196,15 @@ public final class ByteChunk implements 
         this.optimizedWrite = optimizedWrite;
     }
 
-    public void setEncoding( String enc ) {
-        this.enc=enc;
+    public void setCharset(Charset charset) {
+        this.charset = charset;
     }
-    public String getEncoding() {
-        if (enc == null)
-            enc=DEFAULT_CHARACTER_ENCODING;
-        return enc;
+
+    public Charset getCharset() {
+        if (charset == null) {
+            charset = DEFAULT_CHARSET;
+        }
+        return charset;
     }
 
     /**
@@ -498,34 +507,15 @@ public final class ByteChunk implements 
     }
     
     public String toStringInternal() {
-        String strValue=null;
-        try {
-            Charset charset;
-            if (enc == null) {
-                charset = DEFAULT_CHARSET;
-            } else {
-                charset = B2CConverter.getCharset(enc);
-            }
-            strValue = charset.decode(
-                    ByteBuffer.wrap(buff, start, end-start)).toString();
-            /*
-             Does not improve the speed too much on most systems,
-             it's safer to use the "clasical" new String().
-             
-             Most overhead is in creating char[] and copying,
-             the internal implementation of new String() is very close to
-             what we do. The decoder is nice for large buffers and if
-             we don't go to String ( so we can take advantage of reduced GC)
-             
-             // Method is commented out, in:
-              return B2CConverter.decodeString( enc );
-              */
-        } catch (java.io.UnsupportedEncodingException e) {
-            // Use the platform encoding in that case; the usage of a bad
-            // encoding will have been logged elsewhere already
-            strValue = new String(buff, start, end-start);
+        if (charset == null) {
+            charset = DEFAULT_CHARSET;
         }
-        return strValue;
+        // new String(byte[], int, int, Charset) takes a defensive copy of the
+        // entire byte array. This is expensive if only a small subset of the
+        // bytes will be used. The code below is from Apache Harmony.
+        CharBuffer cb;
+        cb = charset.decode(ByteBuffer.wrap(buff, start, end-start));
+        return new String(cb.array(), cb.arrayOffset(), cb.length());
     }
 
     public int getInt()

Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/MessageBytes.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/MessageBytes.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/MessageBytes.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/MessageBytes.java Thu Nov 10 22:24:17 2011
@@ -21,6 +21,7 @@ import java.text.*;
 import java.util.*;
 import java.io.Serializable;
 import java.io.IOException;
+import java.nio.charset.Charset;
 
 /**
  * This class is used to represent a subarray of bytes in an HTTP message.
@@ -140,13 +141,13 @@ public final class MessageBytes implemen
      *  previous conversion is reset.
      *  If no encoding is set, we'll use 8859-1.
      */
-    public void setEncoding( String enc ) {
-	if( !byteC.isNull() ) {
-	    // if the encoding changes we need to reset the converion results
-	    charC.recycle();
-	    hasStrValue=false;
-	}
-	byteC.setEncoding(enc);
+    public void setCharset(Charset charset) {
+        if( !byteC.isNull() ) {
+            // if the encoding changes we need to reset the conversion results
+            charC.recycle();
+            hasStrValue=false;
+        }
+        byteC.setCharset(charset);
     }
 
     /** 

Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/StringCache.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/StringCache.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/StringCache.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/StringCache.java Thu Nov 10 22:24:17 2011
@@ -17,6 +17,7 @@
 
 package org.apache.tomcat.util.buf;
 
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -292,7 +293,7 @@ public class StringCache {
                             entry.name = new byte[bc.getLength()];
                             System.arraycopy(bc.getBuffer(), start, entry.name, 0, end - start);
                             // Set encoding
-                            entry.enc = bc.getEncoding();
+                            entry.charset = bc.getCharset();
                             // Initialize occurrence count to one 
                             count = new int[1];
                             count[0] = 1;
@@ -472,7 +473,7 @@ public class StringCache {
     protected static final String find(ByteChunk name) {
         int pos = findClosest(name, bcCache, bcCache.length);
         if ((pos < 0) || (compare(name, bcCache[pos].name) != 0)
-                || !(name.getEncoding().equals(bcCache[pos].enc))) {
+                || !(name.getCharset().equals(bcCache[pos].charset))) {
             return null;
         } else {
             return bcCache[pos].value;
@@ -624,7 +625,7 @@ public class StringCache {
     public static class ByteEntry {
 
         public byte[] name = null;
-        public String enc = null;
+        public Charset charset = null;
         public String value = null;
 
         public String toString() {

Added: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/LocalStrings.properties?rev=1200601&view=auto
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/LocalStrings.properties (added)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/LocalStrings.properties Thu Nov 10 22:24:17 2011
@@ -0,0 +1,23 @@
+# 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.
+
+parameters.bytes=Start processing with input [{0}]
+parameters.copyFail=Failed to create copy of original parameter values for debug logging purposes
+parameters.decodeFail.debug=Character decoding failed. Parameter [{0}] with value [{1}] has been ignored.
+parameters.decodeFail.info=Character decoding failed. Parameter [{0}] with value [{1}] has been ignored. Note that the name and value quoted here may be corrupted due to the failed decoding. Use debug level logging to see the original, non-corrupted values.
+parameters.invalidChunk=Invalid chunk starting at byte [{0}] and ending at byte [{1}] with a value of [{2}] ignored
+parameters.maxCountFail=More than the maximum number of request parameters (GET plus POST) for a single request ([{0}]) were detected. Any parameters beyond this limit have been ignored. To change this limit, set the maxParameterCount attribute on the Connector.
+parameters.multipleDecodingFail=Character decoding failed. A total of [{0}] failures were detected but only the first was logged. Enable debug level logging for this logger to log all failures.
+parameters.noequal=Parameter starting at position [{0}] and ending at position [{1}] with a value of [{0}] was not followed by an '=' character

Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java Thu Nov 10 22:24:17 2011
@@ -19,228 +19,144 @@ package org.apache.tomcat.util.http;
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Enumeration;
-import java.util.Hashtable;
+import java.util.HashMap;
+import java.util.Map;
 
+import org.apache.tomcat.util.buf.B2CConverter;
 import org.apache.tomcat.util.buf.ByteChunk;
-import org.apache.tomcat.util.buf.CharChunk;
 import org.apache.tomcat.util.buf.MessageBytes;
 import org.apache.tomcat.util.buf.UDecoder;
-import org.apache.tomcat.util.collections.MultiMap;
+import org.apache.tomcat.util.res.StringManager;
 
 /**
  * 
  * @author Costin Manolache
  */
-public final class Parameters extends MultiMap {
+public final class Parameters {
 
-    
-    private static org.apache.juli.logging.Log log=
+    private static final org.apache.juli.logging.Log log =
         org.apache.juli.logging.LogFactory.getLog(Parameters.class );
-    
-    // Transition: we'll use the same Hashtable( String->String[] )
-    // for the beginning. When we are sure all accesses happen through
-    // this class - we can switch to MultiMap
-    private Hashtable<String,String[]> paramHashStringArray =
-        new Hashtable<String,String[]>();
+
+    protected static final StringManager sm =
+        StringManager.getManager("org.apache.tomcat.util.http");
+
+    private final HashMap<String,ArrayList<String>> paramHashValues =
+        new HashMap<String,ArrayList<String>>();
+
     private boolean didQueryParameters=false;
-    private boolean didMerge=false;
     
     MessageBytes queryMB;
-    MimeHeaders  headers;
 
     UDecoder urlDec;
     MessageBytes decodedQuery=MessageBytes.newInstance();
-    
-    public static final int INITIAL_SIZE=4;
-
-    // Garbage-less parameter merging.
-    // In a sub-request with parameters, the new parameters
-    // will be stored in child. When a getParameter happens,
-    // the 2 are merged togheter. The child will be altered
-    // to contain the merged values - the parent is allways the
-    // original request.
-    private Parameters child=null;
-    private Parameters parent=null;
-    private Parameters currentChild=null;
 
     String encoding=null;
     String queryStringEncoding=null;
     
+    private int limit = -1;
+    private int parameterCount = 0;
+
     /**
-     * 
+     * Is set to <code>true</code> if there were failures during parameter
+     * parsing.
      */
+    private boolean parseFailed = false;
+
     public Parameters() {
-        super( INITIAL_SIZE );
+        // NO-OP
     }
 
     public void setQuery( MessageBytes queryMB ) {
         this.queryMB=queryMB;
     }
 
-    public void setHeaders( MimeHeaders headers ) {
-        this.headers=headers;
+    public void setLimit(int limit) {
+        this.limit = limit;
+    }
+
+    public String getEncoding() {
+        return encoding;
     }
 
     public void setEncoding( String s ) {
         encoding=s;
-        if(debug>0) log( "Set encoding to " + s );
+        if(log.isDebugEnabled()) {
+            log.debug( "Set encoding to " + s );
+        }
     }
 
     public void setQueryStringEncoding( String s ) {
         queryStringEncoding=s;
-        if(debug>0) log( "Set query string encoding to " + s );
+        if(log.isDebugEnabled()) {
+            log.debug( "Set query string encoding to " + s );
+        }
+    }
+
+    public boolean isParseFailed() {
+        return parseFailed;
+    }
+
+    public void setParseFailed(boolean parseFailed) {
+        this.parseFailed = parseFailed;
     }
 
     public void recycle() {
-        super.recycle();
-        paramHashStringArray.clear();
+        parameterCount = 0;
+        paramHashValues.clear();
         didQueryParameters=false;
-        currentChild=null;
-        didMerge=false;
         encoding=null;
         decodedQuery.recycle();
+        parseFailed = false;
     }
-    
-    // -------------------- Sub-request support --------------------
 
-    public Parameters getCurrentSet() {
-        if( currentChild==null )
-            return this;
-        return currentChild;
-    }
-    
-    /** Create ( or reuse ) a child that will be used during a sub-request.
-        All future changes ( setting query string, adding parameters )
-        will affect the child ( the parent request is never changed ).
-        Both setters and getters will return the data from the deepest
-        child, merged with data from parents.
-    */
-    public void push() {
-        // We maintain a linked list, that will grow to the size of the
-        // longest include chain.
-        // The list has 2 points of interest:
-        // - request.parameters() is the original request and head,
-        // - request.parameters().currentChild() is the current set.
-        // The ->child and parent<- links are preserved ( currentChild is not
-        // the last in the list )
-        
-        // create a new element in the linked list
-        // note that we reuse the child, if any - pop will not
-        // set child to null !
-        if( currentChild==null ) {
-            currentChild=new Parameters();
-            currentChild.setURLDecoder( urlDec );
-            currentChild.parent=this;
-            return;
-        }
-        if( currentChild.child==null ) {
-            currentChild.child=new Parameters();
-            currentChild.setURLDecoder( urlDec );
-            currentChild.child.parent=currentChild;
-        } // it is not null if this object already had a child
-        // i.e. a deeper include() ( we keep it )
-
-        // the head will be the new element.
-        currentChild=currentChild.child;
-        currentChild.setEncoding( encoding );
-    }
-
-    /** Discard the last child. This happens when we return from a
-        sub-request and the parameters are locally modified.
-     */
-    public void pop() {
-        if( currentChild==null ) {
-            throw new RuntimeException( "Attempt to pop without a push" );
-        }
-        currentChild.recycle();
-        currentChild=currentChild.parent;
-        // don't remove the top.
-    }
-    
     // -------------------- Data access --------------------
     // Access to the current name/values, no side effect ( processing ).
-    // You must explicitely call handleQueryParameters and the post methods.
+    // You must explicitly call handleQueryParameters and the post methods.
     
-    // This is the original data representation ( hash of String->String[])
-
-    public void addParameterValues( String key, String[] newValues) {
-        if ( key==null ) return;
-        String values[];
-        if (paramHashStringArray.containsKey(key)) {
-            String oldValues[] = (String[])paramHashStringArray.get(key);
-            values = new String[oldValues.length + newValues.length];
-            for (int i = 0; i < oldValues.length; i++) {
-                values[i] = oldValues[i];
-            }
-            for (int i = 0; i < newValues.length; i++) {
-                values[i+ oldValues.length] = newValues[i];
-            }
+    public void addParameterValues(String key, String[] newValues) {
+        if (key == null) {
+            return;
+        }
+        ArrayList<String> values = paramHashValues.get(key);
+        if (values == null) {
+            values = new ArrayList<String>(newValues.length);
+            paramHashValues.put(key, values);
         } else {
-            values = newValues;
+            values.ensureCapacity(values.size() + newValues.length);
+        }
+        for (String newValue : newValues) {
+            values.add(newValue);
         }
-
-        paramHashStringArray.put(key, values);
     }
 
     public String[] getParameterValues(String name) {
         handleQueryParameters();
-        // sub-request
-        if( currentChild!=null ) {
-            currentChild.merge();
-            return (String[])currentChild.paramHashStringArray.get(name);
-        }
-
         // no "facade"
-        String values[]=(String[])paramHashStringArray.get(name);
-        return values;
-    }
- 
-    public Enumeration getParameterNames() {
-        handleQueryParameters();
-        // Slow - the original code
-        if( currentChild!=null ) {
-            currentChild.merge();
-            return currentChild.paramHashStringArray.keys();
+        ArrayList<String> values = paramHashValues.get(name);
+        if (values == null) {
+            return null;
         }
-
-        // merge in child
-        return paramHashStringArray.keys();
+        return values.toArray(new String[values.size()]);
     }
-
-    /** Combine the parameters from parent with our local ones
-     */
-    private void merge() {
-        // recursive
-        if( debug > 0 ) {
-            log("Before merging " + this + " " + parent + " " + didMerge );
-            log(  paramsAsString());
-        }
-        // Local parameters first - they take precedence as in spec.
+ 
+    public Enumeration<String> getParameterNames() {
         handleQueryParameters();
-
-        // we already merged with the parent
-        if( didMerge ) return;
-
-        // we are the top level
-        if( parent==null ) return;
-
-        // Add the parent props to the child ( lower precedence )
-        parent.merge();
-        Hashtable<String,String[]> parentProps=parent.paramHashStringArray;
-        merge2( paramHashStringArray , parentProps);
-        didMerge=true;
-        if(debug > 0 )
-            log("After " + paramsAsString());
+        return Collections.enumeration(paramHashValues.keySet());
     }
 
-
     // Shortcut.
     public String getParameter(String name ) {
-        String[] values = getParameterValues(name);
+        handleQueryParameters();
+        ArrayList<String> values = paramHashValues.get(name);
         if (values != null) {
-            if( values.length==0 ) return "";
-            return values[0];
+            if(values.size() == 0) {
+                return "";
+            }
+            return values.get(0);
         } else {
             return null;
         }
@@ -256,8 +172,10 @@ public final class Parameters extends Mu
         if( queryMB==null || queryMB.isNull() )
             return;
         
-        if( debug > 0  )
-            log( "Decoding query " + decodedQuery + " " + queryStringEncoding);
+        if(log.isDebugEnabled()) {
+            log.debug("Decoding query " + decodedQuery + " " +
+                    queryStringEncoding);
+        }
 
         try {
             decodedQuery.duplicate( queryMB );
@@ -268,63 +186,15 @@ public final class Parameters extends Mu
         processParameters( decodedQuery, queryStringEncoding );
     }
 
-    // --------------------
-    
-    /** Combine 2 hashtables into a new one.
-     *  ( two will be added to one ).
-     *  Used to combine child parameters ( RequestDispatcher's query )
-     *  with parent parameters ( original query or parent dispatcher )
-     */
-    private static void merge2(Hashtable<String,String[]> one,
-            Hashtable<String,String[]> two ) {
-        Enumeration e = two.keys();
-
-        while (e.hasMoreElements()) {
-            String name = (String) e.nextElement();
-            String[] oneValue = one.get(name);
-            String[] twoValue = two.get(name);
-            String[] combinedValue;
 
-            if (twoValue == null) {
-                continue;
-            } else {
-                if( oneValue==null ) {
-                    combinedValue = new String[twoValue.length];
-                    System.arraycopy(twoValue, 0, combinedValue,
-                                     0, twoValue.length);
-                } else {
-                    combinedValue = new String[oneValue.length +
-                                               twoValue.length];
-                    System.arraycopy(oneValue, 0, combinedValue, 0,
-                                     oneValue.length);
-                    System.arraycopy(twoValue, 0, combinedValue,
-                                     oneValue.length, twoValue.length);
-                }
-                one.put(name, combinedValue);
-            }
-        }
-    }
-
-    // incredibly inefficient data representation for parameters,
-    // until we test the new one
     private void addParam( String key, String value ) {
         if( key==null ) return;
-        String values[];
-        if (paramHashStringArray.containsKey(key)) {
-            String oldValues[] = (String[])paramHashStringArray.
-                get(key);
-            values = new String[oldValues.length + 1];
-            for (int i = 0; i < oldValues.length; i++) {
-                values[i] = oldValues[i];
-            }
-            values[oldValues.length] = value;
-        } else {
-            values = new String[1];
-            values[0] = value;
+        ArrayList<String> values = paramHashValues.get(key);
+        if (values == null) {
+            values = new ArrayList<String>(1);
+            paramHashValues.put(key, values);
         }
-        
-        
-        paramHashStringArray.put(key, values);
+        values.add(value);
     }
 
     public void setURLDecoder( UDecoder u ) {
@@ -332,134 +202,187 @@ public final class Parameters extends Mu
     }
 
     // -------------------- Parameter parsing --------------------
-
-    // This code is not used right now - it's the optimized version
-    // of the above.
-
     // we are called from a single thread - we can do it the hard way
     // if needed
     ByteChunk tmpName=new ByteChunk();
     ByteChunk tmpValue=new ByteChunk();
     private ByteChunk origName=new ByteChunk();
     private ByteChunk origValue=new ByteChunk();
-    CharChunk tmpNameC=new CharChunk(1024);
-    CharChunk tmpValueC=new CharChunk(1024);
     private static final String DEFAULT_ENCODING = "ISO-8859-1";
+    private static final Charset DEFAULT_CHARSET =
+        Charset.forName(DEFAULT_ENCODING);
+    
     
     public void processParameters( byte bytes[], int start, int len ) {
-        processParameters(bytes, start, len, encoding);
+        processParameters(bytes, start, len, getCharset(encoding));
     }
 
-    public void processParameters( byte bytes[], int start, int len, 
-                                   String enc ) {
-        int end=start+len;
-        int pos=start;
+    private void processParameters(byte bytes[], int start, int len,
+                                  Charset charset) {
         
         if(log.isDebugEnabled()) {
             try {
-                log.debug("Bytes: " +
-                        new String(bytes, start, len, DEFAULT_ENCODING));
-            } catch (UnsupportedEncodingException e) {
-                // Should never happen...
-                log.error("Unable to convert bytes", e);
+                log.debug(sm.getString("parameters.bytes",
+                        new String(bytes, start, len, DEFAULT_CHARSET.name())));
+            } catch (UnsupportedEncodingException uee) {
+                // Not possible. All JVMs must support ISO-8859-1
             }
         }
 
-        do {
-            boolean noEq=false;
-            int valStart=-1;
-            int valEnd=-1;
-            
-            int nameStart=pos;
-            int nameEnd=ByteChunk.indexOf(bytes, nameStart, end, '=' );
-            // Workaround for a&b&c encoding
-            int nameEnd2=ByteChunk.indexOf(bytes, nameStart, end, '&' );
-            if( (nameEnd2!=-1 ) &&
-                ( nameEnd==-1 || nameEnd > nameEnd2) ) {
-                nameEnd=nameEnd2;
-                noEq=true;
-                valStart=nameEnd;
-                valEnd=nameEnd;
-                if(log.isDebugEnabled()) {
-                    try {
-                        log.debug("no equal " + nameStart + " " + nameEnd + " " +
-                                new String(bytes, nameStart, nameEnd-nameStart,
-                                        DEFAULT_ENCODING) );
-                    } catch (UnsupportedEncodingException e) {
-                        // Should never happen...
-                        log.error("Unable to convert bytes", e);
-                    }
+        int decodeFailCount = 0;
+            
+        int pos = start;
+        int end = start + len;
+
+        while(pos < end) {
+            parameterCount ++;
+
+            if (limit > -1 && parameterCount >= limit) {
+                parseFailed = true;
+                log.warn(sm.getString("parameters.maxCountFail",
+                        Integer.valueOf(limit)));
+                break;
+            }
+            int nameStart = pos;
+            int nameEnd = -1;
+            int valueStart = -1;
+            int valueEnd = -1;
+
+            boolean parsingName = true;
+            boolean decodeName = false;
+            boolean decodeValue = false;
+            boolean parameterComplete = false;
+
+            do {
+                switch(bytes[pos]) {
+                    case '=':
+                        if (parsingName) {
+                            // Name finished. Value starts from next character
+                            nameEnd = pos;
+                            parsingName = false;
+                            valueStart = ++pos;
+                        } else {
+                            // Equals character in value
+                            pos++;
+                        }
+                        break;
+                    case '&':
+                        if (parsingName) {
+                            // Name finished. No value.
+                            nameEnd = pos;
+                        } else {
+                            // Value finished
+                            valueEnd  = pos;
+                        }
+                        parameterComplete = true;
+                        pos++;
+                        break;
+                    case '%':
+                        // Decoding required
+                        if (parsingName) {
+                            decodeName = true;
+                        } else {
+                            decodeValue = true;
+                        }
+                        pos ++;
+                        break;
+                    default:
+                        pos ++;
+                        break;
                 }
-            }
-            if( nameEnd== -1 ) 
-                nameEnd=end;
+            } while (!parameterComplete && pos < end);
 
-            if( ! noEq ) {
-                valStart= (nameEnd < end) ? nameEnd+1 : end;
-                valEnd=ByteChunk.indexOf(bytes, valStart, end, '&');
-                if( valEnd== -1 ) valEnd = (valStart < end) ? end : valStart;
+            if (pos == end) {
+                if (nameEnd == -1) {
+                    nameEnd = pos;
+                } else if (valueStart > -1 && valueEnd == -1){
+                    valueEnd = pos;
+                }
             }
             
-            pos=valEnd+1;
+            if (log.isDebugEnabled() && valueStart == -1) {
+                try {
+                    log.debug(sm.getString("parameters.noequal",
+                            Integer.valueOf(nameStart),
+                            Integer.valueOf(nameEnd),
+                            new String(bytes, nameStart, nameEnd-nameStart,
+                                    DEFAULT_CHARSET.name())));
+                } catch (UnsupportedEncodingException uee) {
+                    // Not possible. All JVMs must support ISO-8859-1
+                }
+            }
             
-            if( nameEnd<=nameStart ) {
+            if (nameEnd <= nameStart ) {
                 if (log.isInfoEnabled()) {
-                    StringBuilder msg = new StringBuilder("Parameters: Invalid chunk ");
-                    // No name eg ...&=xx&... will trigger this
-                    if (valEnd >= nameStart) {
-                        msg.append('\'');
+                    if (valueEnd >= nameStart && log.isDebugEnabled()) {
+                        String extract = null;
                         try {
-                            msg.append(new String(bytes, nameStart,
-                                    valEnd - nameStart, DEFAULT_ENCODING));
-                        } catch (UnsupportedEncodingException e) {
-                            // Should never happen...
-                            log.error("Unable to convert bytes", e);
+                            extract = new String(bytes, nameStart,
+                                    valueEnd - nameStart,
+                                    DEFAULT_CHARSET.name());
+                        } catch (UnsupportedEncodingException uee) {
+                            // Not possible. All JVMs must support ISO-8859-1
                         }
-                        msg.append("' ");
+                        log.info(sm.getString("parameters.invalidChunk",
+                                Integer.valueOf(nameStart),
+                                Integer.valueOf(valueEnd),
+                                extract));
+                    } else {
+                        log.info(sm.getString("parameters.invalidChunk",
+                                Integer.valueOf(nameStart),
+                                Integer.valueOf(nameEnd),
+                                null));
                     }
-                    msg.append("ignored.");
-                    log.info(msg);
                 }
+                parseFailed = true;
                 continue;
                 // invalid chunk - it's better to ignore
             }
-            tmpName.setBytes( bytes, nameStart, nameEnd-nameStart );
-            tmpValue.setBytes( bytes, valStart, valEnd-valStart );
             
+            tmpName.setBytes(bytes, nameStart, nameEnd - nameStart);
+            tmpValue.setBytes(bytes, valueStart, valueEnd - valueStart);
+
             // Take copies as if anything goes wrong originals will be
             // corrupted. This means original values can be logged.
             // For performance - only done for debug
             if (log.isDebugEnabled()) {
                 try {
-                    origName.append(bytes, nameStart, nameEnd-nameStart);
-                    origValue.append(bytes, valStart, valEnd-valStart);
+                    origName.append(bytes, nameStart, nameEnd - nameStart);
+                    origValue.append(bytes, valueStart, valueEnd - valueStart);
                 } catch (IOException ioe) {
                     // Should never happen...
-                    log.error("Error copying parameters", ioe);
+                    log.error(sm.getString("parameters.copyFail"), ioe);
                 }
             }
             
             try {
-                addParam( urlDecode(tmpName, enc), urlDecode(tmpValue, enc) );
+                String name;
+                String value;
+
+                if (decodeName) {
+                    urlDecode(tmpName);
+                }
+                tmpName.setCharset(charset);
+                name = tmpName.toString();
+
+                if (decodeValue) {
+                    urlDecode(tmpValue);
+                }
+                tmpValue.setCharset(charset);
+                value = tmpValue.toString();
+
+                addParam(name, value);
             } catch (IOException e) {
-                StringBuilder msg =
-                    new StringBuilder("Parameters: Character decoding failed.");
-                msg.append(" Parameter '");
-                if (log.isDebugEnabled()) {
-                    msg.append(origName.toString());
-                    msg.append("' with value '");
-                    msg.append(origValue.toString());
-                    msg.append("' has been ignored.");
-                    log.debug(msg, e);
-                } else if (log.isInfoEnabled()) {
-                    msg.append(tmpName.toString());
-                    msg.append("' with value '");
-                    msg.append(tmpValue.toString());
-                    msg.append("' has been ignored. Note that the name and ");
-                    msg.append("value quoted here may corrupted due to the ");
-                    msg.append("failed decoding. Use debug level logging to ");
-                    msg.append("see the original, non-corrupted values.");
-                    log.info(msg);
+                parseFailed = true;
+                decodeFailCount++;
+                if (decodeFailCount == 1 || log.isDebugEnabled()) {
+                    if (log.isDebugEnabled()) {
+                        log.debug(sm.getString("parameters.decodeFail.debug",
+                                origName.toString(), origValue.toString()), e);
+                    } else if (log.isInfoEnabled()) {
+                        log.info(sm.getString("parameters.decodeFail.info",
+                                tmpName.toString(), tmpValue.toString()), e);
+                    }
                 }
             }
 
@@ -470,217 +393,58 @@ public final class Parameters extends Mu
                 origName.recycle();
                 origValue.recycle();
             }
-        } while( pos<end );
+        }
+
+        if (decodeFailCount > 1 && !log.isDebugEnabled()) {
+            log.info(sm.getString("parameters.multipleDecodingFail",
+                    Integer.valueOf(decodeFailCount)));
+        }
     }
 
-    private String urlDecode(ByteChunk bc, String enc)
+    private void urlDecode(ByteChunk bc)
         throws IOException {
         if( urlDec==null ) {
             urlDec=new UDecoder();   
         }
         urlDec.convert(bc);
-        String result = null;
-        if (enc != null) {
-            bc.setEncoding(enc);
-            result = bc.toString();
-        } else {
-            CharChunk cc = tmpNameC;
-            int length = bc.getLength();
-            cc.allocate(length, -1);
-            // Default encoding: fast conversion
-            byte[] bbuf = bc.getBuffer();
-            char[] cbuf = cc.getBuffer();
-            int start = bc.getStart();
-            for (int i = 0; i < length; i++) {
-                cbuf[i] = (char) (bbuf[i + start] & 0xff);
-            }
-            cc.setChars(cbuf, 0, length);
-            result = cc.toString();
-            cc.recycle();
-        }
-        return result;
-    }
-
-    public void processParameters( char chars[], int start, int len ) {
-        int end=start+len;
-        int pos=start;
-        
-        if( debug>0 ) 
-            log( "Chars: " + new String( chars, start, len ));
-        do {
-            boolean noEq=false;
-            int nameStart=pos;
-            int valStart=-1;
-            int valEnd=-1;
-            
-            int nameEnd=CharChunk.indexOf(chars, nameStart, end, '=' );
-            int nameEnd2=CharChunk.indexOf(chars, nameStart, end, '&' );
-            if( (nameEnd2!=-1 ) &&
-                ( nameEnd==-1 || nameEnd > nameEnd2) ) {
-                nameEnd=nameEnd2;
-                noEq=true;
-                valStart=nameEnd;
-                valEnd=nameEnd;
-                if( debug>0) log("no equal " + nameStart + " " + nameEnd + " " + new String(chars, nameStart, nameEnd-nameStart) );
-            }
-            if( nameEnd== -1 ) nameEnd=end;
-            
-            if( ! noEq ) {
-                valStart= (nameEnd < end) ? nameEnd+1 : end;
-                valEnd=CharChunk.indexOf(chars, valStart, end, '&');
-                if( valEnd== -1 ) valEnd = (valStart < end) ? end : valStart;
-            }
-            
-            pos=valEnd+1;
-            
-            if( nameEnd<=nameStart ) {
-                continue;
-                // invalid chunk - no name, it's better to ignore
-                // XXX log it ?
-            }
-            
-            try {
-                tmpNameC.append( chars, nameStart, nameEnd-nameStart );
-                tmpValueC.append( chars, valStart, valEnd-valStart );
-
-                if( debug > 0 )
-                    log( tmpNameC + "= " + tmpValueC);
-
-                if( urlDec==null ) {
-                    urlDec=new UDecoder();   
-                }
-
-                urlDec.convert( tmpNameC );
-                urlDec.convert( tmpValueC );
-
-                if( debug > 0 )
-                    log( tmpNameC + "= " + tmpValueC);
-                
-                addParam( tmpNameC.toString(), tmpValueC.toString() );
-            } catch( IOException ex ) {
-                ex.printStackTrace();
-            }
-
-            tmpNameC.recycle();
-            tmpValueC.recycle();
-
-        } while( pos<end );
-    }
-    
-    public void processParameters( MessageBytes data ) {
-        processParameters(data, encoding);
     }
 
     public void processParameters( MessageBytes data, String encoding ) {
         if( data==null || data.isNull() || data.getLength() <= 0 ) return;
 
-        if( data.getType() == MessageBytes.T_BYTES ) {
-            ByteChunk bc=data.getByteChunk();
-            processParameters( bc.getBytes(), bc.getOffset(),
-                               bc.getLength(), encoding);
-        } else {
-            if (data.getType()!= MessageBytes.T_CHARS ) 
-                data.toChars();
-            CharChunk cc=data.getCharChunk();
-            processParameters( cc.getChars(), cc.getOffset(),
-                               cc.getLength());
+        if( data.getType() != MessageBytes.T_BYTES ) {
+            data.toBytes();
         }
+        ByteChunk bc=data.getByteChunk();
+        processParameters( bc.getBytes(), bc.getOffset(),
+                           bc.getLength(), getCharset(encoding));
     }
 
-    /** Debug purpose
-     */
-    public String paramsAsString() {
-        StringBuffer sb=new StringBuffer();
-        Enumeration en= paramHashStringArray.keys();
-        while( en.hasMoreElements() ) {
-            String k=(String)en.nextElement();
-            sb.append( k ).append("=");
-            String v[]=(String[])paramHashStringArray.get( k );
-            for( int i=0; i<v.length; i++ )
-                sb.append( v[i] ).append(",");
-            sb.append("\n");
+    private Charset getCharset(String encoding) {
+        if (encoding == null) {
+            return DEFAULT_CHARSET;
+        }
+        try {
+            return B2CConverter.getCharset(encoding);
+        } catch (UnsupportedEncodingException e) {
+            return DEFAULT_CHARSET;
         }
-        return sb.toString();
     }
 
-    private static int debug=0;
-    private void log(String s ) {
-        if (log.isDebugEnabled())
-            log.debug("Parameters: " + s );
-    }
-   
-    // -------------------- Old code, needs rewrite --------------------
-    
-    /** Used by RequestDispatcher
+    /** 
+     * Debug purpose
      */
-    public void processParameters( String str ) {
-        int end=str.length();
-        int pos=0;
-        if( debug > 0)
-            log("String: " + str );
-        
-        do {
-            boolean noEq=false;
-            int valStart=-1;
-            int valEnd=-1;
-            
-            int nameStart=pos;
-            int nameEnd=str.indexOf('=', nameStart );
-            int nameEnd2=str.indexOf('&', nameStart );
-            if( nameEnd2== -1 ) nameEnd2=end;
-            if( (nameEnd2!=-1 ) &&
-                ( nameEnd==-1 || nameEnd > nameEnd2) ) {
-                nameEnd=nameEnd2;
-                noEq=true;
-                valStart=nameEnd;
-                valEnd=nameEnd;
-                if( debug>0) log("no equal " + nameStart + " " + nameEnd + " " + str.substring(nameStart, nameEnd) );
-            }
-
-            if( nameEnd== -1 ) nameEnd=end;
-
-            if( ! noEq ) {
-                valStart=nameEnd+1;
-                valEnd=str.indexOf('&', valStart);
-                if( valEnd== -1 ) valEnd = (valStart < end) ? end : valStart;
-            }
-            
-            pos=valEnd+1;
-            
-            if( nameEnd<=nameStart ) {
-                continue;
-            }
-            if( debug>0)
-                log( "XXX " + nameStart + " " + nameEnd + " "
-                     + valStart + " " + valEnd );
-            
-            try {
-                tmpNameC.append(str, nameStart, nameEnd-nameStart );
-                tmpValueC.append(str, valStart, valEnd-valStart );
-            
-                if( debug > 0 )
-                    log( tmpNameC + "= " + tmpValueC);
-
-                if( urlDec==null ) {
-                    urlDec=new UDecoder();   
-                }
-
-                urlDec.convert( tmpNameC );
-                urlDec.convert( tmpValueC );
-
-                if( debug > 0 )
-                    log( tmpNameC + "= " + tmpValueC);
-                
-                addParam( tmpNameC.toString(), tmpValueC.toString() );
-            } catch( IOException ex ) {
-                ex.printStackTrace();
+    public String paramsAsString() {
+        StringBuilder sb = new StringBuilder();
+        for (Map.Entry<String, ArrayList<String>> e : paramHashValues.entrySet()) {
+            sb.append(e.getKey()).append('=');
+            ArrayList<String> values = e.getValue();
+            for (String value : values) {
+                sb.append(value).append(',');
             }
-
-            tmpNameC.recycle();
-            tmpValueC.recycle();
-
-        } while( pos<end );
+            sb.append('\n');
+        }
+        return sb.toString();
     }
 
-
 }

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Thu Nov 10 22:24:17 2011
@@ -122,6 +122,17 @@
         separate method and expose this new method <code>isAllowed</code>
         through JMX. (kkolinko)
       </update>
+      <add>
+        Improve performance of parameter processing for GET and POST requests.
+        Also add an option to limit the maximum number of parameters processed
+        per request. This defaults to 10000. Excessive parameters are ignored.
+        Note that <code>FailedRequestFilter</code> can be used to reject the
+        request if some parameters were ignored. (markt/kkolinko)
+      </add>
+      <add>
+        New filter <code>FailedRequestFilter</code> that will reject a request
+        if there were errors during HTTP parameter parsing. (kkolinko)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Coyote">

Modified: tomcat/tc6.0.x/trunk/webapps/docs/config/ajp.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/config/ajp.xml?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/config/ajp.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/config/ajp.xml Thu Nov 10 22:24:17 2011
@@ -93,6 +93,14 @@
       By default, DNS lookups are enabled.</p>
     </attribute>
 
+    <attribute name="maxParameterCount" required="false">
+      <p>The maximum number of parameters (GET plus POST) which will be
+      automatically parsed by the container. A value of less than 0 means no
+      limit. If not specified, a default of 10000 is used. Note that
+      <code>FailedRequestFilter</code> <a href="filter.html">filter</a> can be
+      used to reject requests that hit the limit.</p>
+    </attribute>
+
     <attribute name="maxPostSize" required="false">
       <p>The maximum size in bytes of the POST which will be handled by
       the container FORM URL parameter parsing. The feature can be disabled by

Modified: tomcat/tc6.0.x/trunk/webapps/docs/config/filter.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/config/filter.xml?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/config/filter.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/config/filter.xml Thu Nov 10 22:24:17 2011
@@ -112,6 +112,46 @@
 </section>
 
 
+<section name="Failed Request Filter">
+
+  <subsection name="Introduction">
+
+    <p>This filter triggers parameters parsing in a request and rejects the
+    request if some parameters were skipped during parameter parsing because
+    of parsing errors or request size limitations (such as
+    <code>maxParameterCount</code> attribute in a
+    <a href="http.html">Connector</a>).
+    This filter can be used to ensure that none parameter values submitted by
+    client are lost.</p>
+
+    <p>Note that parameter parsing may consume the body of an HTTP request, so
+    caution is needed if the servlet protected by this filter uses
+    <code>request.getInputStream()</code> or <code>request.getReader()</code>
+    calls. In general the risk of breaking a web application by adding this
+    filter is not so high, because parameter parsing does check content type
+    of the request before consuming the request body.</p>
+
+    <p>The request is rejected with HTTP status code 400 (Bad Request).</p>
+
+  </subsection>
+
+  <subsection name="Filter Class Name">
+
+    <p>The filter class name for the Failed Request Filter is
+    <strong><code>org.apache.catalina.filters.FailedRequestFilter</code>
+    </strong>.</p>
+
+  </subsection>
+
+  <subsection name="Initialisation parameters">
+
+    <p>The Failed Request Filter does not support any initialization parameters.</p>
+
+  </subsection>
+
+</section>
+
+
 </body>
 
 

Modified: tomcat/tc6.0.x/trunk/webapps/docs/config/http.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/config/http.xml?rev=1200601&r1=1200600&r2=1200601&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/config/http.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/config/http.xml Thu Nov 10 22:24:17 2011
@@ -100,6 +100,14 @@
       By default, DNS lookups are enabled.</p>
     </attribute>
 
+    <attribute name="maxParameterCount" required="false">
+      <p>The maximum number of parameters (GET plus POST) which will be
+      automatically parsed by the container. A value of less than 0 means no
+      limit. If not specified, a default of 10000 is used. Note that
+      <code>FailedRequestFilter</code> <a href="filter.html">filter</a> can be
+      used to reject requests that hit the limit.</p>
+    </attribute>
+
     <attribute name="maxPostSize" required="false">
       <p>The maximum size in bytes of the POST which will be handled by
       the container FORM URL parameter parsing. The limit can be disabled by



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