Skip to content

Commit 0c65bcf

Browse files
committed
Regression in storage url signing of objects with slashes in names fixed (#1346)
1 parent 66b83a0 commit 0c65bcf

File tree

2 files changed

+34
-3
lines changed

2 files changed

+34
-3
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ public URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOptio
525525
if (blobInfo.name().startsWith("/")) {
526526
path.setLength(path.length() - 1);
527527
}
528-
path.append(UrlEscapers.urlPathSegmentEscaper().escape(blobInfo.name()));
528+
path.append(UrlEscapers.urlFragmentEscaper().escape(blobInfo.name()));
529529
stBuilder.append(path);
530530
try {
531531
byte[] signatureBytes = authCredentials.sign(stBuilder.toString().getBytes(UTF_8));

google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,7 +1253,7 @@ public void testSignUrlLeadingSlash() throws NoSuchAlgorithmException, InvalidKe
12531253
ServiceAccountAuthCredentials.createFor(ACCOUNT, privateKey);
12541254
storage = options.toBuilder().authCredentials(authCredentials).build().service();
12551255
URL url = storage.signUrl(BlobInfo.builder(BUCKET_NAME1, blobName).build(), 14, TimeUnit.DAYS);
1256-
String escapedBlobName = UrlEscapers.urlPathSegmentEscaper().escape(blobName);
1256+
String escapedBlobName = UrlEscapers.urlFragmentEscaper().escape(blobName);
12571257
String stringUrl = url.toString();
12581258
String expectedUrl = new StringBuilder("https://storage.googleapis.com/").append(BUCKET_NAME1)
12591259
.append(escapedBlobName).append("?GoogleAccessId=").append(ACCOUNT).append("&Expires=")
@@ -1317,7 +1317,7 @@ public void testSignUrlForBlobWithSpecialChars() throws NoSuchAlgorithmException
13171317
String blobName = "/a" + specialChar + "b";
13181318
URL url =
13191319
storage.signUrl(BlobInfo.builder(BUCKET_NAME1, blobName).build(), 14, TimeUnit.DAYS);
1320-
String escapedBlobName = UrlEscapers.urlPathSegmentEscaper().escape(blobName);
1320+
String escapedBlobName = UrlEscapers.urlFragmentEscaper().escape(blobName);
13211321
String stringUrl = url.toString();
13221322
String expectedUrl = new StringBuilder("https://storage.googleapis.com/").append(BUCKET_NAME1)
13231323
.append(escapedBlobName).append("?GoogleAccessId=").append(ACCOUNT).append("&Expires=")
@@ -1337,6 +1337,37 @@ public void testSignUrlForBlobWithSpecialChars() throws NoSuchAlgorithmException
13371337
}
13381338
}
13391339

1340+
@Test
1341+
public void testSignUrlForBlobWithSlashes() throws NoSuchAlgorithmException,
1342+
InvalidKeyException, SignatureException, UnsupportedEncodingException {
1343+
EasyMock.replay(storageRpcMock);
1344+
ServiceAccountAuthCredentials authCredentials =
1345+
ServiceAccountAuthCredentials.createFor(ACCOUNT, privateKey);
1346+
storage = options.toBuilder().authCredentials(authCredentials).build().service();
1347+
1348+
String blobName = "/foo/bar/baz #%20other cool stuff.txt";
1349+
URL url =
1350+
storage.signUrl(BlobInfo.builder(BUCKET_NAME1, blobName).build(), 14, TimeUnit.DAYS);
1351+
1352+
String escapedBlobName = UrlEscapers.urlFragmentEscaper().escape(blobName);
1353+
String stringUrl = url.toString();
1354+
String expectedUrl = new StringBuilder("https://storage.googleapis.com/").append(BUCKET_NAME1)
1355+
.append(escapedBlobName).append("?GoogleAccessId=").append(ACCOUNT).append("&Expires=")
1356+
.append(42L + 1209600).append("&Signature=").toString();
1357+
assertTrue(stringUrl.startsWith(expectedUrl));
1358+
String signature = stringUrl.substring(expectedUrl.length());
1359+
1360+
StringBuilder signedMessageBuilder = new StringBuilder();
1361+
signedMessageBuilder.append(HttpMethod.GET).append("\n\n\n").append(42L + 1209600)
1362+
.append("\n/").append(BUCKET_NAME1).append(escapedBlobName);
1363+
1364+
Signature signer = Signature.getInstance("SHA256withRSA");
1365+
signer.initVerify(publicKey);
1366+
signer.update(signedMessageBuilder.toString().getBytes(UTF_8));
1367+
assertTrue(signer.verify(BaseEncoding.base64().decode(
1368+
URLDecoder.decode(signature, UTF_8.name()))));
1369+
}
1370+
13401371
@Test
13411372
public void testGetAllArray() {
13421373
BlobId blobId1 = BlobId.of(BUCKET_NAME1, BLOB_NAME1);

0 commit comments

Comments
 (0)