Skip to content

Commit

Permalink
[Storage] Fix for truncating URL paths with "#" (#44941)
Browse files Browse the repository at this point in the history
* WIP - still need to record tests

* Recordings

* Cleanup

* Fixed recordings

* Added fix for Shares and Datalake, Updated tests to use OAuth, Updated Changelogs

* Rerecord files tests

* Merge and record tests
  • Loading branch information
amnguye authored Jul 15, 2024
1 parent 7105c91 commit b10e21d
Show file tree
Hide file tree
Showing 14 changed files with 181 additions and 22 deletions.
1 change: 1 addition & 0 deletions sdk/storage/Azure.Storage.Blobs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### Breaking Changes

### Bugs Fixed
- Fixed bug where storage clients when constructed with URLs with '#' character would truncate the blob name at the '#'.

### Other Changes

Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/Azure.Storage.Blobs/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"AssetsRepo": "Azure/azure-sdk-assets",
"AssetsRepoPrefixPath": "net",
"TagPrefix": "net/storage/Azure.Storage.Blobs",
"Tag": "net/storage/Azure.Storage.Blobs_b8e4c4ca1e"
"Tag": "net/storage/Azure.Storage.Blobs_be75b1430c"
}
2 changes: 1 addition & 1 deletion sdk/storage/Azure.Storage.Blobs/src/BlobUriBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public BlobUriBuilder(Uri uri, bool trimBlobNameSlashes)
// Find the account, container, & blob names (if any)
if (!string.IsNullOrEmpty(uri.AbsolutePath))
{
var path = uri.GetPath();
var path = string.Concat(uri.GetPath(), uri.Fragment);

var startIndex = 0;

Expand Down
32 changes: 32 additions & 0 deletions sdk/storage/Azure.Storage.Blobs/tests/BlobBaseClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,38 @@ public async Task Ctor_ConnectionStringEscapeBlobName()
Assert.AreEqual(uploadResponse.Value.ETag, propertiesResponse.Value.ETag);
}

[RecordedTest]
public async Task Ctor_EscapeBlobName()
{
// Arrange
string blobName = "!*'();[]:@&%=+$,/#äÄöÖüÜß";
await using DisposingContainer test = await GetTestContainerAsync();
var data = GetRandomBuffer(Constants.KB);
BlobClient blob = InstrumentClient(test.Container.GetBlobClient(blobName));
ETag originalEtag;
using (var stream = new MemoryStream(data))
{
BlobContentInfo info = await blob.UploadAsync(stream);
originalEtag = info.ETag;
}

// Act
BlobUriBuilder uriBuilder = new BlobUriBuilder(new Uri(Tenants.TestConfigOAuth.BlobServiceEndpoint))
{
BlobContainerName = blob.BlobContainerName,
BlobName = blobName
};
BlobBaseClient freshBlobClient = InstrumentClient(new BlobBaseClient(
uriBuilder.ToUri(),
TestEnvironment.Credential,
GetOptions()));

// Assert
Assert.AreEqual(blobName, freshBlobClient.Name);
BlobProperties propertiesResponse = await freshBlobClient.GetPropertiesAsync();
Assert.AreEqual(originalEtag, propertiesResponse.ETag);
}

[RecordedTest]
public void Ctor_Uri()
{
Expand Down
1 change: 1 addition & 0 deletions sdk/storage/Azure.Storage.Files.DataLake/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### Breaking Changes

### Bugs Fixed
- Fixed bug where storage clients when constructed with URLs with '#' character would truncate the path at the '#'.

### Other Changes

Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/Azure.Storage.Files.DataLake/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"AssetsRepo": "Azure/azure-sdk-assets",
"AssetsRepoPrefixPath": "net",
"TagPrefix": "net/storage/Azure.Storage.Files.DataLake",
"Tag": "net/storage/Azure.Storage.Files.DataLake_133659189e"
"Tag": "net/storage/Azure.Storage.Files.DataLake_186c14971d"
}
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public DataLakeUriBuilder(Uri uri)
// Find the share & directory/file path (if any)
if (!string.IsNullOrEmpty(uri.AbsolutePath))
{
var path = uri.GetPath();
var path = string.Concat(uri.GetPath(), uri.Fragment);

var startIndex = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,34 @@ await TestHelper.AssertExpectedExceptionAsync<RequestFailedException>(
e => Assert.AreEqual("InvalidAuthenticationInfo", e.ErrorCode));
}

[RecordedTest]
public async Task Ctor_EscapeName()
{
// Arrange
string directoryName = "!*'();[]:@&%=+$,#äÄöÖüÜß";
await using DisposingFileSystem test = await GetNewFileSystem();
int size = Constants.KB;
var data = GetRandomBuffer(size);
DataLakeDirectoryClient directory = InstrumentClient(test.Container.GetDirectoryClient(directoryName));
await directory.CreateAsync();

// Act
DataLakeUriBuilder uriBuilder = new DataLakeUriBuilder(new Uri(Tenants.TestConfigHierarchicalNamespace.BlobServiceEndpoint))
{
FileSystemName = directory.FileSystemName,
DirectoryOrFilePath = directoryName
};
DataLakeDirectoryClient freshDirectoryClient = InstrumentClient(new DataLakeDirectoryClient(
uriBuilder.ToUri(),
TestEnvironment.Credential,
GetOptions()));

// Assert
bool exists = await freshDirectoryClient.ExistsAsync();
Assert.True(exists);
Assert.AreEqual(directoryName, freshDirectoryClient.Name);
}

[RecordedTest]
[TestCase(false)]
[TestCase(true)]
Expand Down
33 changes: 33 additions & 0 deletions sdk/storage/Azure.Storage.Files.DataLake/tests/FileClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,39 @@ await TestHelper.AssertExpectedExceptionAsync<RequestFailedException>(
e => Assert.AreEqual("InvalidAuthenticationInfo", e.ErrorCode));
}

[RecordedTest]
public async Task Ctor_EscapeName()
{
// Arrange
string fileName = "!*'();[]:@&%=+$,#äÄöÖüÜß";
await using DisposingFileSystem test = await GetNewFileSystem();
int size = Constants.KB;
var data = GetRandomBuffer(size);
DataLakeFileClient file = InstrumentClient(test.Container.GetFileClient(fileName));
ETag originalEtag;
using (var stream = new MemoryStream(data))
{
PathInfo info = await file.UploadAsync(stream);
originalEtag = info.ETag;
}

// Act
DataLakeUriBuilder uriBuilder = new DataLakeUriBuilder(new Uri(Tenants.TestConfigHierarchicalNamespace.BlobServiceEndpoint))
{
FileSystemName = file.FileSystemName,
DirectoryOrFilePath = fileName
};
DataLakeFileClient freshFileClient = InstrumentClient(new DataLakeFileClient(
uriBuilder.ToUri(),
TestEnvironment.Credential,
GetOptions()));

// Assert
Assert.AreEqual(fileName, freshFileClient.Name);
PathProperties propertiesResponse = await freshFileClient.GetPropertiesAsync();
Assert.AreEqual(originalEtag, propertiesResponse.ETag);
}

[RecordedTest]
[TestCase(false)]
[TestCase(true)]
Expand Down
1 change: 1 addition & 0 deletions sdk/storage/Azure.Storage.Files.Shares/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### Breaking Changes

### Bugs Fixed
- Fixed bug where storage clients when constructed with URLs with '#' character would truncate the path at the '#'.

### Other Changes

Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/Azure.Storage.Files.Shares/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"AssetsRepo": "Azure/azure-sdk-assets",
"AssetsRepoPrefixPath": "net",
"TagPrefix": "net/storage/Azure.Storage.Files.Shares",
"Tag": "net/storage/Azure.Storage.Files.Shares_f7982e093b"
"Tag": "net/storage/Azure.Storage.Files.Shares_093f14c48d"
}
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public ShareUriBuilder(Uri uri)
if (!string.IsNullOrEmpty(uri.AbsolutePath))
{
// If path starts with a slash, remove it
var path = uri.GetPath();
var path = string.Concat(uri.GetPath(), uri.Fragment);

var startIndex = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Azure.Core.TestFramework;
Expand Down Expand Up @@ -215,6 +216,32 @@ await TestHelper.AssertExpectedExceptionAsync<RequestFailedException>(
e => Assert.AreEqual("InvalidAuthenticationInfo", e.ErrorCode));
}

[RecordedTest]
public async Task Ctor_EscapeDirectoryName()
{
// Arrange
string directoryName = "$=;!#öÖ";
await using DisposingShare test = await GetTestShareAsync();
int size = Constants.KB;
var data = GetRandomBuffer(size);
ShareDirectoryClient directory = InstrumentClient(test.Share.GetDirectoryClient(directoryName));
await directory.CreateAsync();

// Act
ShareUriBuilder uriBuilder = new ShareUriBuilder(new Uri(Tenants.TestConfigOAuth.FileServiceEndpoint))
{
ShareName = directory.ShareName,
DirectoryOrFilePath = directoryName
};
ShareDirectoryClient freshDirectoryClient = InstrumentClient(new ShareDirectoryClient(
uriBuilder.ToUri(),
TestEnvironment.Credential,
GetOptions()));

// Assert
Assert.AreEqual(directoryName, freshDirectoryClient.Name);
}

[RecordedTest]
public void DirectoryPathsParsing()
{
Expand Down
68 changes: 52 additions & 16 deletions sdk/storage/Azure.Storage.Files.Shares/tests/FileClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public async Task Ctor_DefaultAudience()
ShareFileClient fileClient = directoryClient.GetFileClient(GetNewFileName());
await fileClient.CreateAsync(Constants.KB);

// Act - Create new blob client with the OAuth Credential and Audience
// Act - Create new Share client with the OAuth Credential and Audience
ShareClientOptions options = GetOptionsWithAudience(ShareAudience.DefaultAudience);

ShareUriBuilder uriBuilder = new ShareUriBuilder(new Uri(Tenants.TestConfigOAuth.FileServiceEndpoint))
Expand Down Expand Up @@ -147,7 +147,7 @@ public async Task Ctor_CustomAudience()
ShareFileClient fileClient = directoryClient.GetFileClient(GetNewFileName());
await fileClient.CreateAsync(Constants.KB);

// Act - Create new blob client with the OAuth Credential and Audience
// Act - Create new Share client with the OAuth Credential and Audience
ShareClientOptions options = GetOptionsWithAudience(new ShareAudience($"https://{test.Share.AccountName}.file.core.windows.net/"));

ShareUriBuilder uriBuilder = new ShareUriBuilder(new Uri(Tenants.TestConfigOAuth.FileServiceEndpoint))
Expand Down Expand Up @@ -177,7 +177,7 @@ public async Task Ctor_StorageAccountAudience()
ShareFileClient fileClient = directoryClient.GetFileClient(GetNewFileName());
await fileClient.CreateAsync(Constants.KB);

// Act - Create new blob client with the OAuth Credential and Audience
// Act - Create new Share client with the OAuth Credential and Audience
ShareClientOptions options = GetOptionsWithAudience(ShareAudience.CreateShareServiceAccountAudience(test.Share.AccountName));

ShareUriBuilder uriBuilder = new ShareUriBuilder(new Uri(Tenants.TestConfigOAuth.FileServiceEndpoint))
Expand All @@ -196,6 +196,42 @@ public async Task Ctor_StorageAccountAudience()
Assert.IsNotNull(exists);
}

[RecordedTest]
public async Task Ctor_EscapeFileName()
{
// Arrange
string fileName = "$=;!#öÖ";
await using DisposingShare test = await GetTestShareAsync();
int size = Constants.KB;
var data = GetRandomBuffer(size);
ShareFileClient file = InstrumentClient(test.Share.GetRootDirectoryClient().GetFileClient(fileName));
ETag originalEtag;
await file.CreateAsync(size);
using (var stream = new MemoryStream(data))
{
ShareFileUploadInfo info = await file.UploadAsync(stream);
originalEtag = info.ETag;
}

// Act
ShareUriBuilder uriBuilder = new ShareUriBuilder(new Uri(Tenants.TestConfigOAuth.FileServiceEndpoint))
{
ShareName = test.Share.Name,
DirectoryOrFilePath = fileName
};
ShareClientOptions options = GetOptions();
options.ShareTokenIntent = ShareTokenIntent.Backup;
ShareFileClient freshFileClient = InstrumentClient(new ShareFileClient(
uriBuilder.ToUri(),
TestEnvironment.Credential,
options));

// Assert
Assert.AreEqual(fileName, freshFileClient.Name);
ShareFileProperties propertiesResponse = await freshFileClient.GetPropertiesAsync();
Assert.AreEqual(originalEtag, propertiesResponse.ETag);
}

[RecordedTest]
public async Task Ctor_AudienceError()
{
Expand All @@ -206,8 +242,8 @@ public async Task Ctor_AudienceError()
ShareFileClient fileClient = directoryClient.GetFileClient(GetNewFileName());
await fileClient.CreateAsync(Constants.KB);

// Act - Create new blob client with the OAuth Credential and Audience
ShareClientOptions options = GetOptionsWithAudience(new ShareAudience("https://badaudience.blob.core.windows.net"));
// Act - Create new Share client with the OAuth Credential and Audience
ShareClientOptions options = GetOptionsWithAudience(new ShareAudience("https://badaudience.Share.core.windows.net"));

ShareUriBuilder uriBuilder = new ShareUriBuilder(new Uri(Tenants.TestConfigOAuth.FileServiceEndpoint))
{
Expand Down Expand Up @@ -4037,7 +4073,7 @@ public async Task ClearRangeAsync_TrailingDot()
[TestCase(30 * Constants.KB)]
[TestCase(50 * Constants.KB)]
[TestCase(501 * Constants.KB)]
public async Task UploadAsync_SmallBlobs(int size) =>
public async Task UploadAsync_SmallShares(int size) =>
// Use a 1KB threshold so we get a lot of individual blocks
await UploadAndVerify(size, Constants.KB);

Expand All @@ -4047,7 +4083,7 @@ public async Task UploadAsync_SmallBlobs(int size) =>
[TestCase(257 * Constants.MB)]
[TestCase(1 * Constants.GB)]
[Explicit("https://github.com/Azure/azure-sdk-for-net/issues/9120")]
public async Task UploadAsync_LargeBlobs(int size) =>
public async Task UploadAsync_LargeShares(int size) =>
// TODO: #6781 We don't want to add 1GB of random data in the recordings
await UploadAndVerify(size, Constants.MB);

Expand Down Expand Up @@ -6342,35 +6378,35 @@ public void CanGenerateSas_ClientConstructors()
{
// Arrange
var constants = TestConstants.Create(this);
var blobEndpoint = new Uri("https://127.0.0.1/" + constants.Sas.Account);
var blobSecondaryEndpoint = new Uri("https://127.0.0.1/" + constants.Sas.Account + "-secondary");
var storageConnectionString = new StorageConnectionString(constants.Sas.SharedKeyCredential, fileStorageUri: (blobEndpoint, blobSecondaryEndpoint));
var ShareEndpoint = new Uri("https://127.0.0.1/" + constants.Sas.Account);
var ShareSecondaryEndpoint = new Uri("https://127.0.0.1/" + constants.Sas.Account + "-secondary");
var storageConnectionString = new StorageConnectionString(constants.Sas.SharedKeyCredential, fileStorageUri: (ShareEndpoint, ShareSecondaryEndpoint));
string connectionString = storageConnectionString.ToString(true);

// Act - ShareDirectoryClient(string connectionString, string blobContainerName, string blobName)
// Act - ShareDirectoryClient(string connectionString, string ShareContainerName, string ShareName)
ShareFileClient directory = InstrumentClient(new ShareFileClient(
connectionString,
GetNewShareName(),
GetNewDirectoryName()));
Assert.IsTrue(directory.CanGenerateSasUri);

// Act - ShareFileClient(string connectionString, string blobContainerName, string blobName, BlobClientOptions options)
// Act - ShareFileClient(string connectionString, string ShareContainerName, string ShareName, ShareClientOptions options)
ShareFileClient directory2 = InstrumentClient(new ShareFileClient(
connectionString,
GetNewShareName(),
GetNewDirectoryName(),
GetOptions()));
Assert.IsTrue(directory2.CanGenerateSasUri);

// Act - ShareFileClient(Uri blobContainerUri, BlobClientOptions options = default)
// Act - ShareFileClient(Uri ShareContainerUri, ShareClientOptions options = default)
ShareFileClient directory3 = InstrumentClient(new ShareFileClient(
blobEndpoint,
ShareEndpoint,
GetOptions()));
Assert.IsFalse(directory3.CanGenerateSasUri);

// Act - ShareFileClient(Uri blobContainerUri, StorageSharedKeyCredential credential, BlobClientOptions options = default)
// Act - ShareFileClient(Uri ShareContainerUri, StorageSharedKeyCredential credential, ShareClientOptions options = default)
ShareFileClient directory4 = InstrumentClient(new ShareFileClient(
blobEndpoint,
ShareEndpoint,
constants.Sas.SharedKeyCredential,
GetOptions()));
Assert.IsTrue(directory4.CanGenerateSasUri);
Expand Down

0 comments on commit b10e21d

Please sign in to comment.