Fixed issue: NETTY-325 (QueryStringDecoder doesn't properly handle missing query string values and other corner cases)

* Rewrote QueryStringDecoder based on Benoit's work
* Added a test case for QueryStringDecoder
This commit is contained in:
Trustin Lee 2010-06-14 10:57:48 +00:00
parent 5097009f8c
commit 6d0fb256c6
2 changed files with 156 additions and 40 deletions

View File

@ -21,11 +21,10 @@ import java.net.URLDecoder;
import java.nio.charset.Charset;
import java.nio.charset.UnsupportedCharsetException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
* Splits an HTTP query string into a path string and key-value parameter pairs.
@ -48,12 +47,10 @@ import java.util.regex.Pattern;
*/
public class QueryStringDecoder {
private static final Pattern PARAM_PATTERN = Pattern.compile("([^=]*)=([^&]*)&*");
private final Charset charset;
private final String uri;
private String path;
private final Map<String, List<String>> params = new HashMap<String, List<String>>();
private Map<String, List<String>> params;
/**
* Creates a new decoder that decodes the specified URI. The decoder will
@ -123,13 +120,13 @@ public class QueryStringDecoder {
* Returns the decoded path string of the URI.
*/
public String getPath() {
//decode lazily
if(path == null) {
if(uri.contains("?")) {
decode();
if (path == null) {
int pathEndPos = uri.indexOf('?');
if (pathEndPos < 0) {
path = uri;
}
else {
path = uri;
return path = uri.substring(0, pathEndPos);
}
}
return path;
@ -139,42 +136,54 @@ public class QueryStringDecoder {
* Returns the decoded key-value parameter pairs of the URI.
*/
public Map<String, List<String>> getParameters() {
if(path == null){
if(uri.contains("?")) {
decode();
}
else {
path = uri;
if (params == null) {
int pathLength = getPath().length();
if (uri.length() == pathLength) {
return Collections.emptyMap();
}
params = decodeParams(uri.substring(pathLength + 1));
}
return params;
}
private void decode() {
int pathEndPos = uri.indexOf('?');
if (pathEndPos < 0) {
path = uri;
} else {
path = uri.substring(0, pathEndPos);
decodeParams(uri.substring(pathEndPos + 1));
}
}
private void decodeParams(String s) {
Matcher m = PARAM_PATTERN.matcher(s);
int pos = 0;
while (m.find(pos)) {
pos = m.end();
String key = decodeComponent(m.group(1), charset);
String value = decodeComponent(m.group(2), charset);
List<String> values = params.get(key);
if(values == null) {
values = new ArrayList<String>();
params.put(key,values);
private Map<String, List<String>> decodeParams(String s) {
Map<String, List<String>> params = new LinkedHashMap<String, List<String>>();
String name = null;
int pos = 0; // Beginning of the unprocessed region
int i; // End of the unprocessed region
char c = 0; // Current character
for (i = 0; i < s.length(); i++) {
c = s.charAt(i);
if (c == '=' && name == null) {
if (pos != i) {
name = decodeComponent(s.substring(pos, i), charset);
}
pos = i + 1;
} else if (c == '&') {
if (name == null && pos != i) {
// We haven't seen an `=' so far but moved forward.
// Must be a param of the form '&a&' so add it with
// an empty value.
addParam(params, decodeComponent(s.substring(pos, i), charset), "");
} else if (name != null) {
addParam(params, name, decodeComponent(s.substring(pos, i), charset));
name = null;
}
pos = i + 1;
}
values.add(value);
}
if (pos != i) { // Are there characters we haven't dealt with?
if (name == null) { // Yes and we haven't seen any `='.
addParam(params, decodeComponent(s.substring(pos, i), charset), "");
} else { // Yes and this must be the last value.
addParam(params, name, decodeComponent(s.substring(pos, i), charset));
}
} else if (name != null) { // Have we seen a name without value?
addParam(params, name, "");
}
return params;
}
private static String decodeComponent(String s, Charset charset) {
@ -188,4 +197,13 @@ public class QueryStringDecoder {
throw new UnsupportedCharsetException(charset.name());
}
}
private static void addParam(Map<String, List<String>> params, String name, String value) {
List<String> values = params.get(name);
if (values == null) {
values = new ArrayList<String>(1); // Often there's only 1 value.
params.put(name, values);
}
values.add(value);
}
}

View File

@ -0,0 +1,98 @@
/*
* Copyright 2010 Red Hat, Inc.
*
* Red Hat 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.jboss.netty.handler.codec.http;
import org.jboss.netty.util.CharsetUtil;
import org.junit.Assert;
import org.junit.Test;
/**
* @author <a href="http://www.jboss.org/netty/">The Netty Project</a>
* @author <a href="http://gleamynode.net/">Trustin Lee</a>
* @version $Rev$, $Date$
*/
public class QueryStringDecoderTest {
@Test
public void testBasic() throws Exception {
QueryStringDecoder d;
d = new QueryStringDecoder("/foo?a=b=c");
Assert.assertEquals("/foo", d.getPath());
Assert.assertEquals(1, d.getParameters().size());
Assert.assertEquals(1, d.getParameters().get("a").size());
Assert.assertEquals("b=c", d.getParameters().get("a").get(0));
d = new QueryStringDecoder("/foo?a=1&a=2");
Assert.assertEquals("/foo", d.getPath());
Assert.assertEquals(1, d.getParameters().size());
Assert.assertEquals(2, d.getParameters().get("a").size());
Assert.assertEquals("1", d.getParameters().get("a").get(0));
Assert.assertEquals("2", d.getParameters().get("a").get(1));
d = new QueryStringDecoder("/foo?a=&a=2");
Assert.assertEquals("/foo", d.getPath());
Assert.assertEquals(1, d.getParameters().size());
Assert.assertEquals(2, d.getParameters().get("a").size());
Assert.assertEquals("", d.getParameters().get("a").get(0));
Assert.assertEquals("2", d.getParameters().get("a").get(1));
d = new QueryStringDecoder("/foo?a=1&a=");
Assert.assertEquals("/foo", d.getPath());
Assert.assertEquals(1, d.getParameters().size());
Assert.assertEquals(2, d.getParameters().get("a").size());
Assert.assertEquals("1", d.getParameters().get("a").get(0));
Assert.assertEquals("", d.getParameters().get("a").get(1));
d = new QueryStringDecoder("/foo?a=1&a=&a=");
Assert.assertEquals("/foo", d.getPath());
Assert.assertEquals(1, d.getParameters().size());
Assert.assertEquals(3, d.getParameters().get("a").size());
Assert.assertEquals("1", d.getParameters().get("a").get(0));
Assert.assertEquals("", d.getParameters().get("a").get(1));
Assert.assertEquals("", d.getParameters().get("a").get(2));
}
@Test
public void testExotic() throws Exception {
assertQueryString("", "");
assertQueryString("foo", "foo");
assertQueryString("/foo", "/foo");
assertQueryString("?a=", "?a");
assertQueryString("foo?a=", "foo?a");
assertQueryString("/foo?a=", "/foo?a");
assertQueryString("/foo?a=", "/foo?a&");
assertQueryString("/foo?a=", "/foo?&a");
assertQueryString("/foo?a=", "/foo?&a&");
assertQueryString("/foo?a=", "/foo?&=a");
assertQueryString("/foo?a=", "/foo?=a&");
assertQueryString("/foo?a=", "/foo?a=&");
assertQueryString("/foo?a=b&c=d", "/foo?a=b&&c=d");
assertQueryString("/foo?a=b&c=d", "/foo?a=b&=&c=d");
assertQueryString("/foo?a=b&c=d", "/foo?a=b&==&c=d");
assertQueryString("/foo?a=b&c=&x=y", "/foo?a=b&c&x=y");
assertQueryString("/foo?a=", "/foo?a=");
assertQueryString("/foo?a=", "/foo?&a=");
assertQueryString("/foo?a=b&c=d", "/foo?a=b&c=d");
assertQueryString("/foo?a=1&a=&a=", "/foo?a=1&a&a=");
}
private static void assertQueryString(String expected, String actual) {
QueryStringDecoder ed = new QueryStringDecoder(expected, CharsetUtil.UTF_8);
QueryStringDecoder ad = new QueryStringDecoder(actual, CharsetUtil.UTF_8);
Assert.assertEquals(ed.getPath(), ad.getPath());
Assert.assertEquals(ed.getParameters(), ad.getParameters());
}
}