From b16a32624e78c178ff5b52152b5bc8b906265068 Mon Sep 17 00:00:00 2001 From: Praful Makani Date: Sat, 17 Nov 2018 18:37:06 +0530 Subject: [PATCH] update as per feedback --- .../com/google/cloud/storage/Storage.java | 8 +- .../com/google/cloud/storage/StorageImpl.java | 11 ++- .../google/cloud/storage/StorageImplTest.java | 74 +++++++++---------- 3 files changed, 48 insertions(+), 45 deletions(-) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java index 047b84c5aae7..beff9ce4c59a 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java @@ -892,7 +892,7 @@ class SignUrlOption implements Serializable { private final Object value; enum Option { - HTTP_METHOD, CONTENT_TYPE, MD5, EXT_HEADERS, SERVICE_ACCOUNT_CRED,SERVICE_ENDPOINT + HTTP_METHOD, CONTENT_TYPE, MD5, EXT_HEADERS, SERVICE_ACCOUNT_CRED, HOST_NAME } private SignUrlOption(Option option, Object value) { @@ -955,10 +955,10 @@ public static SignUrlOption signWith(ServiceAccountSigner signer) { } /** - * Provides a host name to sign the URL. If not provided than host name will be default + * Provides a host name to sign the URL. If not provided than default host name will be used. */ public static SignUrlOption withHostName(String hostName){ - return new SignUrlOption(Option.SERVICE_ENDPOINT, hostName); + return new SignUrlOption(Option.HOST_NAME, hostName); } } @@ -2114,7 +2114,7 @@ public static Builder newBuilder() { * granularity supported is 1 second, finer granularities will be truncated * @param unit time unit of the {@code duration} parameter * @param options optional URL signing options - * {@code SignUrlOption.withExtHostName()} option is used for external host name of signed url + * {@code SignUrlOption.withHostName()} option to sign url with custom hostname. * @throws IllegalStateException if {@link SignUrlOption#signWith(ServiceAccountSigner)} was not * used and no implementation of {@link ServiceAccountSigner} was provided to * {@link StorageOptions} diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java index d2ab2bbc43fb..6ff06b6b04ea 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java @@ -84,7 +84,10 @@ final class StorageImpl extends BaseService 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 = "/"; - private static final String DEFAULT_STORAGE_HOST = "https://storage.googleapis.com"; + /** + * SignedUrls uses GCS XML API endpoint. + */ + private static final String STORAGE_XML_HOST_NAME = "https://storage.googleapis.com"; private static final Function, Boolean> DELETE_FUNCTION = new Function, Boolean>() { @@ -537,11 +540,11 @@ public URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOptio byte[] signatureBytes = credentials.sign(signatureInfo.constructUnsignedPayload().getBytes(UTF_8)); StringBuilder stBuilder = new StringBuilder(); - if (optionMap.get(SignUrlOption.Option.SERVICE_ENDPOINT) == null){ - stBuilder.append(DEFAULT_STORAGE_HOST).append(path); + if (optionMap.get(SignUrlOption.Option.HOST_NAME) == null) { + stBuilder.append(STORAGE_XML_HOST_NAME).append(path); } else { - stBuilder.append(optionMap.get(SignUrlOption.Option.SERVICE_ENDPOINT)).append(path); + stBuilder.append(optionMap.get(SignUrlOption.Option.HOST_NAME)).append(path); } String signature = diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java index 426cc6e68495..cea262cfce35 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java @@ -26,6 +26,37 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.api.core.ApiClock; +import com.google.api.gax.paging.Page; +import com.google.api.services.storage.model.Policy.Bindings; +import com.google.api.services.storage.model.StorageObject; +import com.google.api.services.storage.model.TestIamPermissionsResponse; +import com.google.auth.oauth2.ServiceAccountCredentials; +import com.google.cloud.Identity; +import com.google.cloud.Policy; +import com.google.cloud.ReadChannel; +import com.google.cloud.ServiceOptions; +import com.google.cloud.Tuple; +import com.google.cloud.WriteChannel; +import com.google.cloud.storage.Acl.Project; +import com.google.cloud.storage.Acl.Project.ProjectRole; +import com.google.cloud.storage.Acl.Role; +import com.google.cloud.storage.Acl.User; +import com.google.cloud.storage.Storage.BlobSourceOption; +import com.google.cloud.storage.Storage.BlobTargetOption; +import com.google.cloud.storage.Storage.BlobWriteOption; +import com.google.cloud.storage.Storage.BucketSourceOption; +import com.google.cloud.storage.Storage.CopyRequest; +import com.google.cloud.storage.spi.StorageRpcFactory; +import com.google.cloud.storage.spi.v1.RpcBatch; +import com.google.cloud.storage.spi.v1.StorageRpc; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; +import com.google.common.io.BaseEncoding; +import com.google.common.net.UrlEscapers; + import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.UnsupportedEncodingException; @@ -60,37 +91,6 @@ import org.junit.Test; import org.junit.rules.ExpectedException; -import com.google.api.client.googleapis.json.GoogleJsonError; -import com.google.api.core.ApiClock; -import com.google.api.gax.paging.Page; -import com.google.api.services.storage.model.Policy.Bindings; -import com.google.api.services.storage.model.StorageObject; -import com.google.api.services.storage.model.TestIamPermissionsResponse; -import com.google.auth.oauth2.ServiceAccountCredentials; -import com.google.cloud.Identity; -import com.google.cloud.Policy; -import com.google.cloud.ReadChannel; -import com.google.cloud.ServiceOptions; -import com.google.cloud.Tuple; -import com.google.cloud.WriteChannel; -import com.google.cloud.storage.Acl.Project; -import com.google.cloud.storage.Acl.Project.ProjectRole; -import com.google.cloud.storage.Acl.Role; -import com.google.cloud.storage.Acl.User; -import com.google.cloud.storage.Storage.BlobSourceOption; -import com.google.cloud.storage.Storage.BlobTargetOption; -import com.google.cloud.storage.Storage.BlobWriteOption; -import com.google.cloud.storage.Storage.BucketSourceOption; -import com.google.cloud.storage.Storage.CopyRequest; -import com.google.cloud.storage.spi.StorageRpcFactory; -import com.google.cloud.storage.spi.v1.RpcBatch; -import com.google.cloud.storage.spi.v1.StorageRpc; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Iterables; -import com.google.common.io.BaseEncoding; -import com.google.common.net.UrlEscapers; - public class StorageImplTest { private static final String BUCKET_NAME1 = "b1"; @@ -1638,7 +1638,7 @@ public void testSignUrl() } @Test - public void testSignUrlWithCustomUrl() + public void testSignUrlWithHostName() throws NoSuchAlgorithmException, InvalidKeyException, SignatureException, UnsupportedEncodingException { EasyMock.replay(storageRpcMock); @@ -1721,7 +1721,7 @@ public void testSignUrlLeadingSlash() } @Test - public void testSignUrlLeadingSlashWithCustomUrl() + public void testSignUrlLeadingSlashWithHostName() throws NoSuchAlgorithmException, InvalidKeyException, SignatureException, UnsupportedEncodingException { String blobName = "/b1"; @@ -1815,7 +1815,7 @@ public void testSignUrlWithOptions() } @Test - public void testSignUrlWithOptionsAndCustomUrl() + public void testSignUrlWithOptionsAndHostName() throws NoSuchAlgorithmException, InvalidKeyException, SignatureException, UnsupportedEncodingException { EasyMock.replay(storageRpcMock); @@ -1920,7 +1920,7 @@ public void testSignUrlForBlobWithSpecialChars() } @Test - public void testSignUrlForBlobWithSpecialCharsAndCustomUrl() + public void testSignUrlForBlobWithSpecialCharsAndHostName() throws NoSuchAlgorithmException, InvalidKeyException, SignatureException, UnsupportedEncodingException { // List of chars under test were taken from @@ -2029,7 +2029,7 @@ public void testSignUrlWithExtHeaders() } @Test - public void testSignUrlWithExtHeadersAndCustomUrl() + public void testSignUrlWithExtHeadersAndHostName() throws NoSuchAlgorithmException, InvalidKeyException, SignatureException, UnsupportedEncodingException { EasyMock.replay(storageRpcMock); @@ -2130,7 +2130,7 @@ public void testSignUrlForBlobWithSlashes() } @Test - public void testSignUrlForBlobWithSlashesAndCustomUrl() + public void testSignUrlForBlobWithSlashesAndHostName() throws NoSuchAlgorithmException, InvalidKeyException, SignatureException, UnsupportedEncodingException { EasyMock.replay(storageRpcMock);