From 392f2c27e1a3b43b7981e8594ea53efd86c66f70 Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Tue, 28 Jul 2020 20:01:20 +0300 Subject: [PATCH 1/9] fix: PostPolicyV4 classes could be improved --- .../google/cloud/storage/PostPolicyV4.java | 132 +++++++++++++++--- .../com/google/cloud/storage/Storage.java | 18 +-- 2 files changed, 122 insertions(+), 28 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java index cc9b413a2..46f06ccca 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java @@ -18,56 +18,123 @@ import com.google.gson.JsonArray; import com.google.gson.JsonObject; +import java.net.MalformedURLException; +import java.net.URL; import java.text.SimpleDateFormat; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.TimeUnit; /** - * Presigned V4 post policy. + * Presigned V4 post policy. Instances of {@code PostPolicyV4} include a URL and a map of fields + * that can be specified in an HTML form to submit a POST request to upload an object. * - * @see POST Object + *

See POST Object for + * details of upload by using HTML forms. + * + *

See {@link Storage#generateSignedPostPolicyV4(BlobInfo, long, TimeUnit, + * PostPolicyV4.PostFieldsV4, PostPolicyV4.PostConditionsV4, Storage.PostPolicyV4Option...)} for + * example of usage. */ public final class PostPolicyV4 { private final String url; private final Map fields; private PostPolicyV4(String url, Map fields) { + try { + new URL(url); + } catch (MalformedURLException e) { + throw new IllegalArgumentException(e); + } + PostFieldsV4.validateFields(fields); + this.url = url; - this.fields = fields; + this.fields = Collections.unmodifiableMap(fields); } + /** + * Constructs {@code PostPolicyV4} instance of the given URL and fields map. + * + * @param url URL for the HTTP POST request + * @param fields HTML form fields + * @return constructed object + * @throws IllegalArgumentException if URL is malformed or fields are not valid + */ public static PostPolicyV4 of(String url, Map fields) { return new PostPolicyV4(url, fields); } + /** Returns the URL for the HTTP POST request */ public String getUrl() { return url; } + /** Returns the HTML form fields */ public Map getFields() { return fields; } /** - * Class representing which fields to specify in a V4 POST request. + * A helper class to define fields to be specified in a V4 POST request. Instance of this class + * helps to construct {@code PostPolicyV4} objects. Used in: {@link + * Storage#generateSignedPostPolicyV4(BlobInfo, long, TimeUnit, PostPolicyV4.PostFieldsV4, + * PostPolicyV4.PostConditionsV4, Storage.PostPolicyV4Option...)} * * @see POST * Object Form fields */ public static final class PostFieldsV4 { private final Map fieldsMap; + private static final List VALID_FIELDS = + Arrays.asList( + "acl", + "bucket", + "cache-control", + "content-disposition", + "content-encoding", + "content-type", + "expires", + "file", + "key", + "policy", + "success_action_redirect", + "success_action_status", + "x-goog-algorithm", + "x-goog-credential", + "x-goog-date", + "x-goog-signature"); + + private static void validateFields(Map fields) { + for (String key : fields.keySet()) { + if (!VALID_FIELDS.contains(key.toLowerCase()) + && !key.startsWith(Builder.CUSTOM_FIELD_PREFIX)) { + throw new IllegalArgumentException("Invalid key: " + key); + } + } + } private PostFieldsV4(Builder builder) { - this.fieldsMap = builder.fieldsMap; + this(builder.fieldsMap); } private PostFieldsV4(Map fields) { - this.fieldsMap = fields; + validateFields(fields); + this.fieldsMap = Collections.unmodifiableMap(fields); } + /** + * Constructs {@code PostPolicyV4.PostFieldsV4} object of the given field map. + * + * @param fields a map + * @return constructed object + * @throws IllegalArgumentException if unsupported field is specified. + */ public static PostFieldsV4 of(Map fields) { return new PostFieldsV4(fields); } @@ -112,8 +179,9 @@ public Builder setContentEncoding(String contentEncoding) { return this; } + /** @deprecated Invocation this method has no effect */ + @Deprecated public Builder setContentLength(int contentLength) { - fieldsMap.put("content-length", "" + contentLength); return this; } @@ -160,7 +228,9 @@ public Builder setCustomMetadataField(String field, String value) { } /** - * Class for specifying conditions in a V4 POST Policy document. + * A helper class for specifying conditions in a V4 POST Policy document. Used in: {@link + * Storage#generateSignedPostPolicyV4(BlobInfo, long, TimeUnit, PostPolicyV4.PostFieldsV4, + * PostPolicyV4.PostConditionsV4, Storage.PostPolicyV4Option...)} * * @see * Policy document @@ -183,14 +253,14 @@ public static Builder newBuilder() { } public Set getConditions() { - return conditions; + return Collections.unmodifiableSet(conditions); } public static class Builder { - Set conditions; + private final Set conditions; private Builder() { - this.conditions = new LinkedHashSet<>(); + this(new LinkedHashSet()); } private Builder(Set conditions) { @@ -280,7 +350,7 @@ Builder addCustomCondition(ConditionV4Type type, String field, String value) { } /** - * Class for a V4 POST Policy document. + * Class for a V4 POST Policy document. Used by Storage to construct {@code PostPolicyV4} objects. * * @see * Policy document @@ -367,9 +437,20 @@ public String toJson() { } public enum ConditionV4Type { - MATCHES, - STARTS_WITH, - CONTENT_LENGTH_RANGE + MATCHES("eq"), + STARTS_WITH("starts-with"), + CONTENT_LENGTH_RANGE("content-length-range"); + + private final String name; + + ConditionV4Type(String name) { + this.name = name; + } + + @Override + public String toString() { + return name; + } } /** @@ -378,10 +459,10 @@ public enum ConditionV4Type { * @see * Policy document */ - static final class ConditionV4 { - final ConditionV4Type type; - final String operand1; - final String operand2; + public static final class ConditionV4 { + public final ConditionV4Type type; + public final String operand1; + public final String operand2; private ConditionV4(ConditionV4Type type, String operand1, String operand2) { this.type = type; @@ -401,5 +482,18 @@ public boolean equals(Object other) { public int hashCode() { return Objects.hash(type, operand1, operand2); } + + /** + * Examples of returned strings: {@code ["eq", "$key", "test-object"]}, {@code ["starts-with", + * "$acl", "public"]}, {@code ["content-length-range", 246, 266]} + */ + @Override + public String toString() { + String body = + type == ConditionV4Type.CONTENT_LENGTH_RANGE + ? operand1 + ", " + operand2 + : "\"$" + operand1 + "\", \"" + operand2 + "\""; + return "[\"" + type + "\", " + body + "]"; + } } } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java index 82ba17508..6b8fc4990 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java @@ -2843,6 +2843,8 @@ Blob createFrom( * } * * @param blobInfo the blob uploaded in the form + * @param duration time before expiration + * @param unit duration time unit * @param fields the fields specified in the form * @param conditions which conditions every upload must satisfy * @param duration how long until the form expires, in milliseconds @@ -2861,9 +2863,8 @@ PostPolicyV4 generateSignedPostPolicyV4( /** * Generates a presigned post policy without any conditions. Automatically creates required - * conditions. See full documentation for generateSignedPostPolicyV4( BlobInfo blobInfo, long - * duration, TimeUnit unit, PostFieldsV4 fields, PostConditionsV4 conditions, - * PostPolicyV4Option... options) above. + * conditions. See full documentation for {@link #generateSignedPostPolicyV4(BlobInfo, long, + * TimeUnit, PostPolicyV4.PostFieldsV4, PostPolicyV4.PostConditionsV4, PostPolicyV4Option...)}. */ PostPolicyV4 generateSignedPostPolicyV4( BlobInfo blobInfo, @@ -2874,9 +2875,8 @@ PostPolicyV4 generateSignedPostPolicyV4( /** * Generates a presigned post policy without any fields. Automatically creates required fields. - * See full documentation for generateSignedPostPolicyV4( BlobInfo blobInfo, long duration, - * TimeUnit unit, PostFieldsV4 fields, PostConditionsV4 conditions, PostPolicyV4Option... options) - * above. + * See full documentation for {@link #generateSignedPostPolicyV4(BlobInfo, long, TimeUnit, + * PostPolicyV4.PostFieldsV4, PostPolicyV4.PostConditionsV4, PostPolicyV4Option...)}. */ PostPolicyV4 generateSignedPostPolicyV4( BlobInfo blobInfo, @@ -2887,9 +2887,9 @@ PostPolicyV4 generateSignedPostPolicyV4( /** * Generates a presigned post policy without any fields or conditions. Automatically creates - * required fields and conditions. See full documentation for generateSignedPostPolicyV4( BlobInfo - * blobInfo, long duration, TimeUnit unit, PostFieldsV4 fields, PostConditionsV4 conditions, - * PostPolicyV4Option... options) above. + * required fields and conditions. See full documentation for {@link + * #generateSignedPostPolicyV4(BlobInfo, long, TimeUnit, PostPolicyV4.PostFieldsV4, + * PostPolicyV4.PostConditionsV4, PostPolicyV4Option...)}. */ PostPolicyV4 generateSignedPostPolicyV4( BlobInfo blobInfo, long duration, TimeUnit unit, PostPolicyV4Option... options); From a11a06d0896e702cd5a431e9d5bc01afc604d302 Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Wed, 29 Jul 2020 12:04:52 +0300 Subject: [PATCH 2/9] fix: javadoc --- .../com/google/cloud/storage/PostPolicyV4.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java index 46f06ccca..bfbfa6a90 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java @@ -84,7 +84,7 @@ public Map getFields() { * A helper class to define fields to be specified in a V4 POST request. Instance of this class * helps to construct {@code PostPolicyV4} objects. Used in: {@link * Storage#generateSignedPostPolicyV4(BlobInfo, long, TimeUnit, PostPolicyV4.PostFieldsV4, - * PostPolicyV4.PostConditionsV4, Storage.PostPolicyV4Option...)} + * PostPolicyV4.PostConditionsV4, Storage.PostPolicyV4Option...)}. * * @see POST * Object Form fields @@ -131,9 +131,9 @@ private PostFieldsV4(Map fields) { /** * Constructs {@code PostPolicyV4.PostFieldsV4} object of the given field map. * - * @param fields a map + * @param fields a map the HTML form fields * @return constructed object - * @throws IllegalArgumentException if unsupported field is specified. + * @throws IllegalArgumentException if an unsupported field is specified */ public static PostFieldsV4 of(Map fields) { return new PostFieldsV4(fields); @@ -179,7 +179,7 @@ public Builder setContentEncoding(String contentEncoding) { return this; } - /** @deprecated Invocation this method has no effect */ + /** @deprecated Invocation of this method has no effect. */ @Deprecated public Builder setContentLength(int contentLength) { return this; @@ -190,7 +190,7 @@ public Builder setContentType(String contentType) { return this; } - /** @deprecated use {@link #setExpires(String)} */ + /** @deprecated Use {@link #setExpires(String)}. */ @Deprecated public Builder Expires(String expires) { return setExpires(expires); @@ -211,7 +211,7 @@ public Builder setSuccessActionStatus(int successActionStatus) { return this; } - /** @deprecated use {@link #setCustomMetadataField(String, String)} */ + /** @deprecated Use {@link #setCustomMetadataField(String, String)}. */ @Deprecated public Builder AddCustomMetadataField(String field, String value) { return setCustomMetadataField(field, value); @@ -230,7 +230,7 @@ public Builder setCustomMetadataField(String field, String value) { /** * A helper class for specifying conditions in a V4 POST Policy document. Used in: {@link * Storage#generateSignedPostPolicyV4(BlobInfo, long, TimeUnit, PostPolicyV4.PostFieldsV4, - * PostPolicyV4.PostConditionsV4, Storage.PostPolicyV4Option...)} + * PostPolicyV4.PostConditionsV4, Storage.PostPolicyV4Option...)}. * * @see * Policy document @@ -485,7 +485,7 @@ public int hashCode() { /** * Examples of returned strings: {@code ["eq", "$key", "test-object"]}, {@code ["starts-with", - * "$acl", "public"]}, {@code ["content-length-range", 246, 266]} + * "$acl", "public"]}, {@code ["content-length-range", 246, 266]}. */ @Override public String toString() { From 00ecda4c9011cf51bef66f54be418ff385152d61 Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Wed, 29 Jul 2020 12:28:06 +0300 Subject: [PATCH 3/9] fix: javadoc --- .../main/java/com/google/cloud/storage/PostPolicyV4.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java index bfbfa6a90..816fa2c26 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java @@ -131,7 +131,7 @@ private PostFieldsV4(Map fields) { /** * Constructs {@code PostPolicyV4.PostFieldsV4} object of the given field map. * - * @param fields a map the HTML form fields + * @param fields a map of the HTML form fields * @return constructed object * @throws IllegalArgumentException if an unsupported field is specified */ @@ -179,7 +179,12 @@ public Builder setContentEncoding(String contentEncoding) { return this; } - /** @deprecated Invocation of this method has no effect. */ + /** + * @deprecated Invocation of this method has no effect, because All valid HTML form fields + * except Content-Length can use exact matching. Use {@link + * PostPolicyV4.PostConditionsV4.Builder#addContentLengthRangeCondition(int, int)} to + * specify a range for the content-length. + */ @Deprecated public Builder setContentLength(int contentLength) { return this; From d91689b5e0331b3d51125753457216b3ff83e34e Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Wed, 29 Jul 2020 12:48:50 +0300 Subject: [PATCH 4/9] fix: javadoc --- .../src/main/java/com/google/cloud/storage/PostPolicyV4.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java index 816fa2c26..05ade433e 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java @@ -180,7 +180,7 @@ public Builder setContentEncoding(String contentEncoding) { } /** - * @deprecated Invocation of this method has no effect, because All valid HTML form fields + * @deprecated Invocation of this method has no effect, because all valid HTML form fields * except Content-Length can use exact matching. Use {@link * PostPolicyV4.PostConditionsV4.Builder#addContentLengthRangeCondition(int, int)} to * specify a range for the content-length. From fb607dac958d6d13c4308e1127b82029698c9036 Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Wed, 29 Jul 2020 17:53:35 +0300 Subject: [PATCH 5/9] test: PostPolicyV4Test unit tests --- .../cloud/storage/PostPolicyV4Test.java | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java new file mode 100644 index 000000000..6feb80823 --- /dev/null +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java @@ -0,0 +1,158 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import static org.junit.Assert.*; + +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import org.junit.Test; + +public class PostPolicyV4Test { + + private void assertNotSameButEqual(Map expected, Map returned) { + assertNotSame(expected, returned); + assertEquals(expected.size(), returned.size()); + for (String key : expected.keySet()) { + assertEquals(expected.get(key), returned.get(key)); + } + } + + private static final String[] VALID_FIELDS = { + "acl", + "bucket", + "cache-control", + "content-disposition", + "content-encoding", + "content-type", + "expires", + "file", + "key", + "policy", + "success_action_redirect", + "success_action_status", + "x-goog-algorithm", + "x-goog-credential", + "x-goog-date", + "x-goog-signature", + }; + + private static Map initAllFields() { + Map fields = new HashMap<>(); + for (String key : VALID_FIELDS) { + fields.put(key, "value of " + key); + } + fields.put("x-goog-meta-custom", "value of custom field"); + return Collections.unmodifiableMap(fields); + } + + private static final Map ALL_FIELDS = initAllFields(); + + @Test + public void testPostPolicyV4_of() { + String url = "http://example.com"; + PostPolicyV4 policy = PostPolicyV4.of(url, ALL_FIELDS); + assertEquals(url, policy.getUrl()); + assertNotSameButEqual(ALL_FIELDS, policy.getFields()); + } + + @Test + public void testPostPolicyV4_ofMalformedURL() { + try { + PostPolicyV4.of("not a url", new HashMap()); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("java.net.MalformedURLException: no protocol: not a url", e.getMessage()); + } + } + + @Test + public void testPostPolicyV4_ofInvalidField() { + Map fields = new HashMap<>(ALL_FIELDS); + fields.put("$file", "file.txt"); + try { + PostPolicyV4.of("http://google.com", fields); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Invalid key: $file", e.getMessage()); + } + } + + @Test + public void testPostFieldsV4_of() { + PostPolicyV4.PostFieldsV4 fields = PostPolicyV4.PostFieldsV4.of(ALL_FIELDS); + assertNotSameButEqual(ALL_FIELDS, fields.getFieldsMap()); + } + + @Test + public void testPostFieldsV4_ofInvalidField() { + Map map = new HashMap<>(); + map.put("$file", "file.txt"); + try { + PostPolicyV4.PostFieldsV4.of(map); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Invalid key: $file", e.getMessage()); + } + } + + @Test + public void testPostConditionsV4_create() { + PostPolicyV4.PostConditionsV4.Builder builder = PostPolicyV4.PostConditionsV4.newBuilder(); + + assertTrue(builder.build().getConditions().isEmpty()); + + builder.addBucketCondition(PostPolicyV4.ConditionV4Type.MATCHES, "my-bucket"); + builder.addAclCondition(PostPolicyV4.ConditionV4Type.STARTS_WITH, "pub"); + Set conditions = builder.build().getConditions(); + + assertEquals(2, conditions.size()); + try { + conditions.clear(); + fail(); + } catch (UnsupportedOperationException e) { + // expected + } + } + + @Test + public void testPostConditionsV4_toString() { + PostPolicyV4.PostConditionsV4.Builder builder = PostPolicyV4.PostConditionsV4.newBuilder(); + builder.addKeyCondition(PostPolicyV4.ConditionV4Type.MATCHES, "test-object"); + builder.addAclCondition(PostPolicyV4.ConditionV4Type.STARTS_WITH, "public"); + builder.addContentLengthRangeCondition(246, 266); + + Set toStringSet = new HashSet<>(); + for (PostPolicyV4.ConditionV4 conditionV4 : builder.build().getConditions()) { + toStringSet.add(conditionV4.toString()); + } + assertEquals(3, toStringSet.size()); + + String[] expectedStrings = { + "[\"eq\", \"$key\", \"test-object\"]", + "[\"starts-with\", \"$acl\", \"public\"]", + "[\"content-length-range\", 246, 266]" + }; + + for (String expected : expectedStrings) { + assertTrue(expected + "/" + toStringSet, toStringSet.contains(expected)); + } + } +} From 7dec9170b84c93b4cc1398157891eb130f8657e2 Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Thu, 30 Jul 2020 13:44:47 +0300 Subject: [PATCH 6/9] test: PostPolicyV4Test unit tests --- .../google/cloud/storage/PostPolicyV4.java | 49 ++- .../cloud/storage/PostPolicyV4Test.java | 415 +++++++++++++++++- 2 files changed, 447 insertions(+), 17 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java index 05ade433e..e00d27a9d 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java @@ -224,7 +224,7 @@ public Builder AddCustomMetadataField(String field, String value) { public Builder setCustomMetadataField(String field, String value) { if (!field.startsWith(CUSTOM_FIELD_PREFIX)) { - field = CUSTOM_FIELD_PREFIX + value; + field = CUSTOM_FIELD_PREFIX + field; } fieldsMap.put(field, value); return this; @@ -281,64 +281,93 @@ public PostConditionsV4 build() { } public Builder addAclCondition(ConditionV4Type type, String acl) { + checkType(type, "acl"); conditions.add(new ConditionV4(type, "acl", acl)); return this; } public Builder addBucketCondition(ConditionV4Type type, String bucket) { + checkType(type, "bucket"); conditions.add(new ConditionV4(type, "bucket", bucket)); return this; } public Builder addCacheControlCondition(ConditionV4Type type, String cacheControl) { + checkType(type, "cache-control"); conditions.add(new ConditionV4(type, "cache-control", cacheControl)); return this; } public Builder addContentDispositionCondition( ConditionV4Type type, String contentDisposition) { + checkType(type, "content-disposition"); conditions.add(new ConditionV4(type, "content-disposition", contentDisposition)); return this; } public Builder addContentEncodingCondition(ConditionV4Type type, String contentEncoding) { + checkType(type, "content-encoding"); conditions.add(new ConditionV4(type, "content-encoding", contentEncoding)); return this; } + /** + * @deprecated Invocation of this method has no effect. Use {@link + * #addContentLengthRangeCondition(int, int)} to specify a range for the content-length. + */ public Builder addContentLengthCondition(ConditionV4Type type, int contentLength) { - conditions.add(new ConditionV4(type, "content-length", "" + contentLength)); return this; } public Builder addContentTypeCondition(ConditionV4Type type, String contentType) { + checkType(type, "content-type"); conditions.add(new ConditionV4(type, "content-type", contentType)); return this; } + /** @deprecated Use {@link #addExpiresCondition(long)} */ + @Deprecated public Builder addExpiresCondition(ConditionV4Type type, long expires) { - conditions.add(new ConditionV4(type, "expires", dateFormat.format(expires))); - return this; + return addExpiresCondition(expires); } + /** @deprecated Use {@link #addExpiresCondition(String)} */ + @Deprecated public Builder addExpiresCondition(ConditionV4Type type, String expires) { - conditions.add(new ConditionV4(type, "expires", expires)); + return addExpiresCondition(expires); + } + + public Builder addExpiresCondition(long expires) { + return addExpiresCondition(dateFormat.format(expires)); + } + + public Builder addExpiresCondition(String expires) { + conditions.add(new ConditionV4(ConditionV4Type.MATCHES, "expires", expires)); return this; } public Builder addKeyCondition(ConditionV4Type type, String key) { + checkType(type, "key"); conditions.add(new ConditionV4(type, "key", key)); return this; } public Builder addSuccessActionRedirectUrlCondition( ConditionV4Type type, String successActionRedirectUrl) { + checkType(type, "success_action_redirect"); conditions.add(new ConditionV4(type, "success_action_redirect", successActionRedirectUrl)); return this; } + /** @deprecated Use {@link #addSuccessActionStatusCondition(int)} */ + @Deprecated public Builder addSuccessActionStatusCondition(ConditionV4Type type, int status) { - conditions.add(new ConditionV4(type, "success_action_status", "" + status)); + return addSuccessActionStatusCondition(status); + } + + public Builder addSuccessActionStatusCondition(int status) { + conditions.add( + new ConditionV4(ConditionV4Type.MATCHES, "success_action_status", "" + status)); return this; } @@ -351,6 +380,12 @@ Builder addCustomCondition(ConditionV4Type type, String field, String value) { conditions.add(new ConditionV4(type, field, value)); return this; } + + private void checkType(ConditionV4Type type, String field) { + if (type != ConditionV4Type.MATCHES && type != ConditionV4Type.STARTS_WITH) { + throw new IllegalArgumentException("Field " + field + " can't use " + type); + } + } } } @@ -469,7 +504,7 @@ public static final class ConditionV4 { public final String operand1; public final String operand2; - private ConditionV4(ConditionV4Type type, String operand1, String operand2) { + ConditionV4(ConditionV4Type type, String operand1, String operand2) { this.type = type; this.operand1 = operand1; this.operand2 = operand2; diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java index 6feb80823..4382c3aac 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java @@ -16,22 +16,26 @@ package com.google.cloud.storage; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.Callable; import org.junit.Test; public class PostPolicyV4Test { private void assertNotSameButEqual(Map expected, Map returned) { assertNotSame(expected, returned); - assertEquals(expected.size(), returned.size()); + assertEquals("map sizes", expected.size(), returned.size()); for (String key : expected.keySet()) { - assertEquals(expected.get(key), returned.get(key)); + assertEquals("value of $" + key, expected.get(key), returned.get(key)); } } @@ -54,12 +58,14 @@ private void assertNotSameButEqual(Map expected, Map initAllFields() { Map fields = new HashMap<>(); for (String key : VALID_FIELDS) { fields.put(key, "value of " + key); } - fields.put("x-goog-meta-custom", "value of custom field"); + fields.put(CUSTOM_PREFIX + "custom", "value of custom field"); return Collections.unmodifiableMap(fields); } @@ -114,22 +120,411 @@ public void testPostFieldsV4_ofInvalidField() { } @Test - public void testPostConditionsV4_create() { - PostPolicyV4.PostConditionsV4.Builder builder = PostPolicyV4.PostConditionsV4.newBuilder(); + public void testPostPolicyV4_builder() { + PostPolicyV4.PostFieldsV4.Builder builder = PostPolicyV4.PostFieldsV4.newBuilder(); + builder.setAcl("acl"); + builder.setCacheControl("cache-control"); + builder.setContentDisposition("content-disposition"); + builder.setContentType("content-type"); + builder.setExpires("expires"); + builder.setSuccessActionRedirect("success_action_redirect"); + Map map = builder.build().getFieldsMap(); + assertEquals("map size", 6, map.size()); + for (String key : map.keySet()) { + assertEquals("value of $" + key, key, map.get(key)); + } + + Map expectedUpdated = new HashMap<>(map); + builder.setCustomMetadataField("xxx", "XXX"); + builder.setCustomMetadataField(CUSTOM_PREFIX + "yyy", "YYY"); + builder.setAcl(null); + builder.setContentType("new-content-type"); + builder.setSuccessActionStatus(42); + expectedUpdated.put(CUSTOM_PREFIX + "xxx", "XXX"); + expectedUpdated.put(CUSTOM_PREFIX + "yyy", "YYY"); + expectedUpdated.put("acl", null); + expectedUpdated.put("content-type", "new-content-type"); + expectedUpdated.put("success_action_status", "42"); + Map updated = builder.build().getFieldsMap(); + assertNotSameButEqual(expectedUpdated, updated); + } + + @Test + public void testPostPolicyV4_setContentLength() { + PostPolicyV4.PostFieldsV4.Builder builder = PostPolicyV4.PostFieldsV4.newBuilder(); + builder.setContentLength(12345); + assertTrue(builder.build().getFieldsMap().isEmpty()); + } + @Test + public void testPostConditionsV4_builder() { + PostPolicyV4.PostConditionsV4.Builder builder = PostPolicyV4.PostConditionsV4.newBuilder(); assertTrue(builder.build().getConditions().isEmpty()); - builder.addBucketCondition(PostPolicyV4.ConditionV4Type.MATCHES, "my-bucket"); - builder.addAclCondition(PostPolicyV4.ConditionV4Type.STARTS_WITH, "pub"); - Set conditions = builder.build().getConditions(); + builder.addAclCondition(PostPolicyV4.ConditionV4Type.STARTS_WITH, "public"); + builder.addBucketCondition(PostPolicyV4.ConditionV4Type.MATCHES, "travel-maps"); + builder.addContentLengthRangeCondition(0, 100000); + + PostPolicyV4.PostConditionsV4 postConditionsV4 = builder.build(); + Set conditions = postConditionsV4.getConditions(); + assertEquals(3, conditions.size()); - assertEquals(2, conditions.size()); try { conditions.clear(); fail(); } catch (UnsupportedOperationException e) { // expected } + + PostPolicyV4.PostConditionsV4 postConditionsV4Extended = + postConditionsV4 + .toBuilder() + .addCustomCondition(PostPolicyV4.ConditionV4Type.STARTS_WITH, "key", "") + .build(); + assertEquals(4, postConditionsV4Extended.getConditions().size()); + } + + interface ConditionTest { + /** + * Calls one of addCondition method on the given builder and returns expected ConditionV4 + * object. + */ + PostPolicyV4.ConditionV4 addCondition(PostPolicyV4.PostConditionsV4.Builder builder); + } + + @Test + public void testPostConditionsV4_addCondition() { + // shortcuts + final PostPolicyV4.ConditionV4Type eq = PostPolicyV4.ConditionV4Type.MATCHES; + final PostPolicyV4.ConditionV4Type startsWith = PostPolicyV4.ConditionV4Type.STARTS_WITH; + final PostPolicyV4.ConditionV4Type range = PostPolicyV4.ConditionV4Type.CONTENT_LENGTH_RANGE; + + ConditionTest[] cases = { + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addContentLengthRangeCondition(123, 456); + return new PostPolicyV4.ConditionV4(range, "123", "456"); + } + + @Override + public String toString() { + return "addContentLengthRangeCondition()"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addExpiresCondition(2000000000000L); + return new PostPolicyV4.ConditionV4(eq, "expires", "2033-05-18T06:33:20Z"); + } + + @Override + public String toString() { + return "addExpiresCondition(long)"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addExpiresCondition("2030-Dec-31"); + return new PostPolicyV4.ConditionV4(eq, "expires", "2030-Dec-31"); + } + + @Override + public String toString() { + return "addExpiresCondition(String)"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addExpiresCondition(range, 2000000000000L); + return new PostPolicyV4.ConditionV4(eq, "expires", "2033-05-18T06:33:20Z"); + } + + @Override + public String toString() { + return "@deprecated addExpiresCondition(type,long)"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addExpiresCondition(startsWith, "2030-Dec-31"); + return new PostPolicyV4.ConditionV4(eq, "expires", "2030-Dec-31"); + } + + @Override + public String toString() { + return "@deprecated addExpiresCondition(type,String)"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addSuccessActionStatusCondition(202); + return new PostPolicyV4.ConditionV4(eq, "success_action_status", "202"); + } + + @Override + public String toString() { + return "addSuccessActionStatusCondition(int)"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addSuccessActionStatusCondition(startsWith, 202); + return new PostPolicyV4.ConditionV4(eq, "success_action_status", "202"); + } + + @Override + public String toString() { + return "@deprecated addSuccessActionStatusCondition(type,int)"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addAclCondition(startsWith, "read"); + return new PostPolicyV4.ConditionV4(startsWith, "acl", "read"); + } + + @Override + public String toString() { + return "addAclCondition()"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addBucketCondition(eq, "my-bucket"); + return new PostPolicyV4.ConditionV4(eq, "bucket", "my-bucket"); + } + + @Override + public String toString() { + return "addBucketCondition()"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addCacheControlCondition(eq, "false"); + return new PostPolicyV4.ConditionV4(eq, "cache-control", "false"); + } + + @Override + public String toString() { + return "addCacheControlCondition()"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addContentDispositionCondition(startsWith, "gzip"); + return new PostPolicyV4.ConditionV4(startsWith, "content-disposition", "gzip"); + } + + @Override + public String toString() { + return "addContentDispositionCondition()"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addContentEncodingCondition(eq, "koi8"); + return new PostPolicyV4.ConditionV4(eq, "content-encoding", "koi8"); + } + + @Override + public String toString() { + return "addContentEncodingCondition()"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addContentTypeCondition(startsWith, "application/"); + return new PostPolicyV4.ConditionV4(startsWith, "content-type", "application/"); + } + + @Override + public String toString() { + return "addContentTypeCondition()"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addKeyCondition(startsWith, ""); + return new PostPolicyV4.ConditionV4(startsWith, "key", ""); + } + + @Override + public String toString() { + return "addKeyCondition()"; + } + }, + new ConditionTest() { + @Override + public PostPolicyV4.ConditionV4 addCondition( + PostPolicyV4.PostConditionsV4.Builder builder) { + builder.addSuccessActionRedirectUrlCondition(eq, "fail"); + return new PostPolicyV4.ConditionV4(eq, "success_action_redirect", "fail"); + } + + @Override + public String toString() { + return "addSuccessActionRedirectUrlCondition()"; + } + }, + }; + + for (ConditionTest testCase : cases) { + PostPolicyV4.PostConditionsV4.Builder builder = PostPolicyV4.PostConditionsV4.newBuilder(); + PostPolicyV4.ConditionV4 expected = testCase.addCondition(builder); + Set conditions = builder.build().getConditions(); + assertEquals("size", 1, conditions.size()); + PostPolicyV4.ConditionV4 actual = conditions.toArray(new PostPolicyV4.ConditionV4[1])[0]; + assertEquals(testCase.toString(), expected, actual); + } + } + + @Test + public void testPostConditionsV4_addConditionFail() { + final PostPolicyV4.PostConditionsV4.Builder builder = + PostPolicyV4.PostConditionsV4.newBuilder(); + final PostPolicyV4.ConditionV4Type range = PostPolicyV4.ConditionV4Type.CONTENT_LENGTH_RANGE; + + Callable[] cases = { + new Callable() { + @Override + public Void call() { + builder.addAclCondition(range, ""); + return null; + } + + @Override + public String toString() { + return "acl"; + } + }, + new Callable() { + @Override + public Void call() { + builder.addBucketCondition(range, ""); + return null; + } + + @Override + public String toString() { + return "bucket"; + } + }, + new Callable() { + @Override + public Void call() { + builder.addCacheControlCondition(range, ""); + return null; + } + + @Override + public String toString() { + return "cache-control"; + } + }, + new Callable() { + @Override + public Void call() { + builder.addContentDispositionCondition(range, ""); + return null; + } + + @Override + public String toString() { + return "content-disposition"; + } + }, + new Callable() { + @Override + public Void call() { + builder.addContentEncodingCondition(range, ""); + return null; + } + + @Override + public String toString() { + return "content-encoding"; + } + }, + new Callable() { + @Override + public Void call() { + builder.addContentTypeCondition(range, ""); + return null; + } + + @Override + public String toString() { + return "content-type"; + } + }, + new Callable() { + @Override + public Void call() { + builder.addKeyCondition(range, ""); + return null; + } + + @Override + public String toString() { + return "key"; + } + }, + new Callable() { + @Override + public Void call() { + builder.addSuccessActionRedirectUrlCondition(range, ""); + return null; + } + + @Override + public String toString() { + return "success_action_redirect"; + } + }, + }; + + for (Callable testCase : cases) { + try { + testCase.call(); + fail(); + } catch (Exception e) { + String expected = + "java.lang.IllegalArgumentException: Field " + + testCase + + " can't use content-length-range"; + assertEquals(expected, e.toString()); + } + } + assertTrue(builder.build().getConditions().isEmpty()); } @Test From 3256a0a9e4025630dee42009465d92e31aff88d4 Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Thu, 30 Jul 2020 13:54:56 +0300 Subject: [PATCH 7/9] test: PostPolicyV4Test unit tests --- .../com/google/cloud/storage/PostPolicyV4Test.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java index 4382c3aac..e7263a6fc 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.text.SimpleDateFormat; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -30,6 +31,7 @@ import org.junit.Test; public class PostPolicyV4Test { + private static SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); private void assertNotSameButEqual(Map expected, Map returned) { assertNotSame(expected, returned); @@ -217,8 +219,9 @@ public String toString() { @Override public PostPolicyV4.ConditionV4 addCondition( PostPolicyV4.PostConditionsV4.Builder builder) { - builder.addExpiresCondition(2000000000000L); - return new PostPolicyV4.ConditionV4(eq, "expires", "2033-05-18T06:33:20Z"); + long date = 2000000000000L; + builder.addExpiresCondition(date); + return new PostPolicyV4.ConditionV4(eq, "expires", dateFormat.format(date)); } @Override @@ -243,8 +246,8 @@ public String toString() { @Override public PostPolicyV4.ConditionV4 addCondition( PostPolicyV4.PostConditionsV4.Builder builder) { - builder.addExpiresCondition(range, 2000000000000L); - return new PostPolicyV4.ConditionV4(eq, "expires", "2033-05-18T06:33:20Z"); + builder.addExpiresCondition(range, 0); + return new PostPolicyV4.ConditionV4(eq, "expires", dateFormat.format(0)); } @Override From fc8e2d0a8005e6fab3827f07621cae22d29fdd7a Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Thu, 30 Jul 2020 15:41:49 +0300 Subject: [PATCH 8/9] test: PostPolicyV4Test unit tests --- .../cloud/storage/PostPolicyV4Test.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java index e7263a6fc..883d00329 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java @@ -553,4 +553,31 @@ public void testPostConditionsV4_toString() { assertTrue(expected + "/" + toStringSet, toStringSet.contains(expected)); } } + + @Test + public void testPostPolicyV4Document_of_toJson() { + PostPolicyV4.PostConditionsV4 emptyConditions = + PostPolicyV4.PostConditionsV4.newBuilder().build(); + PostPolicyV4.PostPolicyV4Document emptyDocument = + PostPolicyV4.PostPolicyV4Document.of("", emptyConditions); + String emptyJson = emptyDocument.toJson(); + assertEquals(emptyJson, "{\"conditions\":[],\"expiration\":\"\"}"); + + PostPolicyV4.PostConditionsV4 postConditionsV4 = + PostPolicyV4.PostConditionsV4.newBuilder() + .addBucketCondition(PostPolicyV4.ConditionV4Type.MATCHES, "my-bucket") + .addKeyCondition(PostPolicyV4.ConditionV4Type.STARTS_WITH, "") + .addContentLengthRangeCondition(1, 1000) + .build(); + + String expiration = dateFormat.format(System.currentTimeMillis()); + PostPolicyV4.PostPolicyV4Document document = + PostPolicyV4.PostPolicyV4Document.of(expiration, postConditionsV4); + String json = document.toJson(); + assertEquals( + json, + "{\"conditions\":[{\"bucket\":\"my-bucket\"},[\"starts-with\",\"$key\",\"\"],[\"content-length-range\",1,1000]],\"expiration\":\"" + + expiration + + "\"}"); + } } From b5a4f90ed0ee853dbac5b2c7e0c586915cff9870 Mon Sep 17 00:00:00 2001 From: dmitry-fa Date: Fri, 21 Aug 2020 17:26:30 +0300 Subject: [PATCH 9/9] fix: better URL syntax check --- .../java/com/google/cloud/storage/PostPolicyV4.java | 10 ++++++---- .../com/google/cloud/storage/PostPolicyV4Test.java | 13 +++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java index e00d27a9d..96afca06e 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/PostPolicyV4.java @@ -18,8 +18,8 @@ import com.google.gson.JsonArray; import com.google.gson.JsonObject; -import java.net.MalformedURLException; -import java.net.URL; +import java.net.URI; +import java.net.URISyntaxException; import java.text.SimpleDateFormat; import java.util.Arrays; import java.util.Collections; @@ -48,8 +48,10 @@ public final class PostPolicyV4 { private PostPolicyV4(String url, Map fields) { try { - new URL(url); - } catch (MalformedURLException e) { + if (!new URI(url).isAbsolute()) { + throw new IllegalArgumentException(url + " is not an absolute URL"); + } + } catch (URISyntaxException e) { throw new IllegalArgumentException(e); } PostFieldsV4.validateFields(fields); diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java index 883d00329..8213f6093 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/PostPolicyV4Test.java @@ -84,10 +84,19 @@ public void testPostPolicyV4_of() { @Test public void testPostPolicyV4_ofMalformedURL() { try { - PostPolicyV4.of("not a url", new HashMap()); + PostPolicyV4.of("example.com", new HashMap()); fail(); } catch (IllegalArgumentException e) { - assertEquals("java.net.MalformedURLException: no protocol: not a url", e.getMessage()); + assertEquals("example.com is not an absolute URL", e.getMessage()); + } + + try { + PostPolicyV4.of("Scio nescio", new HashMap()); + fail(); + } catch (IllegalArgumentException e) { + assertEquals( + "java.net.URISyntaxException: Illegal character in path at index 4: Scio nescio", + e.getMessage()); } }