fix: update Signed URL default scheme to resolve from storage options host (#2880)

Fixes #2870

Author: BenWhitehead

google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java

@@ -72,6 +72,7 @@
 import java.io.UnsupportedEncodingException;
 import java.net.MalformedURLException;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLEncoder;
 import java.nio.ByteBuffer;
@@ -104,15 +105,9 @@ final class StorageImpl extends BaseService<StorageOptions> implements Storage,
   private static final String EMPTY_BYTE_ARRAY_MD5 = "1B2M2Y8AsgTpgAmY7PhCfg==";
   private static final String EMPTY_BYTE_ARRAY_CRC32C = "AAAAAA==";
   private static final String PATH_DELIMITER = "/";
-  /** Signed URLs are only supported through the GCS XML API endpoint. */
-  private static final String STORAGE_XML_URI_SCHEME = "https";
 
-  // TODO: in the future, this can be replaced by getOptions().getHost()
-  private final String STORAGE_XML_URI_HOST_NAME =
-      getOptions()
-          .getResolvedApiaryHost("storage")
-          .replaceFirst("http(s)?://", "")
-          .replace("/", "");
+  private final String STORAGE_XML_URI_SCHEME;
+  private final String STORAGE_XML_URI_HOST_NAME;
 
   private static final int DEFAULT_BUFFER_SIZE = 15 * 1024 * 1024;
   private static final int MIN_BUFFER_SIZE = 256 * 1024;
@@ -128,6 +123,14 @@ final class StorageImpl extends BaseService<StorageOptions> implements Storage,
     this.retryAlgorithmManager = options.getRetryAlgorithmManager();
     this.storageRpc = options.getStorageRpcV1();
     this.writerFactory = writerFactory;
+    try {
+      String resolvedApiaryHost = options.getResolvedApiaryHost("storage");
+      URI uri = new URI(resolvedApiaryHost);
+      STORAGE_XML_URI_HOST_NAME = uri.getHost();
+      STORAGE_XML_URI_SCHEME = firstNonNull(uri.getScheme(), "https");
+    } catch (URISyntaxException e) {
+      throw StorageException.coalesce(e);
+    }
   }
 
   @Override

google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITSignedUrlTest.java

@@ -16,6 +16,8 @@
 
 package com.google.cloud.storage.it;
 
+import static com.google.cloud.storage.TestUtils.assertAll;
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -33,6 +35,7 @@
 import com.google.cloud.storage.PostPolicyV4;
 import com.google.cloud.storage.PostPolicyV4.PostFieldsV4;
 import com.google.cloud.storage.Storage;
+import com.google.cloud.storage.Storage.SignUrlOption;
 import com.google.cloud.storage.StorageOptions;
 import com.google.cloud.storage.TransportCompatibility.Transport;
 import com.google.cloud.storage.it.runner.StorageITRunner;
@@ -45,6 +48,7 @@
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URI;
 import java.net.URL;
 import java.net.URLConnection;
 import java.nio.ByteBuffer;
@@ -261,4 +265,24 @@ public void testUploadUsingSignedURL() throws Exception {
       assertTrue(storage.delete(bucketName, blobName));
     }
   }
+
+  @Test
+  public void generatingSignedURLForHttpProducesTheCorrectScheme() throws Exception {
+    StorageOptions options =
+        storage.getOptions().toBuilder().setHost("http://[::1]").setProjectId("no-project").build();
+    try (Storage s = options.getService()) {
+      BlobInfo info = BlobInfo.newBuilder("no-bucket", "no-object").build();
+      URL urlV2 = s.signUrl(info, 5, TimeUnit.MINUTES, SignUrlOption.withV2Signature());
+      URL urlV4 = s.signUrl(info, 5, TimeUnit.MINUTES, SignUrlOption.withV4Signature());
+      URI uriV2 = urlV2.toURI();
+      URI uriV4 = urlV4.toURI();
+      assertAll(
+          () -> assertThat(uriV2.getScheme()).isEqualTo("http"),
+          () -> assertThat(uriV2.getHost()).isEqualTo("[::1]"),
+          () -> assertThat(uriV2.getPath()).contains("no-bucket/no-object"),
+          () -> assertThat(uriV4.getScheme()).isEqualTo("http"),
+          () -> assertThat(uriV4.getHost()).isEqualTo("[::1]"),
+          () -> assertThat(uriV4.getPath()).contains("no-bucket/no-object"));
+    }
+  }
 }