Update V4 signature to pass conformance tests (#114)

googleapis · Feb 5, 2020 · c86276b · c86276b
1 parent 47b1495
commit c86276b
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 19 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
@@ -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
@@ -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,44 @@ 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', or a Google Cloud Load
+     * Balancer which routes to a bucket you own, e.g. 'my-load-balancer-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 <a href="https://cloud.google.com/storage/docs/request-endpoints#cname">CNAME
+     *     Redirects</a>
+     * @see <a
+     *     href="https://cloud.google.com/load-balancing/docs/https/adding-backend-buckets-to-load-balancers">
+     *     GCLB Redirects</a>
+     */
+    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', or a Google Cloud Load
+     * Balancer which routes to a bucket you own, e.g. 'my-load-balancer-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 <a href="https://cloud.google.com/storage/docs/request-endpoints#cname">CNAME
+     *     Redirects</a>
+     * @see <a
+     *     href="https://cloud.google.com/load-balancing/docs/https/adding-backend-buckets-to-load-balancers">
+     *     GCLB Redirects</a>
+     */
+    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
@@ -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<SignUrlOption.Option, Object> 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<SignUrlOption.Option, Object> 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/StorageImplTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java
@@ -1808,6 +1808,7 @@ public void testSignUrlLeadingSlash()
     String expectedUrl =
         new StringBuilder("https://storage.googleapis.com/")
             .append(BUCKET_NAME1)
+            .append("/")
             .append(expectedResourcePath)
             .append("?GoogleAccessId=")
             .append(ACCOUNT)
@@ -1825,6 +1826,7 @@ public void testSignUrlLeadingSlash()
         .append(42L + 1209600)
         .append("\n/")
         .append(BUCKET_NAME1)
+        .append("/")
         .append(expectedResourcePath);
 
     Signature signer = Signature.getInstance("SHA256withRSA");
@@ -1857,6 +1859,7 @@ public void testSignUrlLeadingSlashWithHostName()
     String expectedUrl =
         new StringBuilder("https://example.com/")
             .append(BUCKET_NAME1)
+            .append("/")
             .append(escapedBlobName)
             .append("?GoogleAccessId=")
             .append(ACCOUNT)
@@ -1874,6 +1877,7 @@ public void testSignUrlLeadingSlashWithHostName()
         .append(42L + 1209600)
         .append("\n/")
         .append(BUCKET_NAME1)
+        .append("/")
         .append(escapedBlobName);
 
     Signature signer = Signature.getInstance("SHA256withRSA");
@@ -2019,6 +2023,7 @@ public void testSignUrlForBlobWithSpecialChars()
       String expectedUrl =
           new StringBuilder("https://storage.googleapis.com/")
               .append(BUCKET_NAME1)
+              .append("/")
               .append(expectedBlobName)
               .append("?GoogleAccessId=")
               .append(ACCOUNT)
@@ -2036,6 +2041,7 @@ public void testSignUrlForBlobWithSpecialChars()
           .append(42L + 1209600)
           .append("\n/")
           .append(BUCKET_NAME1)
+          .append("/")
           .append(expectedBlobName);
 
       Signature signer = Signature.getInstance("SHA256withRSA");
@@ -2075,6 +2081,7 @@ public void testSignUrlForBlobWithSpecialCharsAndHostName()
       String expectedUrl =
           new StringBuilder("https://example.com/")
               .append(BUCKET_NAME1)
+              .append("/")
               .append(expectedBlobName)
               .append("?GoogleAccessId=")
               .append(ACCOUNT)
@@ -2092,6 +2099,7 @@ public void testSignUrlForBlobWithSpecialCharsAndHostName()
           .append(42L + 1209600)
           .append("\n/")
           .append(BUCKET_NAME1)
+          .append("/")
           .append(expectedBlobName);
 
       Signature signer = Signature.getInstance("SHA256withRSA");
@@ -2243,6 +2251,7 @@ public void testSignUrlForBlobWithSlashes()
     String expectedUrl =
         new StringBuilder("https://storage.googleapis.com/")
             .append(BUCKET_NAME1)
+            .append("/")
             .append(escapedBlobName)
             .append("?GoogleAccessId=")
             .append(ACCOUNT)
@@ -2260,6 +2269,7 @@ public void testSignUrlForBlobWithSlashes()
         .append(42L + 1209600)
         .append("\n/")
         .append(BUCKET_NAME1)
+        .append("/")
         .append(escapedBlobName);
 
     Signature signer = Signature.getInstance("SHA256withRSA");
@@ -2293,6 +2303,7 @@ public void testSignUrlForBlobWithSlashesAndHostName()
     String expectedUrl =
         new StringBuilder("https://example.com/")
             .append(BUCKET_NAME1)
+            .append("/")
             .append(escapedBlobName)
             .append("?GoogleAccessId=")
             .append(ACCOUNT)
@@ -2310,6 +2321,7 @@ public void testSignUrlForBlobWithSlashesAndHostName()
         .append(42L + 1209600)
         .append("\n/")
         .append(BUCKET_NAME1)
+        .append("/")
         .append(escapedBlobName);
 
     Signature signer = Signature.getInstance("SHA256withRSA");

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
@@ -16,11 +16,8 @@
 
 package com.google.cloud.storage;
 
-import static org.hamcrest.core.Is.is;
-import static org.hamcrest.core.IsNot.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assume.assumeThat;
 
 import com.google.api.core.ApiClock;
 import com.google.auth.oauth2.ServiceAccountCredentials;
@@ -94,11 +91,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 +102,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 +123,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
@@ -212,7 +212,7 @@
       <dependency>
         <groupId>com.google.cloud</groupId>
         <artifactId>google-cloud-conformance-tests</artifactId>
-        <version>0.0.4</version>
+        <version>0.0.5</version>
         <scope>test</scope>
       </dependency>
     </dependencies>
@@ -225,7 +225,10 @@
           <groupId>org.apache.maven.plugins</groupId>
           <artifactId>maven-dependency-plugin</artifactId>
           <configuration>
-            <ignoredUnusedDeclaredDependencies>org.objenesis:objenesis</ignoredUnusedDeclaredDependencies>
+            <ignoredUnusedDeclaredDependencies>
+              <ignoredUnusedDeclaredDependency>org.hamcrest:hamcrest</ignoredUnusedDeclaredDependency>
+              <ignoredUnusedDeclaredDependency>org.objenesis:objenesis</ignoredUnusedDeclaredDependency>
+            </ignoredUnusedDeclaredDependencies>
           </configuration>
         </plugin>
       </plugins>