From a76845a6bb4e630ca05e928f5c9c74b025a4e89f Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Mon, 3 Feb 2020 16:24:08 -0800 Subject: [PATCH] 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 f26157d9d0..03630c61a4 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 77c88e97c6..6471220d48 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 63c1985570..d76aa0d298 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 8e1ad31045..582c0d0ebc 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 03489e8c53..46adf3b7f9 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