From e5743e4781416cdf20f5c7b9d566931808a00e13 Mon Sep 17 00:00:00 2001 From: Konstantin Kirillov Date: Fri, 28 Oct 2016 10:00:01 +0300 Subject: [PATCH 1/2] Fix regression in storage url signing of objects with slashes in names --- .../com/google/cloud/storage/StorageImpl.java | 3 +- .../google/cloud/storage/StorageImplTest.java | 35 +++++++++++++++++-- .../cloud/storage/it/ITStorageTest.java | 2 +- 3 files changed, 36 insertions(+), 4 deletions(-) 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 7cdda52aad0d..49cf62baca71 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 @@ -528,7 +528,8 @@ public URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOptio if (blobInfo.getName().startsWith("/")) { path.setLength(path.length() - 1); } - path.append(UrlEscapers.urlPathSegmentEscaper().escape(blobInfo.getName())); + String escapedName = UrlEscapers.urlFragmentEscaper().escape(blobInfo.getName()); + path.append(escapedName.replace("?", "%3F")); stBuilder.append(path); try { byte[] signatureBytes = authCredentials.sign(stBuilder.toString().getBytes(UTF_8)); 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 index 261bfa31512a..7a64a1021ced 100644 --- 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 @@ -1259,7 +1259,7 @@ public void testSignUrlLeadingSlash() throws NoSuchAlgorithmException, InvalidKe storage = options.toBuilder().authCredentials(authCredentials).build().service(); URL url = storage.signUrl(BlobInfo.newBuilder(BUCKET_NAME1, blobName).build(), 14, TimeUnit.DAYS); - String escapedBlobName = UrlEscapers.urlPathSegmentEscaper().escape(blobName); + String escapedBlobName = UrlEscapers.urlFragmentEscaper().escape(blobName); String stringUrl = url.toString(); String expectedUrl = new StringBuilder("https://storage.googleapis.com/").append(BUCKET_NAME1) .append(escapedBlobName).append("?GoogleAccessId=").append(ACCOUNT).append("&Expires=") @@ -1323,7 +1323,8 @@ public void testSignUrlForBlobWithSpecialChars() throws NoSuchAlgorithmException String blobName = "/a" + specialChar + "b"; URL url = storage.signUrl(BlobInfo.newBuilder(BUCKET_NAME1, blobName).build(), 14, TimeUnit.DAYS); - String escapedBlobName = UrlEscapers.urlPathSegmentEscaper().escape(blobName); + String escapedBlobName = + UrlEscapers.urlFragmentEscaper().escape(blobName).replace("?", "%3F"); String stringUrl = url.toString(); String expectedUrl = new StringBuilder("https://storage.googleapis.com/").append(BUCKET_NAME1) .append(escapedBlobName).append("?GoogleAccessId=").append(ACCOUNT).append("&Expires=") @@ -1343,6 +1344,36 @@ public void testSignUrlForBlobWithSpecialChars() throws NoSuchAlgorithmException } } + @Test + public void testSignUrlForBlobWithSlashes() throws NoSuchAlgorithmException, + InvalidKeyException, SignatureException, UnsupportedEncodingException { + EasyMock.replay(storageRpcMock); + ServiceAccountAuthCredentials authCredentials = + ServiceAccountAuthCredentials.createFor(ACCOUNT, privateKey); + storage = options.toBuilder().authCredentials(authCredentials).build().service(); + + String blobName = "/foo/bar/baz #%20other cool stuff.txt"; + URL url = + storage.signUrl(BlobInfo.newBuilder(BUCKET_NAME1, blobName).build(), 14, TimeUnit.DAYS); + String escapedBlobName = UrlEscapers.urlFragmentEscaper().escape(blobName); + String stringUrl = url.toString(); + String expectedUrl = new StringBuilder("https://storage.googleapis.com/").append(BUCKET_NAME1) + .append(escapedBlobName).append("?GoogleAccessId=").append(ACCOUNT).append("&Expires=") + .append(42L + 1209600).append("&Signature=").toString(); + assertTrue(stringUrl.startsWith(expectedUrl)); + String signature = stringUrl.substring(expectedUrl.length()); + + StringBuilder signedMessageBuilder = new StringBuilder(); + signedMessageBuilder.append(HttpMethod.GET).append("\n\n\n").append(42L + 1209600) + .append("\n/").append(BUCKET_NAME1).append(escapedBlobName); + + Signature signer = Signature.getInstance("SHA256withRSA"); + signer.initVerify(publicKey); + signer.update(signedMessageBuilder.toString().getBytes(UTF_8)); + assertTrue(signer.verify(BaseEncoding.base64().decode( + URLDecoder.decode(signature, UTF_8.name())))); + } + @Test public void testGetAllArray() { BlobId blobId1 = BlobId.of(BUCKET_NAME1, BLOB_NAME1); diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java index 3a9b7c16c93c..e48c50fec2c6 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java @@ -1193,7 +1193,7 @@ public void testWriteChannelExistingBlob() throws IOException { @Test public void testGetSignedUrl() throws IOException { - String blobName = "test-get-signed-url-blob"; + String blobName = "test-get-signed-url-blob/with/slashes/and?special=char*"; BlobInfo blob = BlobInfo.newBuilder(BUCKET, blobName).build(); Blob remoteBlob = storage.create(blob, BLOB_BYTE_CONTENT); assertNotNull(remoteBlob); From 988c1c46c6f724106c1bebf0122c741803834c75 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Fri, 28 Oct 2016 19:02:51 +0200 Subject: [PATCH 2/2] Add special characters to signUrl integration test --- .../test/java/com/google/cloud/storage/it/ITStorageTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java index e48c50fec2c6..6a06b3abcd77 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java @@ -1193,7 +1193,7 @@ public void testWriteChannelExistingBlob() throws IOException { @Test public void testGetSignedUrl() throws IOException { - String blobName = "test-get-signed-url-blob/with/slashes/and?special=char*"; + String blobName = "test-get-signed-url-blob/with/slashes/and?special=!#$&'()*+,:;=?@[]"; BlobInfo blob = BlobInfo.newBuilder(BUCKET, blobName).build(); Blob remoteBlob = storage.create(blob, BLOB_BYTE_CONTENT); assertNotNull(remoteBlob);