Author: markt
Date: Sat Oct 29 16:16:18 2011
New Revision: 1194917
URL: http://svn.apache.org/viewvc?rev=1194917&view=rev
Log:
Review comments from kkolinko for parameter parsing improvements
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java
tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/TestParameters.java
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Sat Oct 29 16:16:18 2011
@@ -1 +1 @@
-/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166693,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172556,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096
,1173241,1173256,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175690,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245,1177850,1177862,1177978,1178209,1178228,1178233,1178449,1178542,1178681,1178684,1178721,1179268,1179274,1180261,1180865,1180891,1180894,1180907,1181028,1181123,1181125,1181136,1181291,1181743,1182796,1183078,1183105,1183142,1183328,1183339-1183340,1183492-1183494,1183605,1184917,1184919,1185018,1185020,1185200,1185588,1185626,1185756,1185758,1186011,1186042-1186045,1186104,1186123,1186137,1186153,1186254,1186257,1186377-1186379,1186479-1186480,1186712,1186743,1186750,1186763,1186890-1186892,1186894,1186949,1187018,1187027-1187028,1187381,1187755,1187
775,1187827,1188301,1188303-1188305,1188399,1188822,1188930-1188931,1189116,1189129,1189183,1189240,1189256,1189386,1189413-1189414,1189477,1189685,1189805,1189857,1189864,1189882,1190034,1190185,1190279,1190339,1190371,1190388-1190389,1190474,1190481
+/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166693,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172556,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096
,1173241,1173256,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175690,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245,1177850,1177862,1177978,1178209,1178228,1178233,1178449,1178542,1178681,1178684,1178721,1179268,1179274,1180261,1180865,1180891,1180894,1180907,1181028,1181123,1181125,1181136,1181291,1181743,1182796,1183078,1183105,1183142,1183328,1183339-1183340,1183492-1183494,1183605,1184917,1184919,1185018,1185020,1185200,1185588,1185626,1185756,1185758,1186011,1186042-1186045,1186104,1186123,1186137,1186153,1186254,1186257,1186377-1186379,1186479-1186480,1186712,1186743,1186750,1186763,1186890-1186892,1186894,1186949,1187018,1187027-1187028,1187381,1187755,1187
775,1187827,1188301,1188303-1188305,1188399,1188822,1188930-1188931,1189116,1189129,1189183,1189240,1189256,1189386,1189413-1189414,1189477,1189685,1189805,1189857,1189864,1189882,1190034,1190185,1190279,1190339,1190371,1190388-1190389,1190474,1190481,1194915
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java?rev=1194917&r1=1194916&r2=1194917&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java Sat
Oct 29 16:16:18 2011
@@ -106,14 +106,22 @@ public class B2CConverter {
static final int BUFFER_SIZE=8192;
char result[]=new char[BUFFER_SIZE];
- public void convert( ByteChunk bb, CharChunk cb, int limit)
+ /**
+ * 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
{
iis.setByteChunk( bb );
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/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java?rev=1194917&r1=1194916&r2=1194917&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java Sat Oct
29 16:16:18 2011
@@ -521,7 +521,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/tc7.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java?rev=1194917&r1=1194916&r2=1194917&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/Parameters.java Sat
Oct 29 16:16:18 2011
@@ -101,18 +101,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 ) return;
+ 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);
}
@@ -170,8 +170,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;
ArrayList<String> values;
@@ -197,7 +196,7 @@ public final class Parameters {
private 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);
@@ -205,7 +204,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()) {
@@ -404,19 +403,20 @@ 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("\n");
+ sb.append(value).append(',');
}
+ sb.append('\n');
}
return sb.toString();
}
Modified:
tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/TestParameters.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/TestParameters.java?rev=1194917&r1=1194916&r2=1194917&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/TestParameters.java
(original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/TestParameters.java
Sat Oct 29 16:16:18 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: [email protected]
For additional commands, e-mail: [email protected]
