From a76845a6bb4e630ca05e928f5c9c74b025a4e89f Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Mon, 3 Feb 2020 16:24:08 -0800 Subject: [PATCH 1/5] Update V4 signature to pass conformance tests --- .../google/cloud/storage/SignatureInfo.java | 4 +- .../com/google/cloud/storage/Storage.java | 42 +++++++++++++++++++ .../com/google/cloud/storage/StorageImpl.java | 25 +++++++---- .../google/cloud/storage/V4SigningTest.java | 23 +++++++--- pom.xml | 2 +- 5 files changed, 81 insertions(+), 15 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/SignatureInfo.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/SignatureInfo.java index f26157d9d..03630c61a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/SignatureInfo.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/SignatureInfo.java @@ -169,7 +169,9 @@ private String constructV4CanonicalRequestHash() { canonicalRequest .append(serializer.serializeHeaderNames(canonicalizedExtensionHeaders)) .append(COMPONENT_SEPARATOR); - canonicalRequest.append("UNSIGNED-PAYLOAD"); + + String userProvidedHash = canonicalizedExtensionHeaders.get("X-Goog-Content-SHA256"); + canonicalRequest.append(userProvidedHash == null ? "UNSIGNED-PAYLOAD" : userProvidedHash); return Hashing.sha256() .hashString(canonicalRequest.toString(), StandardCharsets.UTF_8) 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 77c88e97c..6471220d4 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 @@ -161,6 +161,21 @@ public String getSelector() { } } + enum UriScheme { + HTTP("http"), + HTTPS("https"); + + private final String scheme; + + UriScheme(String scheme) { + this.scheme = scheme; + } + + public String getScheme() { + return scheme; + } + } + /** Class for specifying bucket target options. */ class BucketTargetOption extends Option { @@ -1038,6 +1053,7 @@ enum Option { HOST_NAME, PATH_STYLE, VIRTUAL_HOSTED_STYLE, + BUCKET_BOUND_HOST_NAME, QUERY_PARAMS } @@ -1160,6 +1176,32 @@ public static SignUrlOption withPathStyle() { return new SignUrlOption(Option.PATH_STYLE, ""); } + /** + * Use a bucket-bound hostname, which replaces the storage.googleapis.com host with the name of + * a CNAME bucket, e.g. a bucket named 'gcs-subdomain.my.domain.tld'. Note that this cannot be + * used alongside {@code withVirtualHostedStyle()} or {@code withPathStyle()}. This method + * signature uses HTTP for the URI scheme, and is equivalent to calling {@code + * withBucketBoundHostname("...", UriScheme.HTTP).} @ see CNAME Redirects + */ + public static SignUrlOption withBucketBoundHostname(String bucketBoundHostname) { + return withBucketBoundHostname(bucketBoundHostname, UriScheme.HTTP); + } + + /** + * Use a bucket-bound hostname, which replaces the storage.googleapis.com host with the name of + * a CNAME bucket, e.g. a bucket named 'gcs-subdomain.my.domain.tld'. Note that this cannot be + * used alongside {@code withVirtualHostedStyle()} or {@code withPathStyle()}. The bucket name + * itself should not include the URI scheme (http or https), so it is specified via a local + * enum. @ see CNAME + * Redirects + */ + public static SignUrlOption withBucketBoundHostname( + String bucketBoundHostname, UriScheme uriScheme) { + return new SignUrlOption( + Option.BUCKET_BOUND_HOST_NAME, uriScheme.getScheme() + "://" + bucketBoundHostname); + } + /** * Use if the URL should contain additional query parameters. * diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java index 63c198557..d76aa0d29 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java @@ -660,8 +660,10 @@ public URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOptio checkArgument( !(optionMap.containsKey(SignUrlOption.Option.VIRTUAL_HOSTED_STYLE) - && optionMap.containsKey(SignUrlOption.Option.PATH_STYLE)), - "Cannot specify both the VIRTUAL_HOSTED_STYLE and PATH_STYLE SignUrlOptions together."); + && optionMap.containsKey(SignUrlOption.Option.PATH_STYLE) + && optionMap.containsKey(SignUrlOption.Option.BUCKET_BOUND_HOST_NAME)), + "Only one of VIRTUAL_HOSTED_STYLE, PATH_STYLE, or BUCKET_BOUND_HOST_NAME SignUrlOptions can be" + + " specified."); String bucketName = slashlessBucketNameFromBlobInfo(blobInfo); String escapedBlobName = ""; @@ -676,6 +678,10 @@ public URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOptio ? STORAGE_XML_URI_SCHEME + "://" + getBaseStorageHostName(optionMap) : STORAGE_XML_URI_SCHEME + "://" + bucketName + "." + getBaseStorageHostName(optionMap); + if (optionMap.containsKey(SignUrlOption.Option.BUCKET_BOUND_HOST_NAME)) { + storageXmlHostName = (String) optionMap.get(SignUrlOption.Option.BUCKET_BOUND_HOST_NAME); + } + String stPath = usePathStyle ? constructResourceUriPath(bucketName, escapedBlobName, optionMap) @@ -753,9 +759,7 @@ private String constructResourceUriPath( } return pathBuilder.toString(); } - if (!escapedBlobName.startsWith(PATH_DELIMITER)) { - pathBuilder.append(PATH_DELIMITER); - } + pathBuilder.append(PATH_DELIMITER); pathBuilder.append(escapedBlobName); return pathBuilder.toString(); } @@ -776,7 +780,8 @@ private SignUrlOption.SignatureVersion getPreferredSignatureVersion( private boolean shouldUsePathStyleForSignedUrl(EnumMap optionMap) { // TODO(#6362): If we decide to change the default style used to generate URLs, switch this // logic to return false unless PATH_STYLE was explicitly specified. - if (optionMap.containsKey(SignUrlOption.Option.VIRTUAL_HOSTED_STYLE)) { + if (optionMap.containsKey(SignUrlOption.Option.VIRTUAL_HOSTED_STYLE) + || optionMap.containsKey(SignUrlOption.Option.BUCKET_BOUND_HOST_NAME)) { return false; } return true; @@ -836,7 +841,8 @@ private SignatureInfo buildSignatureInfo( extHeadersBuilder.put( "host", slashlessBucketNameFromBlobInfo(blobInfo) + "." + getBaseStorageHostName(optionMap)); - } else if (optionMap.containsKey(SignUrlOption.Option.HOST_NAME)) { + } else if (optionMap.containsKey(SignUrlOption.Option.HOST_NAME) + || optionMap.containsKey(SignUrlOption.Option.BUCKET_BOUND_HOST_NAME)) { extHeadersBuilder.put("host", getBaseStorageHostName(optionMap)); } } @@ -868,9 +874,14 @@ private String slashlessBucketNameFromBlobInfo(BlobInfo blobInfo) { /** Returns the hostname used to send requests to Cloud Storage, e.g. "storage.googleapis.com". */ private String getBaseStorageHostName(Map optionMap) { String specifiedBaseHostName = (String) optionMap.get(SignUrlOption.Option.HOST_NAME); + String bucketBoundHostName = + (String) optionMap.get(SignUrlOption.Option.BUCKET_BOUND_HOST_NAME); if (!Strings.isNullOrEmpty(specifiedBaseHostName)) { return specifiedBaseHostName.replaceFirst("http(s)?://", ""); } + if (!Strings.isNullOrEmpty(bucketBoundHostName)) { + return bucketBoundHostName.replaceFirst("http(s)?://", ""); + } return STORAGE_XML_URI_HOST_NAME; } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/V4SigningTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/V4SigningTest.java index 8e1ad3104..582c0d0eb 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/V4SigningTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/V4SigningTest.java @@ -20,6 +20,7 @@ import static org.hamcrest.core.IsNot.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; import static org.junit.Assume.assumeThat; import com.google.api.core.ApiClock; @@ -94,11 +95,6 @@ public V4SigningTest( @Test public void test() { - assumeThat( - "Test skipped until b/136171758 is resolved", - testName.getMethodName(), - is(not("test[Headers should be trimmed]"))); - Storage storage = RemoteStorageHelper.create() .getOptions() @@ -110,6 +106,19 @@ public void test() { BlobInfo blob = BlobInfo.newBuilder(testData.getBucket(), testData.getObject()).build(); + SignUrlOption style = SignUrlOption.withPathStyle(); + + if (testData.getUrlStyle().equals(SigningV4Test.UrlStyle.VIRTUAL_HOSTED_STYLE)) { + style = SignUrlOption.withVirtualHostedStyle(); + } else if (testData.getUrlStyle().equals(SigningV4Test.UrlStyle.PATH_STYLE)) { + style = SignUrlOption.withPathStyle(); + } else if (testData.getUrlStyle().equals(SigningV4Test.UrlStyle.BUCKET_BOUND_DOMAIN)) { + style = + SignUrlOption.withBucketBoundHostname( + testData.getBucketBoundDomain(), + Storage.UriScheme.valueOf(testData.getScheme().toUpperCase())); + } + final String signedUrl = storage .signUrl( @@ -118,7 +127,9 @@ public void test() { TimeUnit.SECONDS, SignUrlOption.httpMethod(HttpMethod.valueOf(testData.getMethod())), SignUrlOption.withExtHeaders(testData.getHeadersMap()), - SignUrlOption.withV4Signature()) + SignUrlOption.withV4Signature(), + SignUrlOption.withQueryParams(testData.getQueryParametersMap()), + style) .toString(); assertEquals(testData.getExpectedUrl(), signedUrl); } diff --git a/pom.xml b/pom.xml index 03489e8c5..46adf3b7f 100644 --- a/pom.xml +++ b/pom.xml @@ -212,7 +212,7 @@ com.google.cloud google-cloud-conformance-tests - 0.0.4 + 0.0.5 test From a413ae61afa85371404cf390fda8947a1ae98c11 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 30 Apr 2020 08:19:40 -0700 Subject: [PATCH 2/5] unrevert InternalExtensionOnly --- .../src/main/java/com/google/cloud/storage/Storage.java | 2 ++ 1 file changed, 2 insertions(+) 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 2163ac197..62a13c55f 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 @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import com.google.api.core.InternalExtensionOnly; import com.google.api.gax.paging.Page; import com.google.auth.ServiceAccountSigner; import com.google.auth.ServiceAccountSigner.SigningException; @@ -57,6 +58,7 @@ * * @see Google Cloud Storage */ +@InternalExtensionOnly public interface Storage extends Service { enum PredefinedAcl { From c323ac9225ce42a55cea8015a249c54504f8f5b1 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 30 Apr 2020 08:30:05 -0700 Subject: [PATCH 3/5] revert accidental deletion of comment updates --- .../com/google/cloud/storage/Storage.java | 80 ++++++++++--------- 1 file changed, 43 insertions(+), 37 deletions(-) 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 62a13c55f..991638ebb 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 @@ -2010,6 +2010,8 @@ Blob create( * BlobId blobId = BlobId.of(bucketName, blobName); * Blob blob = storage.get(blobId, BlobGetOption.decryptionKey(blobEncryptionKey)); * } + * + * @throws StorageException upon failure */ Blob get(BlobId blob, BlobGetOption... options); @@ -2101,11 +2103,23 @@ Blob create( * {@link StorageException} is thrown. * * 
{@code
-   * String bucketName = "my-unique-bucket";
-   * String blobName = "my-blob-name";
-   * Blob blob = storage.get(bucketName, blobName);
-   * BlobInfo updatedInfo = blob.toBuilder().setContentType("text/plain").build();
-   * storage.update(updatedInfo, BlobTargetOption.metagenerationMatch());
+   * BlobId blobId = BlobId.of(bucketName, blobName);
+   * BlobInfo blobInfo = BlobInfo.newBuilder(blobId).setContentType("text/plain").build();
+   * Blob blob = storage.create(blobInfo);
+   *
+   * doSomething();
+   *
+   * BlobInfo update = blob.toBuilder().setContentType("multipart/form-data").build();
+   * Storage.BlobTargetOption option = Storage.BlobTargetOption.metagenerationMatch();
+   * try {
+   *   storage.update(update, option);
+   * } catch (StorageException e) {
+   *   if (e.getCode() == 412) {
+   *     // the properties were updated externally
+   *   } else {
+   *     throw e;
+   *   }
+   * }
    * }
* * @return the updated blob @@ -2114,37 +2128,29 @@ Blob create( Blob update(BlobInfo blobInfo, BlobTargetOption... options); /** - * Updates blob information. Original metadata are merged with metadata in the provided {@code - * blobInfo}. If the original metadata already contains a key specified in the provided {@code - * blobInfo's} metadata map, it will be replaced by the new value. Removing metadata can be done - * by setting that metadata's value to {@code null}. + * Updates the properties of the blob. This method issues an RPC request to merge the current blob + * properties with the properties in the provided {@code blobInfo}. Properties not defined in + * {@code blobInfo} will not be updated. To unset a blob property this property in {@code + * blobInfo} should be explicitly set to {@code null}. * - *

Example of adding new metadata values or updating existing ones. + *

Example of how to update blob's user provided metadata and unset the content type: * * 

{@code
-   * String bucketName = "my-unique-bucket";
-   * String blobName = "my-blob-name";
-   * Map newMetadata = new HashMap<>();
-   * newMetadata.put("keyToAddOrUpdate", "value");
-   * Blob blob = storage.update(BlobInfo.newBuilder(bucketName, blobName)
-   *     .setMetadata(newMetadata)
-   *     .build());
-   * }
- * - *

Example of removing metadata values. - * - * 

{@code
-   * String bucketName = "my-unique-bucket";
-   * String blobName = "my-blob-name";
-   * Map newMetadata = new HashMap<>();
-   * newMetadata.put("keyToRemove", null);
-   * Blob blob = storage.update(BlobInfo.newBuilder(bucketName, blobName)
-   *     .setMetadata(newMetadata)
-   *     .build());
+   * Map metadataUpdate = new HashMap<>();
+   * metadataUpdate.put("keyToAdd", "new value");
+   * metadataUpdate.put("keyToRemove", null);
+   * BlobInfo blobUpdate = BlobInfo.newBuilder(bucketName, blobName)
+   *     .setMetadata(metadataUpdate)
+   *     .setContentType(null)
+   *     .build();
+   * Blob blob = storage.update(blobUpdate);
    * }
* + * @param blobInfo information to update * @return the updated blob * @throws StorageException upon failure + * @see https://cloud.google.com/storage/docs/json_api/v1/objects/update */ Blob update(BlobInfo blobInfo); @@ -2770,10 +2776,10 @@ PostPolicyV4 generateSignedPostPolicyV4( List get(Iterable blobIds); /** - * Updates the requested blobs. A batch request is used to perform this call. Original metadata - * are merged with metadata in the provided {@code BlobInfo} objects. To replace metadata instead - * you first have to unset them. Unsetting metadata can be done by setting the provided {@code - * BlobInfo} objects metadata to {@code null}. See {@link #update(BlobInfo)} for a code example. + * Updates the requested blobs. A batch request is used to perform this call. The original + * properties are merged with the properties in the provided {@code BlobInfo} objects. Unsetting a + * property can be done by setting the property of the provided {@code BlobInfo} objects to {@code + * null}. See {@link #update(BlobInfo)} for a code example. * *

Example of updating information on several blobs using a single batch request. * @@ -2796,10 +2802,10 @@ PostPolicyV4 generateSignedPostPolicyV4( List update(BlobInfo... blobInfos); /** - * Updates the requested blobs. A batch request is used to perform this call. Original metadata - * are merged with metadata in the provided {@code BlobInfo} objects. To replace metadata instead - * you first have to unset them. Unsetting metadata can be done by setting the provided {@code - * BlobInfo} objects metadata to {@code null}. See {@link #update(BlobInfo)} for a code example. + * Updates the requested blobs. A batch request is used to perform this call. The original + * properties are merged with the properties in the provided {@code BlobInfo} objects. Unsetting a + * property can be done by setting the property of the provided {@code BlobInfo} objects to {@code + * null}. See {@link #update(BlobInfo)} for a code example. * *

Example of updating information on several blobs using a single batch request. * From 1d959fb64fc861c7db4872a901432cb9b04bb4a0 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 30 Apr 2020 08:35:38 -0700 Subject: [PATCH 4/5] revert fix part 2 --- .../java/com/google/cloud/storage/Storage.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) 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 991638ebb..24ac50a1d 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 @@ -2099,8 +2099,10 @@ Blob create( * userProject {@link BlobTargetOption} option which defines the project id to assign operational * costs. * - *

Example of udating a blob, only if the blob's metageneration matches a value, otherwise a - * {@link StorageException} is thrown. + * Updates the blob properties if the preconditions specified by {@code options} are met. The + * property update works as described in {@link #update(BlobInfo)}. + * + *

Example of updating the content type only if the properties are not updated externally: * * 

{@code
    * BlobId blobId = BlobId.of(bucketName, blobName);
@@ -2122,8 +2124,12 @@ Blob create(
    * }
    * }
* + * @param blobInfo information to update + * @param options preconditions to apply the update * @return the updated blob * @throws StorageException upon failure + * @see https://cloud.google.com/storage/docs/json_api/v1/objects/update */ Blob update(BlobInfo blobInfo, BlobTargetOption... options); @@ -2133,6 +2139,11 @@ Blob create( * {@code blobInfo} will not be updated. To unset a blob property this property in {@code * blobInfo} should be explicitly set to {@code null}. * + *

Bucket or blob's name cannot be changed by this method. If you want to rename the blob or + * move it to a different bucket use the {@link Blob#copyTo} and {@link #delete} operations. + * + *

Property update alters the blob metadata generation and doesn't alter the blob generation. + * *

Example of how to update blob's user provided metadata and unset the content type: * * 

{@code
@@ -2780,7 +2791,7 @@ PostPolicyV4 generateSignedPostPolicyV4(
    * properties are merged with the properties in the provided {@code BlobInfo} objects. Unsetting a
    * property can be done by setting the property of the provided {@code BlobInfo} objects to {@code
    * null}. See {@link #update(BlobInfo)} for a code example.
-   *
+
    * 
Example of updating information on several blobs using a single batch request.
    *
    * 
{@code

From da40d032facd86c50b52f90f0193fd5c036687b1 Mon Sep 17 00:00:00 2001
From: Frank Natividad 
Date: Thu, 30 Apr 2020 08:41:09 -0700
Subject: [PATCH 5/5] revert fix part 3

---
 .../main/java/com/google/cloud/storage/Storage.java  | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

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 24ac50a1d..38794cd4b 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
@@ -2093,15 +2093,13 @@ Blob create(
   Bucket update(BucketInfo bucketInfo, BucketTargetOption... options);
 
   /**
-   * Updates blob information. Original metadata are merged with metadata in the provided {@code
-   * blobInfo}. To replace metadata instead you first have to unset them. Unsetting metadata can be
-   * done by setting the provided {@code blobInfo}'s metadata to {@code null}. Accepts an optional
-   * userProject {@link BlobTargetOption} option which defines the project id to assign operational
-   * costs.
-   *
    * Updates the blob properties if the preconditions specified by {@code options} are met. The
    * property update works as described in {@link #update(BlobInfo)}.
    *
+   * 
{@code options} parameter can contain the preconditions for applying the update. E.g. update
+   * of the blob properties might be required only if the properties have not been updated
+   * externally. {@code StorageException} with the code {@code 412} is thrown if preconditions fail.
+   *
    * 
Example of updating the content type only if the properties are not updated externally:
    *
    * 
{@code
@@ -2791,7 +2789,7 @@ PostPolicyV4 generateSignedPostPolicyV4(
    * properties are merged with the properties in the provided {@code BlobInfo} objects. Unsetting a
    * property can be done by setting the property of the provided {@code BlobInfo} objects to {@code
    * null}. See {@link #update(BlobInfo)} for a code example.
-
+   *
    * 
Example of updating information on several blobs using a single batch request.
    *
    * 
{@code