From b548579a3d72da91df7be7fdc73263c28196517d Mon Sep 17 00:00:00 2001 From: Ngoc Dao Date: Wed, 27 Jul 2016 15:19:10 +1000 Subject: [PATCH] Fix #5590 QueryStringDecoder#path should decode the path info Motivation: Currently, QueryStringDecoder#path simply returns the path info as is, without decoding it as the Javadoc states. Modifications: * Make QueryStringDecoder#path decode the path info. * Add tests to QueryStringDecoderTest. Result: QueryStringDecoder#path now decodes the path info as expected. --- .../codec/http/QueryStringDecoder.java | 24 +++++++++---------- .../codec/http/QueryStringDecoderTest.java | 17 +++++++++++++ 2 files changed, 28 insertions(+), 13 deletions(-) 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 fdffb2e217..b499626778 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 class QueryStringDecoder { 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 class QueryStringDecoder { public Map> 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; 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 49b069f6db..caee98a080 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 class QueryStringDecoderTest { 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 class QueryStringDecoderTest { 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 class QueryStringDecoderTest { 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");