-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Storage - Encoding Container Names #47093
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
Conversation
…to storage/encodeContainerNames
…to storage/encodeContainerNames
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances URL encoding throughout the Azure Storage SDK for Java by adding encoding for container names and file system names, which previously were not encoded. The change ensures that resource names containing special characters (like spaces) are properly URL-encoded when constructing resource URLs.
Key Changes
- Added URL encoding for container names and file system names in Blob and Data Lake clients
- Enhanced
BlobUrlPartsto automatically encode/decode container and blob names - Updated batch operations to encode container names
- Removed tests that expected unencoded container names with spaces to fail (since they now succeed with encoding)
Reviewed Changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobContainerClient.java | Added URL encoding for container name in getBlobContainerUrl() |
| sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobContainerAsyncClient.java | Added URL encoding for container name in getBlobContainerUrl() |
| sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobClientBase.java | Added URL encoding for container name in getBlobUrl() |
| sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobAsyncClientBase.java | Added URL encoding for container name in getBlobUrl() |
| sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobUrlParts.java | Enhanced getters/setters to decode on get and encode on set for container and blob names |
| sdk/storage/azure-storage-blob-batch/src/main/java/com/azure/storage/blob/batch/BlobBatch.java | Added URL encoding for container names in batch operations |
| sdk/storage/azure-storage-blob-batch/src/main/java/com/azure/storage/blob/batch/options/BlobBatchSetBlobAccessTierOptions.java | Added encoding in constructor and updated documentation for clarity |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileSystemClient.java | Added URL encoding for file system name in getFileSystemUrl() |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileSystemAsyncClient.java | Added URL encoding for file system name in getFileSystemUrl() |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClient.java | Added URL encoding for file system name in getPathUrl() and rename operations |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathAsyncClient.java | Added URL encoding for file system name in getPathUrl() and rename operations |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/HelperTests.java | Added tests for encoding/decoding behavior and improved assertion order consistency |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ServiceAsyncApiTests.java | Removed test case for unencoded container names (no longer applicable) |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ServiceApiTests.java | Removed test case for unencoded container names (no longer applicable) |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java | Added test to verify container name encoding |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java | Added test to verify container name encoding |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BlobAsyncApiTests.java | Added tests to verify blob and container name encoding |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BlobApiTests.java | Added tests to verify blob and container name encoding |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/AzuriteTests.java | Improved assertion order for consistency |
| sdk/storage/azure-storage-blob-batch/src/test/java/com/azure/storage/blob/batch/BatchApiTests.java | Added comprehensive tests for batch operation encoding |
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/FileSystemAsyncApiTests.java | Added tests for file system name encoding |
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/FileSystemApiTests.java | Added tests for file system name encoding |
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/FileAsyncApiTests.java | Added test for file name encoding |
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/FileApiTest.java | Added test for file name encoding |
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/DirectoryAsyncApiTests.java | Added test for directory name encoding |
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/DirectoryApiTests.java | Added test for directory name encoding |
| sdk/storage/azure-storage-blob/assets.json | Updated test assets tag |
| sdk/storage/azure-storage-blob-batch/assets.json | Updated test assets tag |
| sdk/storage/azure-storage-file-datalake/assets.json | Updated test assets tag |
...ch/src/main/java/com/azure/storage/blob/batch/options/BlobBatchSetBlobAccessTierOptions.java
Outdated
Show resolved
Hide resolved
kyleknap
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for going through all of the SDK to track down what needed to be updated. Just left some small comments and questions.
sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobUrlParts.java
Outdated
Show resolved
Hide resolved
...ch/src/main/java/com/azure/storage/blob/batch/options/BlobBatchSetBlobAccessTierOptions.java
Show resolved
Hide resolved
...ch/src/main/java/com/azure/storage/blob/batch/options/BlobBatchSetBlobAccessTierOptions.java
Outdated
Show resolved
Hide resolved
sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java
Outdated
Show resolved
Hide resolved
...storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/HelperTests.java
Outdated
Show resolved
Hide resolved
...storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/HelperTests.java
Show resolved
Hide resolved
...orage/azure-storage-blob-batch/src/test/java/com/azure/storage/blob/batch/BatchApiTests.java
Outdated
Show resolved
Hide resolved
sdk/storage/azure-storage-blob-batch/src/main/java/com/azure/storage/blob/batch/BlobBatch.java
Outdated
Show resolved
Hide resolved
|
We should also make sure to add changelog entry to note this change as well. |
sdk/storage/azure-storage-blob-batch/src/main/java/com/azure/storage/blob/batch/BlobBatch.java
Outdated
Show resolved
Hide resolved
sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobUrlParts.java
Outdated
Show resolved
Hide resolved
...storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/HelperTests.java
Outdated
Show resolved
Hide resolved
|
/azp run java - storage - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kyleknap
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just two small comments on the docstrings/comments and we should be set.
...storage/azure-storage-blob/src/test/java/com/azure/storage/blob/specialized/HelperTests.java
Outdated
Show resolved
Hide resolved
sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobUrlParts.java
Show resolved
Hide resolved
kyleknap
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 🚢
|
/azp run java - storage - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/check-enforcer override |
Summary
This PR introduces URL encoding for container names and file system paths across Azure Storage SDK components to ensure OneLake compatibility. This change improves consistency in URL generation and prevents issues with URL classes rejecting a resource name due to it containing a special character.
Key Changes
BlobBatch.deleteBlobandBlobBatch.setBlobAccessTierto URL encode container names.BlobBatchSetBlobAccessTierOptionsto return encoded identifiers.BlobContainerClientandBlobContainerAsyncClientto encode container names ingetBlobContainerUrl.BlobClientBaseandBlobAsyncClientBaseto encode container names ingetBlobUrl.getFileSystemUrlandgetPathUrl.Why
Previously, container names with spaces or special characters caused exceptions before a request could get to the service. This update ensures that even if the service may reject an encoded name, the request still reaches the service.