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