fix(storage-blob): normalize backslash to forward slash in SAS canonical resource - #48849
fix(storage-blob): normalize backslash to forward slash in SAS canonical resource#48849Wu Shuwen (dajiaohuang) wants to merge 2 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Thank you for your contribution Wu Shuwen (@dajiaohuang)! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
🟡 Changes recommended
An automated regression test for backslash normalization is still needed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes SAS authentication for blob names containing backslashes by matching Azure Storage canonicalization behavior.
Changes:
- Normalizes backslashes to forward slashes before signing blob resource paths.
File summaries
| File | Description |
|---|---|
sdk/storage/azure-storage-blob/azure/storage/blob/_shared_access_signature.py |
Normalizes the SAS canonical resource path before signature generation. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| # Normalize backslashes to forward slashes to match Azure Storage service behavior | ||
| # Go and .NET SDKs already perform this normalization | ||
| path = path.replace("\\", "/") |
|
Wu Shuwen (@dajiaohuang) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
1 similar comment
|
Wu Shuwen (@dajiaohuang) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
|
Added commit d42de0c with a focused regression test for blob_name="dir\file". It captures the string-to-sign via sts_hook and asserts the canonical resource uses /blob/account/container/dir/file. git diff --check passes. The package test runner is currently blocked in this environment because azure.core and devtools_testutils are not installed; CI should run the new test. |
There was a problem hiding this comment.
🟡 Changes recommended
Add the required test-file license header before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
sdk/storage/azure-storage-blob/azure/storage/blob/_shared_access_signature.py:356
- This user-visible SAS behavior change is missing from the package's unreleased release notes. Please add an entry under
CHANGELOG.md:8(12.32.0b1→Bugs Fixed) so customers can discover that backslash-containing blob names now generate valid SAS tokens.
# Normalize backslashes to forward slashes to match Azure Storage service behavior
# Go and .NET SDKs already perform this normalization
path = path.replace("\\", "/")
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| @@ -0,0 +1,19 @@ | |||
| from datetime import datetime, timedelta | |||
|
Dependency and runtime follow-up: I installed the existing package development dependencies in an isolated environment (D:\repo\repostew.venv-azure-48849) and ran the focused test from the package directory. python -m pytest tests/test_shared_access_signature.py -q now passes: 1 passed (one unrelated datetime deprecation warning, 24.67s). This confirms the new backslash canonical-resource SAS regression test executes successfully. |
Fix #48690 - generate_blob_sas backslash normalization
Description
When building the SAS string-to-sign, the SDK was inserting the blob name verbatim into the canonical resource without normalizing backslash () to forward slash (/).
Azure Storage service normalizes \ to / when validating SAS signatures, causing generated SAS tokens to be rejected (HTTP 403) for blob names containing backslashes.
This fix aligns Python SDK behavior with Go and .NET SDKs which already perform this normalization.
Fix
Added path = path.replace("\\", "/") in add_resource_signature() method before building the canonical resource string.
Testing
The issue reporter verified this fix against Azurite (the official Storage emulator).