Skip to content

Commit

Permalink
fix: add encoded query string for presigning. (#458)
Browse files Browse the repository at this point in the history 
This patch fixes presignv4() by using encodeQueryString().

Fixes #456
  • Loading branch information
@balamurugana @harshavardhana
balamurugana authored and harshavardhana committed Sep 17, 2016
1 parent e12b680 commit 0c865ba
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 39 deletions.
30 changes: 2 additions & 28 deletions src/main/java/io/minio/MinioClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package io.minio;

import com.google.common.net.UrlEscapers;
import com.google.common.escape.Escaper;
import com.google.common.io.ByteStreams;
import com.google.gson.Gson;
import com.squareup.okhttp.HttpUrl;
Expand Down Expand Up @@ -128,7 +126,6 @@
*/
@SuppressWarnings({"SameParameterValue", "WeakerAccess"})
public final class MinioClient {
private static final Escaper QUERY_ESCAPER = UrlEscapers.urlPathSegmentEscaper();
private static final Logger LOGGER = Logger.getLogger(MinioClient.class.getName());
// maximum allowed object size is 5TiB
private static final long MAX_OBJECT_SIZE = 5L * 1024 * 1024 * 1024 * 1024;
Expand Down Expand Up @@ -593,29 +590,6 @@ public void setTimeout(long connectTimeout, long writeTimeout, long readTimeout)
httpClient.setReadTimeout(readTimeout, TimeUnit.MILLISECONDS);
}

/**
* Returns encoded query string for URL.
*/
private static String encodeQueryString(String str) {
return QUERY_ESCAPER.escape(str)
.replaceAll("\\!", "%21")
.replaceAll("\\$", "%24")
.replaceAll("\\&", "%26")
.replaceAll("\\'", "%27")
.replaceAll("\\(", "%28")
.replaceAll("\\)", "%29")
.replaceAll("\\*", "%2A")
.replaceAll("\\+", "%2B")
.replaceAll("\\,", "%2C")
.replaceAll("\\\\", "%2F")
.replaceAll("\\:", "%3A")
.replaceAll("\\;", "%3B")
.replaceAll("\\=", "%3D")
.replaceAll("\\@", "%40")
.replaceAll("\\[", "%5B")
.replaceAll("\\]", "%5D");
}

/**
* Creates Request object for given request parameters.
*
Expand Down Expand Up @@ -684,8 +658,8 @@ private Request createRequest(Method method, String bucketName, String objectNam

if (queryParamMap != null) {
for (Map.Entry<String,String> entry : queryParamMap.entrySet()) {
urlBuilder.addEncodedQueryParameter(encodeQueryString(entry.getKey()),
encodeQueryString(entry.getValue()));
urlBuilder.addEncodedQueryParameter(Signer.encodeQueryString(entry.getKey()),
Signer.encodeQueryString(entry.getValue()));
}
}

Expand Down
48 changes: 42 additions & 6 deletions src/main/java/io/minio/Signer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import com.squareup.okhttp.Headers;
import com.google.common.base.Joiner;
import com.google.common.io.BaseEncoding;
import com.google.common.net.UrlEscapers;
import com.google.common.escape.Escaper;

import io.minio.Digest;

Expand Down Expand Up @@ -68,6 +70,7 @@ class Signer {
//
// Is skipped for obvious reasons
//
private static final Escaper QUERY_ESCAPER = UrlEscapers.urlPathSegmentEscaper();
private static final Set<String> IGNORED_HEADERS = new HashSet<>();

static {
Expand Down Expand Up @@ -245,11 +248,16 @@ private void setPresignCanonicalRequest(int expires) throws NoSuchAlgorithmExcep

HttpUrl.Builder urlBuilder = this.request.httpUrl().newBuilder();
// order of queryparam addition is important ie has to be sorted.
urlBuilder.addQueryParameter("X-Amz-Algorithm", "AWS4-HMAC-SHA256");
urlBuilder.addQueryParameter("X-Amz-Credential", this.accessKey + "/" + this.scope);
urlBuilder.addQueryParameter("X-Amz-Date", this.date.toString(DateFormat.AMZ_DATE_FORMAT));
urlBuilder.addQueryParameter("X-Amz-Expires", Integer.toString(expires));
urlBuilder.addQueryParameter("X-Amz-SignedHeaders", this.signedHeaders);
urlBuilder.addEncodedQueryParameter(encodeQueryString("X-Amz-Algorithm"),
encodeQueryString("AWS4-HMAC-SHA256"));
urlBuilder.addEncodedQueryParameter(encodeQueryString("X-Amz-Credential"),
encodeQueryString(this.accessKey + "/" + this.scope));
urlBuilder.addEncodedQueryParameter(encodeQueryString("X-Amz-Date"),
encodeQueryString(this.date.toString(DateFormat.AMZ_DATE_FORMAT)));
urlBuilder.addEncodedQueryParameter(encodeQueryString("X-Amz-Expires"),
encodeQueryString(Integer.toString(expires)));
urlBuilder.addEncodedQueryParameter(encodeQueryString("X-Amz-SignedHeaders"),
encodeQueryString(this.signedHeaders));
this.url = urlBuilder.build();

setCanonicalQueryString();
Expand Down Expand Up @@ -281,7 +289,8 @@ public static HttpUrl presignV4(Request request, String region, String accessKey
signer.setSignature();

return signer.url.newBuilder()
.addQueryParameter("X-Amz-Signature", signer.signature)
.addEncodedQueryParameter(encodeQueryString("X-Amz-Signature"),
encodeQueryString(signer.signature))
.build();
}

Expand Down Expand Up @@ -320,4 +329,31 @@ public static byte[] sumHmac(byte[] key, byte[] data)

return mac.doFinal();
}

/**
* Returns encoded query string for URL.
*/
public static String encodeQueryString(String str) {
if (str == null) {
return "";
}

return QUERY_ESCAPER.escape(str)
.replaceAll("\\!", "%21")
.replaceAll("\\$", "%24")
.replaceAll("\\&", "%26")
.replaceAll("\\'", "%27")
.replaceAll("\\(", "%28")
.replaceAll("\\)", "%29")
.replaceAll("\\*", "%2A")
.replaceAll("\\+", "%2B")
.replaceAll("\\,", "%2C")
.replaceAll("\\\\", "%2F")
.replaceAll("\\:", "%3A")
.replaceAll("\\;", "%3B")
.replaceAll("\\=", "%3D")
.replaceAll("\\@", "%40")
.replaceAll("\\[", "%5B")
.replaceAll("\\]", "%5D");
}
}
50 changes: 45 additions & 5 deletions test/FunctionalTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,15 @@ public static void presignedGetObject_test1() throws Exception {
response.body().close();
os.close();
} else {
println("FAILED");
String errorXml = "";

// read entire body stream to string.
Scanner scanner = new java.util.Scanner(response.body().charStream()).useDelimiter("\\A");
if (scanner.hasNext()) {
errorXml = scanner.next();
}

println("FAILED", response, errorXml);
}
} else {
println("NO RESPONSE");
Expand Down Expand Up @@ -518,7 +526,15 @@ public static void presignedGetObject_test2() throws Exception {
response.body().close();
os.close();
} else {
println("FAILED");
String errorXml = "";

// read entire body stream to string.
Scanner scanner = new java.util.Scanner(response.body().charStream()).useDelimiter("\\A");
if (scanner.hasNext()) {
errorXml = scanner.next();
}

println("FAILED", response, errorXml);
}
} else {
println("NO RESPONSE");
Expand Down Expand Up @@ -551,7 +567,15 @@ public static void presignedPutObject_test1() throws Exception {

if (response != null) {
if (!response.isSuccessful()) {
println("FAILED");
String errorXml = "";

// read entire body stream to string.
Scanner scanner = new java.util.Scanner(response.body().charStream()).useDelimiter("\\A");
if (scanner.hasNext()) {
errorXml = scanner.next();
}

println("FAILED", response, errorXml);
}
} else {
println("NO RESPONSE");
Expand All @@ -578,7 +602,15 @@ public static void presignedPutObject_test2() throws Exception {

if (response != null) {
if (!response.isSuccessful()) {
println("FAILED");
String errorXml = "";

// read entire body stream to string.
Scanner scanner = new java.util.Scanner(response.body().charStream()).useDelimiter("\\A");
if (scanner.hasNext()) {
errorXml = scanner.next();
}

println("FAILED", response, errorXml);
}
} else {
println("NO RESPONSE");
Expand Down Expand Up @@ -611,7 +643,15 @@ public static void presignedPostPolicy_test() throws Exception {

if (response != null) {
if (!response.isSuccessful()) {
println("FAILED");
String errorXml = "";

// read entire body stream to string.
Scanner scanner = new java.util.Scanner(response.body().charStream()).useDelimiter("\\A");
if (scanner.hasNext()) {
errorXml = scanner.next();
}

println("FAILED", response, errorXml);
}
} else {
println("NO RESPONSE");
Expand Down

0 comments on commit 0c865ba

Please sign in to comment.