Percent encoding query parameters separated from url encoding

Author: RachelTucker

ds3-sdk-integration/src/test/java/com/spectralogic/ds3client/integration/Smoke_Test.java

@@ -177,6 +177,27 @@ public void getObjectsWithPagination() throws IOException, URISyntaxException {
 
     }
 
+    @Test
+    public void getBucketWithAmpersandFolderName() throws IOException, URISyntaxException {
+        final String bucketName = "test_ampersand_folder";
+        final String folderName = "test&folder/";
+        try {
+            HELPERS.ensureBucketExists(bucketName, envDataPolicyId);
+            loadBookTestDataWithPrefix(client, bucketName, folderName);
+
+            final HeadObjectResponse response = client.headObject(new HeadObjectRequest(
+                    bucketName, folderName + "beowulf.txt"));
+            assertThat(response.getStatus(),
+                    is(HeadObjectResponse.Status.EXISTS));
+
+            final GetBucketResponse getBucket = client.getBucket(new GetBucketRequest(bucketName));
+            assertThat(getBucket.getListBucketResult(), is(notNullValue()));
+            assertThat(getBucket.getListBucketResult().getObjects().size(), is(4));
+        } finally {
+            deleteAllContents(client, bucketName);
+        }
+    }
+
     private static boolean s3ObjectExists(final List<S3Object> objects, final String fileName) {
         for (final S3Object obj : objects) {
             if (obj.getName().equals(fileName)) {

ds3-sdk/src/main/java/com/spectralogic/ds3client/commands/interfaces/AbstractRequest.java

@@ -75,7 +75,7 @@ protected final <T> void updateQueryParam(final String name, final T param) {
             this.queryParams.remove(name);
         }
         else {
-            this.queryParams.put(name, SafeStringManipulation.safeUrlEscape(param));
+            this.queryParams.put(name, SafeStringManipulation.safeQueryParamEscape(param));
         }
     }
 }

ds3-sdk/src/main/java/com/spectralogic/ds3client/utils/SafeStringManipulation.java

@@ -32,13 +32,34 @@ public final class SafeStringManipulation {
             "!$'()*,&=" +   // removed ; (so it will be escaped) and added / (so it will not)
             "@:/";          // Their urlFragmentEscaper uses URL_PATH_OTHER_SAFE_CHARS_LACKING_PLUS + "+/?"+
 
+    /**
+     * Specified as query safe characters in spec https://tools.ietf.org/html/rfc3986#section-3.4
+     * with the following exceptions:
+     *  Encoding: "&", ":", "+", "=" as they have special meaning
+     *  Not Encoding: "/"
+     */
+    private static final String DS3_QUERY_PARAM_SAFE_CHARS = "-._~!$'()*,;@/";
+
     private static final Escaper DS3_URL_FRAGMENT_ESCAPER =
             new PercentEscaper(DS3_URL_PATH_FRAGMENT_SAFE_CHARS, false);
 
+    private static final Escaper DS3_QUERY_PARAM_ESCAPER =
+            new PercentEscaper(DS3_QUERY_PARAM_SAFE_CHARS, false);
+
     private SafeStringManipulation() {
         //pass
     }
 
+    /**
+     * Percent encodes user-provided query parameter values.
+     */
+    public static <T> String safeQueryParamEscape(final T obj) {
+        if (obj == null) {
+            return null;
+        }
+        return DS3_QUERY_PARAM_ESCAPER.escape(safeToString(obj));
+    }
+
     public static <T> String safeUrlEscape(final T obj) {
         if (obj == null) {
             return null;
@@ -58,6 +79,7 @@ public static <T> String safeToString(final T obj) {
         }
         return obj.toString();
     }
+
     public static Escaper getDs3Escaper() {
         // escaped characters in DS3 path and query parameter value segments
         return DS3_URL_FRAGMENT_ESCAPER;

ds3-sdk/src/test/java/com/spectralogic/ds3client/utils/SafeString_Manipulation_Test.java

@@ -108,4 +108,13 @@ public void safeUrlEscape_Test() {
         assertThat(SafeStringManipulation.safeUrlEscape("one2three+four"), is("one2three%2Bfour"));
         assertThat(SafeStringManipulation.safeUrlEscape("one two three"), is("one%20two%20three"));
     }
+
+    @Test
+    public void safeQueryParamEscape_Test() {
+        assertThat(SafeStringManipulation.safeQueryParamEscape(null), is(nullValue()));
+        assertThat(SafeStringManipulation.safeQueryParamEscape(""), is(""));
+        assertThat(SafeStringManipulation.safeQueryParamEscape("one2three+four"), is("one2three%2Bfour"));
+        assertThat(SafeStringManipulation.safeQueryParamEscape("one two three"), is("one%20two%20three"));
+        assertThat(SafeStringManipulation.safeQueryParamEscape("one&two&three"), is("one%26two%26three"));
+    }
 }