Repository: netty/netty
Issue: QueryStringDecoder#path should decode the path info

Netty version: 4.1.3.Final, 4.0.39.Final

For example, given "foo%20bar", it should return "foo bar", not "foo%20bar".


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/QueryStringDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/QueryStringDecoder.java
index fdffb2e217e..b4996267788 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/QueryStringDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/QueryStringDecoder.java
@@ -178,14 +178,10 @@ public String uri() {
     public String path() {
         if (path == null) {
             if (!hasPath) {
-                return path = "";
-            }
-
-            int pathEndPos = uri.indexOf('?');
-            if (pathEndPos < 0) {
-                path = uri;
+                path = "";
             } else {
-                return path = uri.substring(0, pathEndPos);
+                int pathEndPos = uri.indexOf('?');
+                path = decodeComponent(pathEndPos < 0 ? uri : uri.substring(0, pathEndPos), this.charset);
             }
         }
         return path;
@@ -197,16 +193,18 @@ public String path() {
     public Map<String, List<String>> parameters() {
         if (params == null) {
             if (hasPath) {
-                int pathLength = path().length();
-                if (uri.length() == pathLength) {
-                    return Collections.emptyMap();
+                int pathEndPos = uri.indexOf('?');
+                if (pathEndPos >= 0 && pathEndPos < uri.length() - 1) {
+                    decodeParams(uri.substring(pathEndPos + 1));
+                } else {
+                    params = Collections.emptyMap();
                 }
-                decodeParams(uri.substring(pathLength + 1));
             } else {
                 if (uri.isEmpty()) {
-                    return Collections.emptyMap();
+                    params = Collections.emptyMap();
+                } else {
+                    decodeParams(uri);
                 }
-                decodeParams(uri);
             }
         }
         return params;


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/QueryStringDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/QueryStringDecoderTest.java
index 49b069f6dba..caee98a0806 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/QueryStringDecoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/QueryStringDecoderTest.java
@@ -38,6 +38,14 @@ public void testBasicUris() throws URISyntaxException {
     public void testBasic() throws Exception {
         QueryStringDecoder d;
 
+        d = new QueryStringDecoder("/foo");
+        Assert.assertEquals("/foo", d.path());
+        Assert.assertEquals(0, d.parameters().size());
+
+        d = new QueryStringDecoder("/foo%20bar");
+        Assert.assertEquals("/foo bar", d.path());
+        Assert.assertEquals(0, d.parameters().size());
+
         d = new QueryStringDecoder("/foo?a=b=c");
         Assert.assertEquals("/foo", d.path());
         Assert.assertEquals(1, d.parameters().size());
@@ -51,6 +59,13 @@ public void testBasic() throws Exception {
         Assert.assertEquals("1", d.parameters().get("a").get(0));
         Assert.assertEquals("2", d.parameters().get("a").get(1));
 
+        d = new QueryStringDecoder("/foo%20bar?a=1&a=2");
+        Assert.assertEquals("/foo bar", d.path());
+        Assert.assertEquals(1, d.parameters().size());
+        Assert.assertEquals(2, d.parameters().get("a").size());
+        Assert.assertEquals("1", d.parameters().get("a").get(0));
+        Assert.assertEquals("2", d.parameters().get("a").get(1));
+
         d = new QueryStringDecoder("/foo?a=&a=2");
         Assert.assertEquals("/foo", d.path());
         Assert.assertEquals(1, d.parameters().size());
@@ -85,6 +100,8 @@ public void testBasic() throws Exception {
     public void testExotic() throws Exception {
         assertQueryString("", "");
         assertQueryString("foo", "foo");
+        assertQueryString("foo", "foo?");
+        assertQueryString("/foo", "/foo?");
         assertQueryString("/foo", "/foo");
         assertQueryString("?a=", "?a");
         assertQueryString("foo?a=", "foo?a");