You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jclouds.apache.org by ga...@apache.org on 2015/09/17 22:24:30 UTC
[5/5] jclouds git commit: JCLOUDS-217: Do not decode query strings.
JCLOUDS-217: Do not decode query strings.
jclouds should not decode query strings that are passed to create HTTP
requests. This is problematic because in some cases a wrong request
may be generated. The most obvious example is if one passes the "+"
character. For example, the following query parameter: "users=me+you"
is stored by the URI builder as "me you" and subsequently appears in
the request as "users=me%20you", as opposed to "users=me%2Byou" (%2b
is percent encoding for "+").
This is not currently a problem because jclouds relies on the
isUrlEncoded() method to check if a query parameter should be decoded
and the situation above is avoided.
This PR attempts to suggest an alternative (and what I believe is
simpler) approach: on the path of crafting requests, jclouds should
only *encode*, not decode strings. Specifically, jclouds should
_never_ be in a situation where it relies on the isUrlEncoded()
method.
Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo
Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/075c912d
Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/075c912d
Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/075c912d
Branch: refs/heads/master
Commit: 075c912d873708a7fbedfea62fae073275c41a05
Parents: 4bc4564
Author: Timur Alperovich <ti...@gmail.com>
Authored: Wed Sep 9 15:42:52 2015 -0700
Committer: Timur Alperovich <ti...@gmail.com>
Committed: Thu Sep 17 13:08:44 2015 -0700
----------------------------------------------------------------------
core/src/main/java/org/jclouds/http/Uris.java | 65 +++-----------------
.../main/java/org/jclouds/util/Strings2.java | 9 ---
.../java/org/jclouds/http/HttpRequestTest.java | 5 +-
.../java/org/jclouds/util/Strings2Test.java | 10 ---
4 files changed, 10 insertions(+), 79 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jclouds/blob/075c912d/core/src/main/java/org/jclouds/http/Uris.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/http/Uris.java b/core/src/main/java/org/jclouds/http/Uris.java
index abab52b..facc181 100644
--- a/core/src/main/java/org/jclouds/http/Uris.java
+++ b/core/src/main/java/org/jclouds/http/Uris.java
@@ -28,17 +28,13 @@ import static org.jclouds.util.Strings2.urlEncode;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Map;
import org.jclouds.javax.annotation.Nullable;
import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
-import com.google.common.collect.ForwardingMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
@@ -103,7 +99,7 @@ public final class Uris {
private String host;
private Integer port;
private String path;
- private Multimap<String, Object> query = DecodingMultimap.create();
+ private Multimap<String, Object> query = LinkedHashMultimap.create();
/**
* override default of {@code / : ; =}
@@ -274,10 +270,12 @@ public final class Uris {
this.scheme = uri.getScheme();
this.host = uri.getHost();
this.port = uri.getPort() == -1 ? null : uri.getPort();
- if (uri.getPath() != null)
- path(unescapeSpecialChars(uri.getPath()));
- if (uri.getQuery() != null)
- query(queryParser().apply(unescapeSpecialChars(uri.getQuery())));
+ if (uri.getRawPath() != null)
+ // path decodes the string, so we need to get at the raw (encoded) string
+ path(unescapeSpecialChars(uri.getRawPath()));
+ if (uri.getRawQuery() != null)
+ // The query parser decodes the strings that are passed to it; we should pass raw (encoded) queries
+ query(queryParser().apply(unescapeSpecialChars(uri.getRawQuery())));
}
public URI build() {
@@ -372,53 +370,4 @@ public final class Uris {
return new StringBuilder().append('/').append(in).toString();
return in;
}
-
- /**
- * Mutable and permits null values. Url decodes all mutations except {@link Multimap#putAll(Multimap)}
- *
- *
- */
- static final class DecodingMultimap extends ForwardingMultimap<String, Object> {
-
- private static Multimap<String, Object> create() {
- return new DecodingMultimap();
- }
-
- private final Multimap<String, Object> delegate = LinkedHashMultimap.create();
- private final Function<Object, Object> urlDecoder = new Function<Object, Object>() {
- public Object apply(Object in) {
- if (in == null) {
- return null;
- }
- return urlDecode(in.toString());
- }
- };
-
- @Override
- public boolean put(String key, Object value) {
- return super.put(urlDecode(key), urlDecoder.apply(value));
- }
-
- @Override
- public boolean putAll(String key, Iterable<? extends Object> values) {
- return super.putAll(urlDecode(key), Iterables.transform(values, urlDecoder));
- }
-
- @Override
- public boolean putAll(Multimap<? extends String, ? extends Object> multimap) {
- return super.putAll(multimap);
- }
-
- @Override
- public Collection<Object> replaceValues(String key, Iterable<? extends Object> values) {
- return super.replaceValues(urlDecode(key), Iterables.transform(values, urlDecoder));
- }
-
- @Override
- protected Multimap<String, Object> delegate() {
- return delegate;
- }
-
- }
-
}
http://git-wip-us.apache.org/repos/asf/jclouds/blob/075c912d/core/src/main/java/org/jclouds/util/Strings2.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/util/Strings2.java b/core/src/main/java/org/jclouds/util/Strings2.java
index e58e0a2..28ba26c 100644
--- a/core/src/main/java/org/jclouds/util/Strings2.java
+++ b/core/src/main/java/org/jclouds/util/Strings2.java
@@ -88,12 +88,6 @@ public class Strings2 {
public static boolean isCidrFormat(String in) {
return CIDR_PATTERN.matcher(in).matches();
}
-
- private static final Pattern URL_ENCODED_PATTERN = Pattern.compile(".*%[a-fA-F0-9][a-fA-F0-9].*");
-
- public static boolean isUrlEncoded(String in) {
- return URL_ENCODED_PATTERN.matcher(in).matches();
- }
/**
* url decodes the input param, if set.
@@ -107,9 +101,6 @@ public class Strings2 {
public static String urlDecode(@Nullable String in) {
if (in == null)
return null;
- if (!isUrlEncoded(in)) {
- return in;
- }
String input = in.toString();
try {
return URLDecoder.decode(input, "UTF-8");
http://git-wip-us.apache.org/repos/asf/jclouds/blob/075c912d/core/src/test/java/org/jclouds/http/HttpRequestTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/jclouds/http/HttpRequestTest.java b/core/src/test/java/org/jclouds/http/HttpRequestTest.java
index 73ad8b5..ffdaadf 100644
--- a/core/src/test/java/org/jclouds/http/HttpRequestTest.java
+++ b/core/src/test/java/org/jclouds/http/HttpRequestTest.java
@@ -23,6 +23,7 @@ import java.net.URI;
import org.jclouds.io.Payload;
import org.jclouds.io.Payloads;
+import org.jclouds.util.Strings2;
import org.testng.annotations.Test;
import com.google.common.collect.ImmutableMultimap;
@@ -67,8 +68,8 @@ public class HttpRequestTest {
// note that + ends up encoded as %2B (plus) and %2F converts back into slash
public void testAddBase64AndUrlEncodedQueryParams() {
- String base64Chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789%2B%2F%3D";
- URI uri = URI.create("http://goo.com:443?header1=" + base64Chars);
+ String base64Chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
+ URI uri = URI.create("http://goo.com:443?header1=" + Strings2.urlEncode(base64Chars));
HttpRequest request = HttpRequest.builder()
.method("GET")
.endpoint(uri)
http://git-wip-us.apache.org/repos/asf/jclouds/blob/075c912d/core/src/test/java/org/jclouds/util/Strings2Test.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/jclouds/util/Strings2Test.java b/core/src/test/java/org/jclouds/util/Strings2Test.java
index 125e0eb..bbfbfee 100644
--- a/core/src/test/java/org/jclouds/util/Strings2Test.java
+++ b/core/src/test/java/org/jclouds/util/Strings2Test.java
@@ -27,16 +27,6 @@ import com.google.common.collect.ImmutableMap;
@Test(groups = "unit")
public class Strings2Test {
- public void testIsEncoded() {
- assert Strings2.isUrlEncoded("/read-tests/%73%6f%6d%65%20%66%69%6c%65");
- assert !Strings2.isUrlEncoded("/read-tests/ tep");
- }
-
- public void testNoDoubleDecode() {
- assertEquals(urlDecode("foo%20bar%2Bbaz"), "foo bar+baz");
- assertEquals(urlDecode("foo bar+baz"), "foo bar+baz");
- }
-
public void testReplaceTokens() {
assertEquals(Strings2.replaceTokens("hello {where}", ImmutableMap.of("where", "world")), "hello world");
}