Apache
/
jclouds
mirror of https://github.com/apache/jclouds.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Timur Alperovich 2015-09-09 15:42:52 -07:00
parent 4bc4564900
commit 075c912d87
4 changed files with 10 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
core/src/main/java/org/jclouds/http/Uris.java
View File

@ -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;
}
}
}

9
core/src/main/java/org/jclouds/util/Strings2.java
View File

@ -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");

5
core/src/test/java/org/jclouds/http/HttpRequestTest.java
View File

@ -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)

10
core/src/test/java/org/jclouds/util/Strings2Test.java
View File

@ -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");
}