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

Fixed storage bug where blob, file, and directory names were not url … #11657

Merged

Conversation

seanmcc-msft
Copy link
Member

@seanmcc-msft seanmcc-msft commented Apr 29, 2020

…encoded

Resolves #11602.

This is not an issue in Data Lake, because we don't have client constructors that take file or directory names, only Uris. This is not an issue in Queues because '-' is the only symbol allowed in queue names.

@@ -61,6 +61,28 @@ public void Ctor_ConnectionString()
Assert.AreEqual("accountName", builder2.AccountName);
}

[Test]
[Ignore("Test framework doesn't allow recorded tests with connection string because the word 'Sanitized' is not base-64 encoded, so we can't pass connection string validation")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be Live test then ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

{
// Arrange
await using DisposingContainer test = await GetTestContainerAsync();
string blobName = "!*'();[]:@&%=+$,/?#";
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I recall (@rickle-msft plz chime in if I'm wrong here) @rickle-msft was/is chasing a bug in Java where German Umlauts got encoded with different target encoding (turns out Url encoding/percent encoding has target encoding...) than expected by server side. Might be worth adding that to test case suite.

Copy link
Member Author

Choose a reason for hiding this comment

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

what's target encoding?

Copy link
Contributor

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Here's the PR where this was addressed in Java.

Copy link
Member Author

@seanmcc-msft seanmcc-msft May 5, 2020

Choose a reason for hiding this comment

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

Added German characters to all the new tests, they are still passing.

@@ -50,6 +50,31 @@ public void Ctor_ConnectionString()
//Assert.AreEqual("accountName", builder.AccountName);
}

[Test]
[Ignore("Test framework doesn't allow recorded tests with connection string because the word 'Sanitized' is not base-64 encoded, so we can't pass connection string validation")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be "Live" as well ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, my bad.

@seanmcc-msft seanmcc-msft merged commit b598d12 into Azure:master May 5, 2020
@seanmcc-msft seanmcc-msft deleted the feature/storage/escapeBlobNames branch May 5, 2020 23:22
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-net that referenced this pull request Dec 2, 2020
[Hub Generated] Review request for Microsoft.CostManagement to add version stable/2019-11-01 (Azure#11657)

* Created Separate Swagger for Exports

* add exports swagger to readme.md

* Exports swagger correctness

* Exports swagger correctness

* dataSet in examples

* fix dataSet in examples

* add dataSet to exportList and exportExecutionList examples

* Update ExportCreateOrUpdateByManagementGroup.json

* dataSet property of QueryDefinition costmanagement.json

* undo changes to costmanagement.json

* use ExportDefinition instead of QueryDefinition

* prettier fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Azure Storage BlobClient doesn't properly escape filenames with certain characters
3 participants