Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added UTF-8 encoding for blob names in methods that are used to build… #5943

Merged
merged 14 commits into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.azure.storage.blob.models.AccessTier;
import com.azure.storage.blob.models.BlobRequestConditions;
import com.azure.storage.blob.models.DeleteSnapshotsOptionType;
import com.azure.storage.common.Utility;
import com.azure.storage.common.policy.StorageSharedKeyCredentialPolicy;
import reactor.core.Disposable;
import reactor.core.Exceptions;
Expand Down Expand Up @@ -136,7 +137,8 @@ public final class BlobBatch {
* @throws UnsupportedOperationException If this batch has already added an operation of another type.
*/
public Response<Void> deleteBlob(String containerName, String blobName) {
return deleteBlobHelper(String.format(PATH_TEMPLATE, containerName, blobName), null, null);
return deleteBlobHelper(String.format(PATH_TEMPLATE, containerName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just call into the next overload so we don't have to worry about missing formatting/encoding logic in one of them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, let's make it call the next overload.

Utility.urlEncode(Utility.urlDecode(blobName))), null, null);
}

/**
Expand All @@ -156,8 +158,8 @@ public Response<Void> deleteBlob(String containerName, String blobName) {
*/
public Response<Void> deleteBlob(String containerName, String blobName,
DeleteSnapshotsOptionType deleteOptions, BlobRequestConditions blobRequestConditions) {
return deleteBlobHelper(String.format(PATH_TEMPLATE, containerName, blobName),
deleteOptions, blobRequestConditions);
return deleteBlobHelper(String.format(PATH_TEMPLATE, containerName,
Utility.urlEncode(Utility.urlDecode(blobName))), deleteOptions, blobRequestConditions);
}

/**
Expand All @@ -167,7 +169,7 @@ public Response<Void> deleteBlob(String containerName, String blobName,
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatch.deleteBlob#String}
*
* @param blobUrl URL of the blob.
* @param blobUrl URL of the blob. Blob name must be encoded to UTF-8.
* @return a {@link Response} that will be used to associate this operation to the response when the batch is
* submitted.
* @throws UnsupportedOperationException If this batch has already added an operation of another type.
Expand All @@ -183,7 +185,7 @@ public Response<Void> deleteBlob(String blobUrl) {
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatch.deleteBlob#String-DeleteSnapshotsOptionType-BlobRequestConditions}
*
* @param blobUrl URL of the blob.
* @param blobUrl URL of the blob. Blob name must be encoded to UTF-8.
* @param deleteOptions Delete options for the blob and its snapshots.
* @param blobRequestConditions Additional access conditions that must be met to allow this operation.
* @return a {@link Response} that will be used to associate this operation to the response when the batch is
Expand Down Expand Up @@ -217,7 +219,8 @@ private Response<Void> deleteBlobHelper(String urlPath, DeleteSnapshotsOptionTyp
* @throws UnsupportedOperationException If this batch has already added an operation of another type.
*/
public Response<Void> setBlobAccessTier(String containerName, String blobName, AccessTier accessTier) {
return setBlobAccessTierHelper(String.format(PATH_TEMPLATE, containerName, blobName), accessTier, null);
return setBlobAccessTierHelper(String.format(PATH_TEMPLATE, containerName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about calling the next overload.

Utility.urlEncode(Utility.urlDecode(blobName))), accessTier, null);
}

/**
Expand All @@ -237,7 +240,8 @@ public Response<Void> setBlobAccessTier(String containerName, String blobName, A
*/
public Response<Void> setBlobAccessTier(String containerName, String blobName, AccessTier accessTier,
String leaseId) {
return setBlobAccessTierHelper(String.format(PATH_TEMPLATE, containerName, blobName), accessTier, leaseId);
return setBlobAccessTierHelper(String.format(PATH_TEMPLATE, containerName,
Utility.urlEncode(Utility.urlDecode(blobName))), accessTier, leaseId);
}

/**
Expand All @@ -247,7 +251,7 @@ public Response<Void> setBlobAccessTier(String containerName, String blobName, A
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatch.setBlobAccessTier#String-AccessTier}
*
* @param blobUrl URL of the blob.
* @param blobUrl URL of the blob. Blob name must be encoded to UTF-8.
* @param accessTier The tier to set on the blob.
* @return a {@link Response} that will be used to associate this operation to the response when the batch is
* submitted.
Expand All @@ -264,7 +268,7 @@ public Response<Void> setBlobAccessTier(String blobUrl, AccessTier accessTier) {
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatch.setBlobAccessTier#String-AccessTier-String}
*
* @param blobUrl URL of the blob.
* @param blobUrl URL of the blob. Blob name must be encoded to UTF-8.
* @param accessTier The tier to set on the blob.
* @param leaseId The lease ID the active lease on the blob must match.
* @return a {@link Response} that will be used to associate this operation to the response when the batch is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ Mono<Response<Void>> submitBatchWithResponse(BlobBatch batch, boolean throwOnAny
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatchAsyncClient.deleteBlobs#List-DeleteSnapshotsOptionType}
*
* @param blobUrls Urls of the blobs to delete.
* @param blobUrls Urls of the blobs to delete. Blob names must be encoded to UTF-8.
* @param deleteOptions The deletion option for all blobs.
* @return The status of each delete operation.
* @throws BlobStorageException If the batch request is malformed.
Expand Down Expand Up @@ -162,7 +162,7 @@ private Mono<PagedResponse<Response<Void>>> submitDeleteBlobsBatch(List<String>
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatchAsyncClient.setBlobsAccessTier#List-AccessTier}
*
* @param blobUrls Urls of the blobs to set their access tier.
* @param blobUrls Urls of the blobs to set their access tier. Blob names must be encoded to UTF-8.
* @param accessTier {@link AccessTier} to set on each blob.
* @return The status of each set tier operation.
* @throws BlobStorageException If the batch request is malformed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public Response<Void> submitBatchWithResponse(BlobBatch batch, boolean throwOnAn
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatchClient.deleteBlobs#List-DeleteSnapshotsOptionType}
*
* @param blobUrls Urls of the blobs to delete.
* @param blobUrls Urls of the blobs to delete. Blob names must be encoded to UTF-8.
* @param deleteOptions The deletion option for all blobs.
* @return The status of each delete operation.
* @throws BlobStorageException If the batch request is malformed.
Expand All @@ -113,7 +113,7 @@ public PagedIterable<Response<Void>> deleteBlobs(List<String> blobUrls, DeleteSn
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatchClient.deleteBlobs#List-DeleteSnapshotsOptionType-Duration-Context}
*
* @param blobUrls Urls of the blobs to delete.
* @param blobUrls Urls of the blobs to delete. Blob names must be encoded to UTF-8.
* @param deleteOptions The deletion option for all blobs.
* @param timeout An optional timeout value beyond which a {@link RuntimeException} will be raised.
* @param context Additional context that is passed through the Http pipeline during the service call.
Expand All @@ -135,7 +135,7 @@ public PagedIterable<Response<Void>> deleteBlobs(List<String> blobUrls, DeleteSn
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatchClient.setBlobsAccessTier#List-AccessTier}
*
* @param blobUrls Urls of the blobs to set their access tier.
* @param blobUrls Urls of the blobs to set their access tier. Blob names must be encoded to UTF-8.
* @param accessTier {@link AccessTier} to set on each blob.
* @return The status of each set tier operation.
* @throws BlobStorageException If the batch request is malformed.
Expand All @@ -153,7 +153,7 @@ public PagedIterable<Response<Void>> setBlobsAccessTier(List<String> blobUrls, A
*
* {@codesnippet com.azure.storage.blob.batch.BlobBatchClient.setBlobsAccessTier#List-AccessTier-Duration-Context}
*
* @param blobUrls Urls of the blobs to set their access tier.
* @param blobUrls Urls of the blobs to set their access tier. Blob names must be encoded to UTF-8.
* @param accessTier {@link AccessTier} to set on each blob.
* @param timeout An optional timeout value beyond which a {@link RuntimeException} will be raised.
* @param context Additional context that is passed through the Http pipeline during the service call.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class APISpec extends Specification {
accountName = Configuration.getGlobalConfiguration().get(accountType + "ACCOUNT_NAME")
accountKey = Configuration.getGlobalConfiguration().get(accountType + "ACCOUNT_KEY")
} else {
accountName = "storageaccount"
accountName = "azstoragesdkaccount"
accountKey = "astorageaccountkey"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.azure.storage.blob.BlobContainerAsyncClient;
import com.azure.storage.blob.BlobServiceVersion;
import com.azure.storage.blob.BlobUrlParts;
import com.azure.storage.common.Utility;
import com.azure.storage.common.implementation.Constants;
import com.azure.storage.common.StorageSharedKeyCredential;
import com.azure.storage.common.implementation.connectionstring.StorageAuthenticationSettings;
Expand Down Expand Up @@ -336,7 +337,7 @@ public EncryptedBlobClientBuilder endpoint(String endpoint) {
this.accountName = parts.getAccountName();
this.endpoint = parts.getScheme() + "://" + parts.getHost();
this.containerName = parts.getBlobContainerName();
this.blobName = parts.getBlobName();
this.blobName = Utility.urlEncode(parts.getBlobName());
this.snapshot = parts.getSnapshot();

String sasToken = parts.getSasQueryParameters().encode();
Expand Down Expand Up @@ -370,7 +371,8 @@ public EncryptedBlobClientBuilder containerName(String containerName) {
* @throws NullPointerException If {@code blobName} is {@code null}
*/
public EncryptedBlobClientBuilder blobName(String blobName) {
this.blobName = Objects.requireNonNull(blobName, "'blobName' cannot be null.");
this.blobName = Utility.urlEncode(Utility.urlDecode(Objects.requireNonNull(blobName,
"'blobName' cannot be null.")));
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class APISpec extends Specification {
accountName = Configuration.getGlobalConfiguration().get(accountType + "ACCOUNT_NAME")
accountKey = Configuration.getGlobalConfiguration().get(accountType + "ACCOUNT_KEY")
} else {
accountName = "storageaccount"
accountName = "azstoragesdkaccount"
accountKey = "astorageaccountkey"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.azure.storage.blob.implementation.util.BuilderHelper;
import com.azure.storage.blob.models.CpkInfo;
import com.azure.storage.blob.models.CustomerProvidedKey;
import com.azure.storage.common.Utility;
import com.azure.storage.common.StorageSharedKeyCredential;
import com.azure.storage.common.implementation.connectionstring.StorageAuthenticationSettings;
import com.azure.storage.common.implementation.connectionstring.StorageConnectionString;
Expand Down Expand Up @@ -263,7 +264,7 @@ public BlobClientBuilder endpoint(String endpoint) {
this.accountName = parts.getAccountName();
this.endpoint = parts.getScheme() + "://" + parts.getHost();
this.containerName = parts.getBlobContainerName();
this.blobName = parts.getBlobName();
this.blobName = Utility.urlEncode(parts.getBlobName());
this.snapshot = parts.getSnapshot();

String sasToken = parts.getSasQueryParameters().encode();
Expand Down Expand Up @@ -297,7 +298,8 @@ public BlobClientBuilder containerName(String containerName) {
* @throws NullPointerException If {@code blobName} is {@code null}
*/
public BlobClientBuilder blobName(String blobName) {
this.blobName = Objects.requireNonNull(blobName, "'blobName' cannot be null.");
this.blobName = Utility.urlEncode(Utility.urlDecode(Objects.requireNonNull(blobName,
"'blobName' cannot be null.")));
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.azure.storage.blob.models.ListBlobsOptions;
import com.azure.storage.blob.models.PublicAccessType;
import com.azure.storage.blob.models.StorageAccountInfo;
import com.azure.storage.common.Utility;
import com.azure.storage.common.implementation.StorageImplUtils;
import reactor.core.publisher.Mono;

Expand Down Expand Up @@ -139,9 +140,9 @@ public BlobAsyncClient getBlobAsyncClient(String blobName) {
* @return A new {@link BlobAsyncClient} object which references the blob with the specified name in this container.
*/
public BlobAsyncClient getBlobAsyncClient(String blobName, String snapshot) {
return new BlobAsyncClient(getHttpPipeline(),
StorageImplUtils.appendToUrlPath(getBlobContainerUrl(), blobName).toString(), getServiceVersion(),
getAccountName(), getBlobContainerName(), blobName, snapshot, getCustomerProvidedKey());
return new BlobAsyncClient(getHttpPipeline(), StorageImplUtils.appendToUrlPath(getBlobContainerUrl(),
Utility.urlEncode(Utility.urlDecode(blobName))).toString(), getServiceVersion(), getAccountName(),
getBlobContainerName(), blobName, snapshot, getCustomerProvidedKey());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,12 @@ public BlobUrlParts setContainerName(String containerName) {
}

/**
* Gets the blob name that will be used as part of the URL path.
* Decodes and gets the blob name that will be used as part of the URL path.
*
* @return the blob name.
* @return the decoded blob name.
*/
public String getBlobName() {
return blobName;
return (blobName == null) ? null : Utility.urlDecode(blobName);
}

/**
Expand All @@ -142,7 +142,7 @@ public String getBlobName() {
* @return the updated BlobUrlParts object.
*/
public BlobUrlParts setBlobName(String blobName) {
this.blobName = blobName;
this.blobName = Utility.urlEncode(Utility.urlDecode(blobName));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overload to skip encoding?

Copy link
Member Author

@vcolin7 vcolin7 Oct 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See answer about overloads and double encoding above.

return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.azure.storage.blob.models.RehydratePriority;
import com.azure.storage.blob.models.ReliableDownloadOptions;
import com.azure.storage.blob.models.StorageAccountInfo;
import com.azure.storage.common.Utility;
import com.azure.storage.common.implementation.Constants;
import com.azure.storage.common.implementation.StorageImplUtils;
import reactor.core.publisher.Flux;
Expand Down Expand Up @@ -113,7 +114,7 @@ protected BlobAsyncClientBase(HttpPipeline pipeline, String url, BlobServiceVers

this.accountName = accountName;
this.containerName = containerName;
this.blobName = blobName;
this.blobName = Utility.urlEncode(Utility.urlDecode(blobName));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we encode in the builder, so encoding in the constructor again would be double encoding?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See answer about overloads and double encoding above.

this.snapshot = snapshot;
this.customerProvidedKey = customerProvidedKey;
}
Expand Down Expand Up @@ -160,16 +161,16 @@ public final String getContainerName() {
}

/**
* Get the blob name.
* Decodes and gets the blob name.
*
* <p><strong>Code Samples</strong></p>
*
* {@codesnippet com.azure.storage.blob.specialized.BlobAsyncClientBase.getBlobName}
*
* @return The name of the blob.
* @return The decoded name of the blob.
*/
public final String getBlobName() {
return blobName;
return (blobName == null) ? null : Utility.urlDecode(blobName);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,13 @@ public final String getContainerName() {
}

/**
* Get the blob name.
* Decodes and gets the blob name.
*
* <p><strong>Code Samples</strong></p>
*
* {@codesnippet com.azure.storage.blob.specialized.BlobClientBase.getBlobName}
*
* @return The name of the blob.
* @return The decoded name of the blob.
*/
public final String getBlobName() {
return client.getBlobName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.azure.storage.blob.BlobSasPermission;
import com.azure.storage.blob.BlobServiceVersion;
import com.azure.storage.blob.models.UserDelegationKey;
import com.azure.storage.common.Utility;
import com.azure.storage.common.implementation.Constants;
import com.azure.storage.common.implementation.StorageImplUtils;
import com.azure.storage.common.StorageSharedKeyCredential;
Expand Down Expand Up @@ -280,10 +281,10 @@ public BlobServiceSasSignatureValues setContainerName(String containerName) {
}

/**
* Gets the name of the blob the SAS user may access. {@code null} or an empty string is returned when a
* Decodes and gets the name of the blob the SAS user may access. {@code null} or an empty string is returned when a
* creating a container SAS.
*
* @return The name of the blob the SAS user may access. {@code null} or an empty string is returned when a
* @return The decoded name of the blob the SAS user may access. {@code null} or an empty string is returned when a
* creating a container SAS.
*/
public String getBlobName() {
Expand All @@ -297,7 +298,7 @@ public String getBlobName() {
* @return The updated BlobServiceSASSignatureValues object.
*/
public BlobServiceSasSignatureValues setBlobName(String blobName) {
this.blobName = blobName;
this.blobName = (blobName == null) ? null : Utility.urlDecode(blobName);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.azure.storage.blob.models.CpkInfo;
import com.azure.storage.blob.models.CustomerProvidedKey;
import com.azure.storage.blob.models.PageRange;
import com.azure.storage.common.Utility;
import com.azure.storage.common.StorageSharedKeyCredential;
import com.azure.storage.common.implementation.connectionstring.StorageAuthenticationSettings;
import com.azure.storage.common.implementation.connectionstring.StorageConnectionString;
Expand Down Expand Up @@ -291,7 +292,7 @@ public SpecializedBlobClientBuilder endpoint(String endpoint) {
this.accountName = parts.getAccountName();
this.endpoint = parts.getScheme() + "://" + parts.getHost();
this.containerName = parts.getBlobContainerName();
this.blobName = parts.getBlobName();
this.blobName = Utility.urlEncode(parts.getBlobName());
this.snapshot = parts.getSnapshot();

String sasToken = parts.getSasQueryParameters().encode();
Expand Down Expand Up @@ -435,7 +436,8 @@ public SpecializedBlobClientBuilder containerName(String containerName) {
* @throws NullPointerException If {@code blobName} is {@code null}
*/
public SpecializedBlobClientBuilder blobName(String blobName) {
this.blobName = Objects.requireNonNull(blobName, "'blobName' cannot be null.");
this.blobName = Utility.urlEncode(Utility.urlDecode(Objects.requireNonNull(blobName,
"'blobName' cannot be null.")));
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class APISpec extends Specification {
accountName = Configuration.getGlobalConfiguration().get(accountType + "ACCOUNT_NAME")
accountKey = Configuration.getGlobalConfiguration().get(accountType + "ACCOUNT_KEY")
} else {
accountName = "storageaccount"
accountName = "azstoragesdkaccount"
accountKey = "astorageaccountkey"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1582,4 +1582,21 @@ class BlobAPITest extends APISpec {
expect:
blobName == bc.getBlobName()
}

def "Get Blob Name and Build Client"() {
when:
BlobClient client = cc.getBlobClient(originalBlobName)
BlobClientBase baseClient = cc.getBlobClient(client.getBlobName()).getBlockBlobClient()

then:
baseClient.getBlobName() == finalBlobName

where:
originalBlobName | finalBlobName
"blob" | "blob"
"path/to]a blob" | "path/to]a blob"
"path%2Fto%5Da%20blob" | "path/to]a blob"
"斑點" | "斑點"
"%E6%96%91%E9%BB%9E" | "斑點"
}
}
Loading