feat(azure): expose split_sas for custom SAS credential providers#721
Open
dbatomic wants to merge 1 commit into
Open
feat(azure): expose split_sas for custom SAS credential providers#721dbatomic wants to merge 1 commit into
split_sas for custom SAS credential providers#721dbatomic wants to merge 1 commit into
Conversation
`split_sas` parses an Azure Shared Access Signature string into the `Vec<(String, String)>` query pairs expected by `AzureCredential::SASToken`. It is what `MicrosoftAzureBuilder` runs internally when a raw SAS string is supplied via `AzureConfigKey::SasKey`. Users implementing their own `CredentialProvider<Credential = AzureCredential>` that fetches SAS tokens at request time (e.g. tokens refreshed from a secret manager or coordination service) currently have to reimplement this parser to feed pairs into `AzureCredential::SASToken`. Reimplementations are easy to get subtly wrong: in particular, using `form_urlencoded` decodes `+` as space, which corrupts base64-encoded SAS signatures. Make `split_sas` public, change its return type to the public `crate::Result<Vec<(String, String)>>` (the inner `Error` variants already convert into `crate::Error` via the existing `From` impl, so the two existing internal call sites are unchanged), document it, and re-export it from `object_store::azure`. Add tests for the empty/`?` edge cases and the missing-`=` error path.
kylebarron
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #720.
Rationale for this change
Users implementing their own
CredentialProvider<Credential = AzureCredential>that fetches SAS tokens at request time (e.g. tokens refreshed from a secret manager or coordination service) currently have to reimplement this parser to feed pairs intoAzureCredential::SASToken. Reimplementations are easy to get subtly wrong: in particular, usingform_urlencodeddecodes+as space, which corrupts base64-encoded SAS signatures.Make
split_saspublic, change its return type to the publiccrate::Result<Vec<(String, String)>>(the innerErrorvariants already convert intocrate::Errorvia the existingFromimpl, so the two existing internal call sites are unchanged), document it, and re-export it fromobject_store::azure. Add tests for the empty/?edge cases and the missing-=error path.What changes are included in this PR?
Plumbing to make
split_saspublic and additional test coverage. I tried to keep changes minimal - let me know if a broader API change is needed here, instead of a free function export.Are there any user-facing changes?
No.