Fixed double query parameter & URL encoding / decoding bugs

This commit is contained in:
Andrei Savu 2012-02-09 16:04:45 +02:00 committed by Adrian Cole
parent 1c0c4e1b76
commit 74ded0302e
7 changed files with 72 additions and 69 deletions

View File

@ -18,23 +18,16 @@
*/
package org.jclouds.cloudstack.filters;
import static com.google.common.base.Preconditions.checkNotNull;
import java.util.Map.Entry;
import javax.annotation.Resource;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Provider;
import javax.inject.Singleton;
import javax.ws.rs.core.UriBuilder;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Multimap;
import org.jclouds.Constants;
import org.jclouds.crypto.Crypto;
import org.jclouds.crypto.CryptoStreams;
import org.jclouds.http.HttpException;
import org.jclouds.http.HttpRequest;
import org.jclouds.http.HttpRequestFilter;
import org.jclouds.http.HttpUtils;
import org.jclouds.http.internal.SignatureWire;
import org.jclouds.http.utils.ModifyRequest;
@ -43,11 +36,15 @@ import org.jclouds.logging.Logger;
import org.jclouds.rest.RequestSigner;
import org.jclouds.util.Strings2;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Multimap;
import javax.annotation.Resource;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Provider;
import javax.inject.Singleton;
import javax.ws.rs.core.UriBuilder;
import java.util.Map.Entry;
import static com.google.common.base.Preconditions.checkNotNull;
/**
*
@ -85,7 +82,7 @@ public class QuerySigner implements AuthenticationFilter, RequestSigner {
public HttpRequest filter(HttpRequest request) throws HttpException {
checkNotNull(request, "request must be present");
Multimap<String, String> decodedParams = ModifyRequest.parseQueryToMap(request.getEndpoint().getQuery());
Multimap<String, String> decodedParams = ModifyRequest.parseQueryToMap(request.getEndpoint().getRawQuery());
addSigningParams(decodedParams);
String stringToSign = createStringToSign(request, decodedParams);
String signature = sign(stringToSign);

View File

@ -47,7 +47,7 @@ public class ReEncodeQueryWithDefaultURLEncoder implements HttpRequestFilter {
public HttpRequest filter(HttpRequest request) throws HttpException {
UriBuilder builder = builders.get();
builder.uri(request.getEndpoint());
Multimap<String, String> map = ModifyRequest.parseQueryToMap(request.getEndpoint().getQuery());
Multimap<String, String> map = ModifyRequest.parseQueryToMap(request.getEndpoint().getRawQuery());
builder.replaceQuery("");
for (String key : map.keySet())
builder.queryParam(key, getOnlyElement(map.get(key)));

View File

@ -18,12 +18,9 @@
*/
package org.jclouds.cloudstack.features;
import java.io.IOException;
import java.lang.reflect.Method;
import java.net.URLEncoder;
import com.google.common.base.Functions;
import com.google.inject.TypeLiteral;
import org.jclouds.cloudstack.filters.QuerySigner;
import org.jclouds.cloudstack.options.ListSSHKeyPairsOptions;
import org.jclouds.crypto.SshKeys;
import org.jclouds.functions.IdentityFunction;
@ -37,6 +34,12 @@ import org.jclouds.rest.functions.ReturnVoidOnNotFoundOr404;
import org.jclouds.rest.internal.RestAnnotationProcessor;
import org.testng.annotations.Test;
import java.io.IOException;
import java.lang.reflect.Method;
import java.net.URLEncoder;
import static org.testng.Assert.assertEquals;
/**
* Tests behavior of {@code SSHKeyPairAsyncClient}
*
@ -112,7 +115,8 @@ public class SSHKeyPairAsyncClientTest extends BaseCloudStackAsyncClientTest<SSH
assertSaxResponseParserClassEquals(method, null);
assertExceptionParserClassEquals(method, MapHttp4xxCodesToExceptions.class);
checkFilters(httpRequest);
assertEquals(httpRequest.getFilters().size(), 2);
assertEquals(httpRequest.getFilters().get(0).getClass(), QuerySigner.class);
}
@ -130,7 +134,6 @@ public class SSHKeyPairAsyncClientTest extends BaseCloudStackAsyncClientTest<SSH
assertExceptionParserClassEquals(method, ReturnVoidOnNotFoundOr404.class);
checkFilters(httpRequest);
}

View File

@ -20,6 +20,7 @@ package org.jclouds.cloudstack.features;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
import com.sun.jersey.api.uri.UriComponent;
import org.jclouds.cloudstack.CloudStackContext;
import org.jclouds.cloudstack.domain.SshKeyPair;
import org.jclouds.crypto.SshKeys;
@ -28,7 +29,6 @@ import org.jclouds.http.HttpResponse;
import org.testng.annotations.Test;
import java.net.URI;
import java.net.URLEncoder;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
@ -165,8 +165,8 @@ public class SSHKeyPairClientExpectTest extends BaseCloudStackRestClientExpectTe
.method("GET")
.endpoint(
URI.create("http://localhost:8080/client/api?response=json&command=registerSSHKeyPair&" +
"name=jclouds-keypair&publickey=" + URLEncoder.encode(publicKey) +
"&apiKey=identity&signature=x6kHcaqhJW%2B7iMV4nLCRkm05AQ4%3D"))
"name=jclouds-keypair&publickey=" + UriComponent.encode(publicKey, UriComponent.Type.QUERY_PARAM) +
"&apiKey=identity&signature=g/6BXLnnvOMlKQBp1yM7GKlvfus%3D"))
.headers(
ImmutableMultimap.<String, String>builder()
.put("Accept", "application/json")

View File

@ -21,6 +21,7 @@ package org.jclouds.cloudstack.filters;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.sun.jersey.api.uri.UriBuilderImpl;
import com.sun.jersey.api.uri.UriComponent;
import org.jclouds.http.HttpRequest;
import org.jclouds.util.Strings2;
import org.testng.annotations.Test;
@ -58,6 +59,7 @@ public class ReEncodeQueryWithJavaNetURLEncoderTest {
}
}).getInstance(ReEncodeQueryWithDefaultURLEncoder.class).filter(request);
assertEquals(request.getEndpoint().toASCIIString(), "http://localhost?foo=" + URLEncoder.encode(input));
assertEquals(request.getEndpoint().toASCIIString(), "http://localhost?foo=" +
UriComponent.encode(input, UriComponent.Type.QUERY_PARAM));
}
}

View File

@ -18,20 +18,6 @@
*/
package org.jclouds.http.utils;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.jclouds.io.Payloads.newUrlEncodedFormPayload;
import java.net.URI;
import java.util.Comparator;
import java.util.Iterator;
import java.util.Map;
import org.jclouds.javax.annotation.Nullable;
import javax.ws.rs.core.UriBuilder;
import org.jclouds.http.HttpRequest;
import org.jclouds.util.Strings2;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
@ -39,6 +25,18 @@ import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.Multimap;
import org.jclouds.http.HttpRequest;
import org.jclouds.javax.annotation.Nullable;
import org.jclouds.util.Strings2;
import javax.ws.rs.core.UriBuilder;
import java.net.URI;
import java.util.Comparator;
import java.util.Iterator;
import java.util.Map;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.jclouds.io.Payloads.newUrlEncodedFormPayload;
/**
*
@ -159,7 +157,7 @@ public class ModifyRequest {
else
map.put(in, null);
} else {
String[] parts = Strings2.urlDecode(in).split("&");
String[] parts = in.split("&");
for (String part : parts) {
parseKeyValueFromStringToMap(part, map);
}
@ -172,7 +170,7 @@ public class ModifyRequest {
int indexOfFirstEquals = stringToParse.indexOf('=');
String key = indexOfFirstEquals == -1 ? stringToParse : stringToParse.substring(0, indexOfFirstEquals);
String value = indexOfFirstEquals == -1 ? null : stringToParse.substring(indexOfFirstEquals + 1);
map.put(key, value);
map.put(key, Strings2.urlDecode(value));
}
public static String makeQueryLine(Multimap<String, String> params,

View File

@ -46,56 +46,56 @@ public class ModifyRequestTest {
HttpRequest request = HttpRequest.builder().method("GET").endpoint(URI.create("http://foo")).build();
assertEquals(ModifyRequest.endpoint(request, URI.create("http://bar")), HttpRequest.builder().method("GET")
.endpoint(URI.create("http://bar")).build());
.endpoint(URI.create("http://bar")).build());
}
public void testReplaceHeader() {
HttpRequest request = HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.of("foo", "bar")).build();
.headers(ImmutableMultimap.of("foo", "bar")).build();
assertEquals(
ModifyRequest.replaceHeader(request, "foo", "baz"),
HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.of("foo", "baz")).build());
ModifyRequest.replaceHeader(request, "foo", "baz"),
HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.of("foo", "baz")).build());
}
public void testRemoveHeader() {
HttpRequest request = HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.of("foo", "bar")).build();
.headers(ImmutableMultimap.of("foo", "bar")).build();
assertEquals(ModifyRequest.removeHeader(request, "foo"),
HttpRequest.builder().method("GET").endpoint(URI.create("http://foo")).build());
HttpRequest.builder().method("GET").endpoint(URI.create("http://foo")).build());
}
public void testReplaceHeaders() {
HttpRequest request = HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.of("foo", "bar", "rabbit", "tree")).build();
.headers(ImmutableMultimap.of("foo", "bar", "rabbit", "tree")).build();
assertEquals(
ModifyRequest.replaceHeaders(request,
ImmutableMultimap.of("foo", "bar", "rabbit", "robot", "robert", "baz")),
HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.of("foo", "bar", "rabbit", "robot", "robert", "baz")).build());
ModifyRequest.replaceHeaders(request,
ImmutableMultimap.of("foo", "bar", "rabbit", "robot", "robert", "baz")),
HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.of("foo", "bar", "rabbit", "robot", "robert", "baz")).build());
}
public void testPutHeadersAddsAnotherValue() {
HttpRequest request = HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.of("foo", "bar")).build();
.headers(ImmutableMultimap.of("foo", "bar")).build();
assertEquals(
ModifyRequest.putHeaders(request, ImmutableMultimap.of("foo", "baz")),
HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.<String, String> builder().put("foo", "bar").put("foo", "baz").build())
.build());
ModifyRequest.putHeaders(request, ImmutableMultimap.of("foo", "baz")),
HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.headers(ImmutableMultimap.<String, String>builder().put("foo", "bar").put("foo", "baz").build())
.build());
}
public void testPutFormParamsAddsAnotherValue() {
HttpRequest request = HttpRequest.builder().method("GET").endpoint(URI.create("http://foo"))
.payload(Payloads.newStringPayload("foo=bar")).build();
.payload(Payloads.newStringPayload("foo=bar")).build();
Payload payload = Payloads.newStringPayload("foo=bar&foo=baz");
payload.getContentMetadata().setContentType(MediaType.APPLICATION_FORM_URLENCODED);
assertEquals(ModifyRequest.putFormParams(request, ImmutableMultimap.of("foo", "baz")), HttpRequest.builder()
.method("GET").endpoint(URI.create("http://foo")).payload(payload).build());
.method("GET").endpoint(URI.create("http://foo")).payload(payload).build());
}
public void testParseBase64InForm() {
@ -106,8 +106,8 @@ public class ModifyRequestTest {
expects.put("Value", "dGVzdA==");
expects.put("InstanceId", "1");
assertEquals(
expects,
parseQueryToMap("Version=2010-06-15&Action=ModifyInstanceAttribute&Attribute=userData&Value=dGVzdA%3D%3D&InstanceId=1"));
parseQueryToMap("Version=2010-06-15&Action=ModifyInstanceAttribute&Attribute=userData&Value=dGVzdA%3D%3D&InstanceId=1"),
expects);
}
@Test
@ -137,10 +137,13 @@ public class ModifyRequestTest {
"DOA2IHV9cUfEDBm1Be5TbpadWwSbS/05E+FARH2/MCO932UgcKUq5PGymS0249fLCBPci5zoLiG5vIym+1ij1hL/nHvkK99NIwe7io+Lmp" +
"9OcF3PTsm3Rgh5T09cRHGX9horp0VoAVa9vKJx6C1/IEHVnG8p0YPPa1lmemvx5kNBEiyoNQNYa34EiFkcJfP6rqNgvY8h/j4nE9SXoUCC" +
"/g6frhMFMOL0tzYqvz0Lczqm1Oh4RnSn3O9X4R934p28qqAobe337hmlLUdb6H5zuf+NwCh0HdZ";
String queryString = "publickey=" + Strings2.urlEncode(key);
Multimap<String, String> parsedMap = parseQueryToMap(queryString);
Set<String> expected = ImmutableSet.of(Strings2.urlEncode(key));
Set<String> expected = ImmutableSet.of(key);
Multimap<String, String> parsedMap = parseQueryToMap("a=1&b=1+2&publickey=" + Strings2.urlEncode(key));
assertEquals(parsedMap.get("publickey"), expected);
parsedMap = parseQueryToMap("publickey=" + Strings2.urlEncode(key));
assertEquals(parsedMap.get("publickey"), expected);
}