From 0c865ba52c4356f793e0ce4e6fee33d70479380d Mon Sep 17 00:00:00 2001 From: Bala FA Date: Sat, 17 Sep 2016 14:34:19 -0700 Subject: [PATCH] fix: add encoded query string for presigning. (#458) This patch fixes presignv4() by using encodeQueryString(). Fixes #456 --- src/main/java/io/minio/MinioClient.java | 30 +-------------- src/main/java/io/minio/Signer.java | 48 +++++++++++++++++++++--- test/FunctionalTest.java | 50 ++++++++++++++++++++++--- 3 files changed, 89 insertions(+), 39 deletions(-) diff --git a/src/main/java/io/minio/MinioClient.java b/src/main/java/io/minio/MinioClient.java index 748b0605e..c1e5d6b23 100644 --- a/src/main/java/io/minio/MinioClient.java +++ b/src/main/java/io/minio/MinioClient.java @@ -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; @@ -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; @@ -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. * @@ -684,8 +658,8 @@ private Request createRequest(Method method, String bucketName, String objectNam if (queryParamMap != null) { for (Map.Entry entry : queryParamMap.entrySet()) { - urlBuilder.addEncodedQueryParameter(encodeQueryString(entry.getKey()), - encodeQueryString(entry.getValue())); + urlBuilder.addEncodedQueryParameter(Signer.encodeQueryString(entry.getKey()), + Signer.encodeQueryString(entry.getValue())); } } diff --git a/src/main/java/io/minio/Signer.java b/src/main/java/io/minio/Signer.java index c4ffde708..6f5afbb60 100644 --- a/src/main/java/io/minio/Signer.java +++ b/src/main/java/io/minio/Signer.java @@ -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; @@ -68,6 +70,7 @@ class Signer { // // Is skipped for obvious reasons // + private static final Escaper QUERY_ESCAPER = UrlEscapers.urlPathSegmentEscaper(); private static final Set IGNORED_HEADERS = new HashSet<>(); static { @@ -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(); @@ -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(); } @@ -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"); + } } diff --git a/test/FunctionalTest.java b/test/FunctionalTest.java index f28eccdd9..613391f11 100644 --- a/test/FunctionalTest.java +++ b/test/FunctionalTest.java @@ -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"); @@ -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"); @@ -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"); @@ -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"); @@ -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");