Author: markt
Date: Sat Oct 29 16:11:02 2011
New Revision: 1194915

URL: http://svn.apache.org/viewvc?rev=1194915&view=rev
Log:
Review comments from kkolinko for parameter parsing improvements

Modified:
    tomcat/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java
    tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.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/tomcat/util/buf/B2CConverter.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java?rev=1194915&r1=1194914&r2=1194915&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java Sat Oct 29 
16:11:02 2011
@@ -104,6 +104,14 @@ public class B2CConverter {
     static final int BUFFER_SIZE=8192;
     char result[]=new char[BUFFER_SIZE];
 
+    /**
+     * 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
     {
@@ -111,7 +119,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/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java?rev=1194915&r1=1194914&r2=1194915&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java Sat Oct 29 
16:11:02 2011
@@ -535,7 +535,7 @@ public final class ByteChunk implements 
         // 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());
+        return new String(cb.array(), cb.arrayOffset(), cb.length());
     }
 
     public int getInt()

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=1194915&r1=1194914&r2=1194915&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java Sat Oct 29 
16:11:02 2011
@@ -99,20 +99,18 @@ public final class Parameters {
     // Access to the current name/values, no side effect ( processing ).
     // 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 ) {
+    public void addParameterValues(String key, String[] newValues) {
+        if (key == null) {
             return;
         }
         ArrayList<String> values;
         if (paramHashValues.containsKey(key)) {
-             values = paramHashValues.get(key);
+            values = paramHashValues.get(key);
+            values.ensureCapacity(values.size() + newValues.length);
         } else {
-            values = new ArrayList<String>(1);
+            values = new ArrayList<String>(newValues.length);
             paramHashValues.put(key, values);
         }
-        values.ensureCapacity(values.size() + newValues.length);
         for (String newValue : newValues) {
             values.add(newValue);
         }
@@ -173,8 +171,7 @@ public final class Parameters {
         processParameters( decodedQuery, queryStringEncoding );
     }
 
-    // incredibly inefficient data representation for parameters,
-    // until we test the new one
+
     private void addParam( String key, String value ) {
         if( key==null ) {
             return;
@@ -202,7 +199,7 @@ public final class Parameters {
     private final ByteChunk origValue=new ByteChunk();
     CharChunk tmpNameC=new CharChunk(1024);
     public static final String DEFAULT_ENCODING = "ISO-8859-1";
-    public static final Charset DEFAULT_CHARSET =
+    private static final Charset DEFAULT_CHARSET =
         Charset.forName(DEFAULT_ENCODING);
 
 
@@ -210,7 +207,7 @@ public final class Parameters {
         processParameters(bytes, start, len, getCharset(encoding));
     }
 
-    public void processParameters(byte bytes[], int start, int len,
+    private void processParameters(byte bytes[], int start, int len,
                                   Charset charset) {
 
         if(log.isDebugEnabled()) {
@@ -411,21 +408,21 @@ public final class Parameters {
         }
     }
 
-    /** Debug purpose
+    /**
+     * Debug purpose
      */
     public String paramsAsString() {
-        StringBuilder sb=new StringBuilder();
-        Enumeration<String> en= paramHashValues.keys();
-        while( en.hasMoreElements() ) {
+        StringBuilder sb = new StringBuilder();
+        Enumeration<String> en = paramHashValues.keys();
+        while (en.hasMoreElements()) {
             String k = en.nextElement();
-            sb.append( k ).append("=");
+            sb.append(k).append('=');
             ArrayList<String> values = paramHashValues.get(k);
             for(String value : values) {
-                sb.append(value).append(",");
+                sb.append(value).append(',');
             }
-            sb.append("\n");
+            sb.append('\n');
         }
         return sb.toString();
     }
-
 }

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=1194915&r1=1194914&r2=1194915&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/http/TestParameters.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/http/TestParameters.java Sat Oct 
29 16:11:02 2011
@@ -20,6 +20,8 @@ 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.assertNull;
 import static org.junit.Assert.assertTrue;
 import org.junit.Test;
 
@@ -37,6 +39,8 @@ public class TestParameters {
         new Parameter("foo4", "");
     private static final Parameter EMPTY =
         new Parameter("");
+    private static final Parameter UTF8 =
+            new Parameter("\ufb6b\ufb6a\ufb72", "\uffee\uffeb\uffe2");
 
     @Test
     public void testProcessParametersByteArrayIntInt() {
@@ -45,16 +49,19 @@ public class TestParameters {
         doTestProcessParametersByteArrayIntInt(NO_VALUE);
         doTestProcessParametersByteArrayIntInt(EMPTY_VALUE);
         doTestProcessParametersByteArrayIntInt(EMPTY);
+        doTestProcessParametersByteArrayIntInt(UTF8);
         doTestProcessParametersByteArrayIntInt(
-                SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY);
+                SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY, UTF8);
         doTestProcessParametersByteArrayIntInt(
-                SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY, SIMPLE);
+                SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY, UTF8, SIMPLE);
         doTestProcessParametersByteArrayIntInt(
-                NO_VALUE, EMPTY_VALUE, EMPTY, SIMPLE, SIMPLE_MULTIPLE);
+                NO_VALUE, EMPTY_VALUE, EMPTY, UTF8, SIMPLE, SIMPLE_MULTIPLE);
         doTestProcessParametersByteArrayIntInt(
-                EMPTY_VALUE, EMPTY, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE);
+                EMPTY_VALUE, EMPTY, UTF8, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE);
         doTestProcessParametersByteArrayIntInt(
-                EMPTY, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE);
+                EMPTY, UTF8, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE);
+        doTestProcessParametersByteArrayIntInt(
+                UTF8, SIMPLE, SIMPLE_MULTIPLE, NO_VALUE, EMPTY_VALUE, EMPTY);
     }
 
     // Make sure the inner Parameter class behaves correctly
@@ -68,6 +75,7 @@ public class TestParameters {
 
     private long doTestProcessParametersByteArrayIntInt(
             Parameter... parameters) {
+
         // Build the byte array
         StringBuilder input = new StringBuilder();
         boolean first = true;
@@ -93,6 +101,60 @@ public class TestParameters {
         return end - start;
     }
 
+    @Test
+    public void testNonExistantParameter() {
+        Parameters p = new Parameters();
+
+        String value = p.getParameter("foo");
+        assertNull(value);
+
+        Enumeration<String> names = p.getParameterNames();
+        assertFalse(names.hasMoreElements());
+
+        String[] values = p.getParameterValues("foo");
+        assertNull(values);
+    }
+
+
+    @Test
+    public void testAddParameters() {
+        Parameters p = new Parameters();
+
+        // Empty at this point
+        Enumeration<String> names = p.getParameterNames();
+        assertFalse(names.hasMoreElements());
+        String[] values = p.getParameterValues("foo");
+        assertNull(values);
+
+        // Add a parameter with two values
+        p.addParameterValues("foo", new String[] {"value1", "value2"});
+
+        names = p.getParameterNames();
+        assertTrue(names.hasMoreElements());
+        assertEquals("foo", names.nextElement());
+        assertFalse(names.hasMoreElements());
+
+        values = p.getParameterValues("foo");
+        assertEquals(2, values.length);
+        assertEquals("value1", values[0]);
+        assertEquals("value2", values[1]);
+
+        // Add two more values
+        p.addParameterValues("foo", new String[] {"value3", "value4"});
+
+        names = p.getParameterNames();
+        assertTrue(names.hasMoreElements());
+        assertEquals("foo", names.nextElement());
+        assertFalse(names.hasMoreElements());
+
+        values = p.getParameterValues("foo");
+        assertEquals(4, values.length);
+        assertEquals("value1", values[0]);
+        assertEquals("value2", values[1]);
+        assertEquals("value3", values[2]);
+        assertEquals("value4", values[3]);
+    }
+
     private void validateParameters(Parameter[] parameters, Parameters p) {
         Enumeration<String> names = p.getParameterNames();
 
@@ -146,7 +208,7 @@ public class TestParameters {
             StringBuilder result = new StringBuilder();
             boolean first = true;
             if (values.length == 0) {
-                return name;
+                return uencoder.encodeURL(name);
             }
             for (String value : values) {
                 if (first) {
@@ -159,7 +221,7 @@ public class TestParameters {
                 }
                 if (value != null) {
                     result.append('=');
-                    result.append(value);
+                    result.append(uencoder.encodeURL(value));
                 }
             }
 



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

Reply via email to