diff --git a/core/src/main/java/org/jclouds/http/Uris.java b/core/src/main/java/org/jclouds/http/Uris.java index bf8449b925..441db62a7e 100644 --- a/core/src/main/java/org/jclouds/http/Uris.java +++ b/core/src/main/java/org/jclouds/http/Uris.java @@ -282,9 +282,9 @@ public final class Uris { return build(ImmutableMap. of()); } - public URI buildNoEncoding(Map variables) { + public URI build(Map variables, boolean encodePath, boolean encodeQuery) { try { - return new URI(expand(variables, false)); + return new URI(expand(variables, encodePath, encodeQuery)); } catch (URISyntaxException e) { throw new IllegalArgumentException(e); } @@ -296,13 +296,13 @@ public final class Uris { */ public URI build(Map variables) { try { - return new URI(expand(variables, true)); + return new URI(expand(variables, true, true)); } catch (URISyntaxException e) { throw new IllegalArgumentException(e); } } - private String expand(Map variables, boolean shouldEncode) { + private String expand(Map variables, boolean encodePath, boolean encodeQuery) { StringBuilder b = new StringBuilder(); if (scheme != null) b.append(scheme).append("://"); @@ -311,14 +311,19 @@ public final class Uris { if (port != null) b.append(':').append(port); if (path != null) { - if (shouldEncode) { + if (encodePath) { b.append(urlEncode(UriTemplates.expand(path, variables), skipPathEncoding)); } else { b.append(UriTemplates.expand(path, variables)); } } - if (!query.isEmpty()) - b.append('?').append(encodeQueryLine(query)); + if (!query.isEmpty()) { + if (encodeQuery) { + b.append('?').append(encodeQueryLine(query)); + } else { + b.append('?').append(buildQueryLine(query)); + } + } return b.toString(); } diff --git a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java index e1c7468974..113e9dce5e 100644 --- a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java +++ b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java @@ -40,6 +40,7 @@ import static org.jclouds.http.Uris.uriBuilder; import static org.jclouds.io.Payloads.newPayload; import static org.jclouds.reflect.Reflection2.getInvokableParameters; import static org.jclouds.util.Strings2.replaceTokens; +import static org.jclouds.util.Strings2.urlEncode; import java.lang.annotation.Annotation; import java.lang.reflect.Array; @@ -102,7 +103,6 @@ import org.jclouds.rest.annotations.VirtualHost; import org.jclouds.rest.annotations.WrapWith; import org.jclouds.rest.binders.BindMapToStringPayload; import org.jclouds.rest.binders.BindToJsonPayloadWrappedWith; -import org.jclouds.util.Strings2; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; @@ -231,10 +231,10 @@ public class RestAnnotationProcessor implements Function formParams; if (caller != null) { formParams = addFormParams(tokenValues, caller); @@ -270,7 +270,8 @@ public class RestAnnotationProcessor implements Function query : options.buildQueryParameters().entries()) { - queryParams.put(query.getKey(), replaceTokens(query.getValue(), tokenValues)); + queryParams.put(urlEncode(query.getKey(), '/', ','), + urlEncode(replaceTokens(query.getValue(), tokenValues), '/', ',')); } for (Entry form : options.buildFormParameters().entries()) { formParams.put(form.getKey(), replaceTokens(form.getValue(), tokenValues)); @@ -291,11 +292,9 @@ public class RestAnnotationProcessor implements Function addPathAndGetTokens(Invocation invocation, UriBuilder uriBuilder, boolean encoded) { + private Multimap addPathAndGetTokens(Invocation invocation, UriBuilder uriBuilder, + boolean encodeFullPath) { if (invocation.getInvokable().getOwnerType().getRawType().isAnnotationPresent(Path.class)) uriBuilder.appendPath(invocation.getInvokable().getOwnerType().getRawType().getAnnotation(Path.class).value()); if (invocation.getInvokable().isAnnotationPresent(Path.class)) uriBuilder.appendPath(invocation.getInvokable().getAnnotation(Path.class).value()); - return getPathParamKeyValues(invocation, encoded); + return getPathParamKeyValues(invocation, encodeFullPath); } private Multimap addFormParams(Multimap tokenValues, Invocation invocation) { @@ -452,11 +452,12 @@ public class RestAnnotationProcessor implements Function queryParams, QueryParams query, Multimap tokenValues) { for (int i = 0; i < query.keys().length; i++) { + String key = urlEncode(query.keys()[i], '/', ','); if (query.values()[i].equals(QueryParams.NULL)) { - queryParams.removeAll(query.keys()[i]); - queryParams.put(query.keys()[i], null); + queryParams.removeAll(key); + queryParams.put(key, null); } else { - queryParams.put(query.keys()[i], replaceTokens(query.values()[i], tokenValues)); + queryParams.put(key, urlEncode(replaceTokens(query.values()[i], tokenValues), '/', ',')); } } } @@ -768,7 +769,7 @@ public class RestAnnotationProcessor implements Function getPathParamKeyValues(Invocation invocation, boolean encodedParams) { + private Multimap getPathParamKeyValues(Invocation invocation, boolean encodeFullPath) { Multimap pathParamValues = LinkedHashMultimap.create(); for (Parameter param : parametersWithAnnotation(invocation.getInvokable(), PathParam.class)) { PathParam pathParam = param.getAnnotation(PathParam.class); @@ -776,8 +777,8 @@ public class RestAnnotationProcessor implements Function paramValue = getParamValue(invocation, param.getAnnotation(ParamParser.class), param.hashCode(), paramKey); if (paramValue.isPresent()) { - if (encodedParams && !param.isAnnotationPresent(Encoded.class)) { - pathParamValues.put(paramKey, Strings2.urlEncode(paramValue.get().toString())); + if (!encodeFullPath && !param.isAnnotationPresent(Encoded.class)) { + pathParamValues.put(paramKey, urlEncode(paramValue.get().toString())); } else { pathParamValues.put(paramKey, paramValue.get().toString()); } @@ -821,16 +822,28 @@ public class RestAnnotationProcessor implements Function queryParamValues = LinkedHashMultimap.create(); for (Parameter param : parametersWithAnnotation(invocation.getInvokable(), QueryParam.class)) { QueryParam queryParam = param.getAnnotation(QueryParam.class); - String paramKey = queryParam.value(); + String paramKey = urlEncode(queryParam.value(), '/', ','); Optional paramValue = getParamValue(invocation, param.getAnnotation(ParamParser.class), param.hashCode(), paramKey); if (paramValue.isPresent()) if (paramValue.get() instanceof Iterable) { @SuppressWarnings("unchecked") Iterable iterableStrings = transform(Iterable.class.cast(paramValue.get()), toStringFunction()); + if (!param.isAnnotationPresent(Encoded.class)) { + iterableStrings = transform(iterableStrings, new Function() { + @Override + public String apply(@Nullable String s) { + return urlEncode(s, '/', ','); + } + }); + } queryParamValues.putAll(paramKey, iterableStrings); } else { - queryParamValues.put(paramKey, paramValue.get().toString()); + String value = paramValue.get().toString(); + if (!param.isAnnotationPresent(Encoded.class)) { + value = urlEncode(value, '/', ','); + } + queryParamValues.put(paramKey, value); } } return queryParamValues; diff --git a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java index 95bed02659..6a9dd737b3 100644 --- a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java +++ b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java @@ -24,6 +24,7 @@ import static org.jclouds.io.Payloads.newInputStreamPayload; import static org.jclouds.io.Payloads.newStringPayload; import static org.jclouds.providers.AnonymousProviderMetadata.forApiOnEndpoint; import static org.jclouds.reflect.Reflection2.method; +import static org.jclouds.util.Strings2.urlEncode; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; @@ -483,6 +484,12 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { @Path("/") public void queryParamIterable(@Nullable @QueryParam("foo") Iterable bars) { } + + @FOO + @Path("/") + @QueryParams(keys = { "test param"}, values = { "foo bar" }) + public void queryKeyEncoded() { + } } public void testQuery() throws SecurityException, NoSuchMethodException { @@ -575,6 +582,30 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { assertEquals(request.getMethod(), "FOO"); } + public void testQueryEncodedKey() throws SecurityException, NoSuchMethodException { + GeneratedHttpRequest request = processor.apply(Invocation.create(method(TestQuery.class, "queryKeyEncoded"))); + assertEquals(request.getEndpoint().getHost(), "localhost"); + assertEquals(request.getEndpoint().getPath(), "/"); + assertEquals(request.getEndpoint().getRawQuery(), "x-ms-version=2009-07-17&test%20param=foo%20bar"); + assertEquals(request.getMethod(), "FOO"); + } + + @QueryParams(keys = "test%param", values = "percent%") + public class TestInterfaceQueryParam { + @FOO + @Path("/") + public void query() { + } + } + + public void testInterfaceEncodedKey() throws SecurityException, NoSuchMethodException { + GeneratedHttpRequest request = processor.apply(Invocation.create(method(TestInterfaceQueryParam.class, "query"))); + assertEquals(request.getEndpoint().getHost(), "localhost"); + assertEquals(request.getEndpoint().getPath(), "/"); + assertEquals(request.getEndpoint().getRawQuery(), "test%25param=percent%25"); + assertEquals(request.getMethod(), "FOO"); + } + interface TestPayloadParamVarargs { @POST void varargs(HttpRequestOptions... options); @@ -663,6 +694,16 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { assertPayloadEquals(request, "fooya", "application/unknown", false); } + public void testQueryVarargsEncoding() throws Exception { + Invokable method = method(TestPayloadParamVarargs.class, "varargsWithReq", String.class, + HttpRequestOptions[].class); + GeneratedHttpRequest request = processor.apply( + Invocation.create(method, + ImmutableList. of("required param", + new TestHttpRequestOptions().queryParams(ImmutableMultimap.of("key", "foo bar"))))); + assertRequestLineEquals(request, "POST http://localhost:9999?key=foo%20bar HTTP/1.1"); + } + public void testDuplicateHeaderAndQueryVarargs() throws Exception { Invokable method = method(TestPayloadParamVarargs.class, "varargs", HttpRequestOptions[].class); @@ -1556,6 +1597,21 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { public void oneQueryParamExtractor(@QueryParam("one") @ParamParser(FirstCharacter.class) String one) { } + @GET + @Path("/") + public void oneQueryParam(@QueryParam("one") String one) { + } + + @GET + @Path("/") + public void encodedQueryParam(@QueryParam("encoded") @Encoded String encoded) { + } + + @GET + @Path("/") + public void encodedQueryListParam(@QueryParam("encoded") @Encoded List encodedStrings) { + } + @POST @Path("/") public void oneFormParamExtractor(@FormParam("one") @ParamParser(FirstCharacter.class) String one) { @@ -1608,6 +1664,41 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { assertPayloadEquals(request, null, null, false); } + @Test + public void testEncodedQueryParam() throws Exception { + Invokable method = method(TestPath.class, "encodedQueryParam", String.class); + GeneratedHttpRequest request = processor.apply(Invocation.create(method, + ImmutableList. of("foo%20bar"))); + assertRequestLineEquals(request, "GET http://localhost:9999/?encoded=foo%20bar HTTP/1.1"); + assertNonPayloadHeadersEqual(request, ""); + assertPayloadEquals(request, null, null, false); + + method = method(TestPath.class, "encodedQueryListParam", List.class); + String [] args = {"foo%20bar", "foo/bar"}; + request = processor.apply(Invocation.create(method, + ImmutableList. of(ImmutableList.of("foo%20bar", "foo/bar")))); + assertRequestLineEquals(request, "GET http://localhost:9999/?encoded=foo%20bar&encoded=foo/bar HTTP/1.1"); + assertNonPayloadHeadersEqual(request, ""); + assertPayloadEquals(request, null, null, false); + } + + @DataProvider(name = "queryStrings") + public Object[][] createQueryData() { + return new Object[][] { { "normal" }, { "sp ace" }, { "qu?stion" }, { "unic₪de" }, { "path/foo" }, { "colon:" }, + { "asteri*k" }, { "quote\"" }, { "greaten" }, { "p|pe" } }; + } + + @Test(dataProvider = "queryStrings") + public void testQueryParam(String val) { + Invokable method = method(TestPath.class, "oneQueryParam", String.class); + GeneratedHttpRequest request = processor.apply(Invocation.create(method, + ImmutableList. of(val))); + assertRequestLineEquals(request, String.format("GET http://localhost:9999/?one=%s HTTP/1.1", + urlEncode(val, '/', ','))); + assertNonPayloadHeadersEqual(request, ""); + assertPayloadEquals(request, null, null, false); + } + @Test public void testFormParamExtractor() throws Exception { Invokable method = method(TestPath.class, "oneFormParamExtractor", String.class);