Resolved the issue in datalake/file where file/path/directory/share/f… (#796)

katmsft · web-flow · commit ef4b7c526896 · 2020-10-16T14:26:50.000+08:00
* Resolved the issue in datalake/file where file/path/directory/share/filesystem name are not encoded.

* Refined special char string

* Resolve comments.
diff --git a/sdk/storage/azure-storage-blobs/CHANGELOG.md b/sdk/storage/azure-storage-blobs/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Release History
 
+## 1.0.0-beta.4
+
+### Bug Fixes
+
+* Unencoded Container/Blob name is now encoded.
+
 ## 1.0.0-beta.3 (2020-10-13)
 
 ### New Features
diff --git a/sdk/storage/azure-storage-blobs/version.txt b/sdk/storage/azure-storage-blobs/version.txt
@@ -1 +1 @@
-1.0.0-beta.3
+1.0.0-beta.4
diff --git a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Release History
 
+## 1.0.0-beta.4
+
+### Bug Fixes
+
+* Unencoded FileSystem/File/Path/Directory name is now encoded.
+
 ## 1.0.0-beta.3 (2020-10-13)
 
 ### New Features
diff --git a/sdk/storage/azure-storage-files-datalake/src/datalake_directory_client.cpp b/sdk/storage/azure-storage-files-datalake/src/datalake_directory_client.cpp
@@ -28,8 +28,8 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   {
     auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);
     auto directoryUri = std::move(parsedConnectionString.DataLakeServiceUri);
-    directoryUri.AppendPath(fileSystemName);
-    directoryUri.AppendPath(path);
+    directoryUri.AppendPath(Storage::Details::UrlEncodePath(fileSystemName));
+    directoryUri.AppendPath(Storage::Details::UrlEncodePath(path));
 
     if (parsedConnectionString.KeyCredential)
     {
@@ -132,9 +132,9 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   FileClient DirectoryClient::GetFileClient(const std::string& path) const
   {
     auto builder = m_dfsUri;
-    builder.AppendPath(path);
+    builder.AppendPath(Storage::Details::UrlEncodePath(path));
     auto blobClient = m_blobClient;
-    blobClient.m_blobUrl.AppendPath(path);
+    blobClient.m_blobUrl.AppendPath(Storage::Details::UrlEncodePath(path));
     auto blockBlobClient = blobClient.GetBlockBlobClient();
     return FileClient(
         std::move(builder), std::move(blobClient), std::move(blockBlobClient), m_pipeline);
@@ -143,9 +143,9 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   DirectoryClient DirectoryClient::GetSubDirectoryClient(const std::string& path) const
   {
     auto builder = m_dfsUri;
-    builder.AppendPath(path);
+    builder.AppendPath(Storage::Details::UrlEncodePath(path));
     auto blobClient = m_blobClient;
-    blobClient.m_blobUrl.AppendPath(path);
+    blobClient.m_blobUrl.AppendPath(Storage::Details::UrlEncodePath(path));
     return DirectoryClient(std::move(builder), std::move(blobClient), m_pipeline);
   }
 
diff --git a/sdk/storage/azure-storage-files-datalake/src/datalake_file_client.cpp b/sdk/storage/azure-storage-files-datalake/src/datalake_file_client.cpp
@@ -106,8 +106,8 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   {
     auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);
     auto fileUri = std::move(parsedConnectionString.DataLakeServiceUri);
-    fileUri.AppendPath(fileSystemName);
-    fileUri.AppendPath(filePath);
+    fileUri.AppendPath(Storage::Details::UrlEncodePath(fileSystemName));
+    fileUri.AppendPath(Storage::Details::UrlEncodePath(filePath));
 
     if (parsedConnectionString.KeyCredential)
     {
diff --git a/sdk/storage/azure-storage-files-datalake/src/datalake_file_system_client.cpp b/sdk/storage/azure-storage-files-datalake/src/datalake_file_system_client.cpp
@@ -46,7 +46,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   {
     auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);
     auto fileSystemUri = std::move(parsedConnectionString.DataLakeServiceUri);
-    fileSystemUri.AppendPath(fileSystemName);
+    fileSystemUri.AppendPath(Storage::Details::UrlEncodePath(fileSystemName));
 
     if (parsedConnectionString.KeyCredential)
     {
@@ -163,15 +163,15 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   PathClient FileSystemClient::GetPathClient(const std::string& path) const
   {
     auto builder = m_dfsUri;
-    builder.AppendPath(path);
+    builder.AppendPath(Storage::Details::UrlEncodePath(path));
     return PathClient(builder, m_blobContainerClient.GetBlobClient(path), m_pipeline);
   }
 
   FileClient FileSystemClient::GetFileClient(const std::string& path) const
   {
 
     auto builder = m_dfsUri;
-    builder.AppendPath(path);
+    builder.AppendPath(Storage::Details::UrlEncodePath(path));
     auto blobClient = m_blobContainerClient.GetBlobClient(path);
     auto blockBlobClient = blobClient.GetBlockBlobClient();
     return FileClient(
@@ -181,7 +181,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   DirectoryClient FileSystemClient::GetDirectoryClient(const std::string& path) const
   {
     auto builder = m_dfsUri;
-    builder.AppendPath(path);
+    builder.AppendPath(Storage::Details::UrlEncodePath(path));
     return DirectoryClient(builder, m_blobContainerClient.GetBlobClient(path), m_pipeline);
   }
 
diff --git a/sdk/storage/azure-storage-files-datalake/src/datalake_path_client.cpp b/sdk/storage/azure-storage-files-datalake/src/datalake_path_client.cpp
@@ -90,8 +90,8 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   {
     auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);
     auto pathUri = std::move(parsedConnectionString.DataLakeServiceUri);
-    pathUri.AppendPath(fileSystemName);
-    pathUri.AppendPath(path);
+    pathUri.AppendPath(Storage::Details::UrlEncodePath(fileSystemName));
+    pathUri.AppendPath(Storage::Details::UrlEncodePath(path));
 
     if (parsedConnectionString.KeyCredential)
     {
diff --git a/sdk/storage/azure-storage-files-datalake/src/datalake_service_client.cpp b/sdk/storage/azure-storage-files-datalake/src/datalake_service_client.cpp
@@ -164,7 +164,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
   FileSystemClient ServiceClient::GetFileSystemClient(const std::string& fileSystemName) const
   {
     auto builder = m_dfsUri;
-    builder.AppendPath(fileSystemName);
+    builder.AppendPath(Storage::Details::UrlEncodePath(fileSystemName));
     return FileSystemClient(
         builder, m_blobServiceClient.GetBlobContainerClient(fileSystemName), m_pipeline);
   }
@@ -179,8 +179,8 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {
     blobOptions.MaxResults = options.MaxResults;
     auto result = m_blobServiceClient.ListBlobContainersSegment(blobOptions);
     auto response = ListFileSystemsSegmentResult();
-    response.ContinuationToken
-        = result->ContinuationToken.empty() ? response.ContinuationToken : result->ContinuationToken;
+    response.ContinuationToken = result->ContinuationToken.empty() ? response.ContinuationToken
+                                                                   : result->ContinuationToken;
     response.Filesystems = FileSystemsFromContainerItems(result->Items);
     return Azure::Core::Response<ListFileSystemsSegmentResult>(
         std::move(response), result.ExtractRawResponse());
diff --git a/sdk/storage/azure-storage-files-datalake/test/datalake_file_system_client_test.cpp b/sdk/storage/azure-storage-files-datalake/test/datalake_file_system_client_test.cpp
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: MIT
 
 #include "datalake_file_system_client_test.hpp"
+#include "azure/storage/common/crypt.hpp"
 #include "azure/storage/files/datalake/datalake_options.hpp"
 
 #include <algorithm>
@@ -249,4 +250,36 @@ namespace Azure { namespace Storage { namespace Test {
       EXPECT_LE(2U, response->Paths.size());
     }
   }
+
+  TEST_F(DataLakeFileSystemClientTest, UnencodedPathDirectoryFileNameWorks)
+  {
+    const std::string non_ascii_word = "\xE6\xB5\x8B\xE8\xAF\x95";
+    const std::string encoded_non_ascii_word = "%E6%B5%8B%E8%AF%95";
+    std::string baseName = "a b c / !@#$%^&*(?/<>,.;:'\"[]{}|`~\\) def" + non_ascii_word;
+    {
+      std::string pathName = baseName + RandomString();
+      auto pathClient = m_fileSystemClient->GetPathClient(pathName);
+      EXPECT_NO_THROW(pathClient.Create(Files::DataLake::PathResourceType::File));
+      auto pathUrl = pathClient.GetUri();
+      EXPECT_EQ(
+          pathUrl, m_fileSystemClient->GetUri() + "/" + Storage::Details::UrlEncodePath(pathName));
+    }
+    {
+      std::string directoryName = baseName + RandomString();
+      auto directoryClient = m_fileSystemClient->GetDirectoryClient(directoryName);
+      EXPECT_NO_THROW(directoryClient.Create());
+      auto directoryUrl = directoryClient.GetUri();
+      EXPECT_EQ(
+          directoryUrl,
+          m_fileSystemClient->GetUri() + "/" + Storage::Details::UrlEncodePath(directoryName));
+    }
+    {
+      std::string fileName = baseName + RandomString();
+      auto fileClient = m_fileSystemClient->GetFileClient(fileName);
+      EXPECT_NO_THROW(fileClient.Create());
+      auto fileUrl = fileClient.GetUri();
+      EXPECT_EQ(
+          fileUrl, m_fileSystemClient->GetUri() + "/" + Storage::Details::UrlEncodePath(fileName));
+    }
+  }
 }}} // namespace Azure::Storage::Test
diff --git a/sdk/storage/azure-storage-files-datalake/version.txt b/sdk/storage/azure-storage-files-datalake/version.txt
@@ -1 +1 @@
-1.0.0-beta.3
+1.0.0-beta.4
diff --git a/sdk/storage/azure-storage-files-shares/CHANGELOG.md b/sdk/storage/azure-storage-files-shares/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Release History
 
+## 1.0.0-beta.4
+
+### Bug Fixes
+
+* Unencoded Share/File/Directory name is now encoded.
+
 ## 1.0.0-beta.3 (2020-10-13)
 
 ### New Features
diff --git a/sdk/storage/azure-storage-files-shares/src/share_client.cpp b/sdk/storage/azure-storage-files-shares/src/share_client.cpp
@@ -24,7 +24,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
   {
     auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);
     auto shareUri = std::move(parsedConnectionString.FileServiceUri);
-    shareUri.AppendPath(shareName);
+    shareUri.AppendPath(Storage::Details::UrlEncodePath(shareName));
 
     if (parsedConnectionString.KeyCredential)
     {
@@ -115,14 +115,14 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
   DirectoryClient ShareClient::GetDirectoryClient(const std::string& directoryPath) const
   {
     auto builder = m_shareUri;
-    builder.AppendPath(directoryPath);
+    builder.AppendPath(Storage::Details::UrlEncodePath(directoryPath));
     return DirectoryClient(builder, m_pipeline);
   }
 
   FileClient ShareClient::GetFileClient(const std::string& filePath) const
   {
     auto builder = m_shareUri;
-    builder.AppendPath(filePath);
+    builder.AppendPath(Storage::Details::UrlEncodePath(filePath));
     return FileClient(builder, m_pipeline);
   }
 
diff --git a/sdk/storage/azure-storage-files-shares/src/share_directory_client.cpp b/sdk/storage/azure-storage-files-shares/src/share_directory_client.cpp
@@ -24,8 +24,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
   {
     auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);
     auto directoryUri = std::move(parsedConnectionString.FileServiceUri);
-    directoryUri.AppendPath(shareName);
-    directoryUri.AppendPath(directoryPath);
+    directoryUri.AppendPath(Storage::Details::UrlEncodePath(shareName));
+    directoryUri.AppendPath(Storage::Details::UrlEncodePath(directoryPath));
 
     if (parsedConnectionString.KeyCredential)
     {
@@ -118,14 +118,14 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
   DirectoryClient DirectoryClient::GetSubDirectoryClient(const std::string& subDirectoryName) const
   {
     auto builder = m_shareDirectoryUri;
-    builder.AppendPath(subDirectoryName);
+    builder.AppendPath(Storage::Details::UrlEncodePath(subDirectoryName));
     return DirectoryClient(builder, m_pipeline);
   }
 
   FileClient DirectoryClient::GetFileClient(const std::string& filePath) const
   {
     auto builder = m_shareDirectoryUri;
-    builder.AppendPath(filePath);
+    builder.AppendPath(Storage::Details::UrlEncodePath(filePath));
     return FileClient(builder, m_pipeline);
   }
 
diff --git a/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp b/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp
@@ -27,8 +27,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
   {
     auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);
     auto fileUri = std::move(parsedConnectionString.FileServiceUri);
-    fileUri.AppendPath(shareName);
-    fileUri.AppendPath(filePath);
+    fileUri.AppendPath(Storage::Details::UrlEncodePath(shareName));
+    fileUri.AppendPath(Storage::Details::UrlEncodePath(filePath));
 
     if (parsedConnectionString.KeyCredential)
     {
diff --git a/sdk/storage/azure-storage-files-shares/src/share_service_client.cpp b/sdk/storage/azure-storage-files-shares/src/share_service_client.cpp
@@ -111,7 +111,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
   ShareClient ServiceClient::GetShareClient(const std::string& shareName) const
   {
     auto builder = m_serviceUri;
-    builder.AppendPath(shareName);
+    builder.AppendPath(Storage::Details::UrlEncodePath(shareName));
     return ShareClient(builder, m_pipeline);
   }
 
diff --git a/sdk/storage/azure-storage-files-shares/test/share_client_test.cpp b/sdk/storage/azure-storage-files-shares/test/share_client_test.cpp
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // SPDX-License-Identifier: MIT
 
+#include "azure/storage/common/crypt.hpp"
 #include "share_client_test.hpp"
 
 #include <algorithm>
@@ -298,4 +299,27 @@ namespace Azure { namespace Storage { namespace Test {
     EXPECT_THROW(m_shareClient->Delete(), StorageError);
   }
 
+  TEST_F(FileShareClientTest, UnencodedDirectoryFileNameWorks)
+  {
+    const std::string non_ascii_word = "\xE6\xB5\x8B\xE8\xAF\x95";
+    const std::string encoded_non_ascii_word = "%E6%B5%8B%E8%AF%95";
+    std::string baseName = "a b c !@#$%^&(,.;'[]{}`~) def" + non_ascii_word;
+
+    {
+      std::string directoryName = baseName + RandomString();
+      auto directoryClient = m_shareClient->GetDirectoryClient(directoryName);
+      EXPECT_NO_THROW(directoryClient.Create());
+      auto directoryUrl = directoryClient.GetUri();
+      EXPECT_EQ(
+          directoryUrl,
+          m_shareClient->GetUri() + "/" + Storage::Details::UrlEncodePath(directoryName));
+    }
+    {
+      std::string fileName = baseName + RandomString();
+      auto fileClient = m_shareClient->GetFileClient(fileName);
+      EXPECT_NO_THROW(fileClient.Create(1024));
+      auto fileUrl = fileClient.GetUri();
+      EXPECT_EQ(fileUrl, m_shareClient->GetUri() + "/" + Storage::Details::UrlEncodePath(fileName));
+    }
+  }
 }}} // namespace Azure::Storage::Test
diff --git a/sdk/storage/azure-storage-files-shares/version.txt b/sdk/storage/azure-storage-files-shares/version.txt
@@ -1 +1 @@
-1.0.0-beta.3
+1.0.0-beta.4

Original file line number	Diff line number	Diff line change
`@@ -106,8 +106,8 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {`
`106`	`106`	`{`
`107`	`107`	`auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);`
`108`	`108`	`auto fileUri = std::move(parsedConnectionString.DataLakeServiceUri);`
`109`		`- fileUri.AppendPath(fileSystemName);`
`110`		`- fileUri.AppendPath(filePath);`
	`109`	`+ fileUri.AppendPath(Storage::Details::UrlEncodePath(fileSystemName));`
	`110`	`+ fileUri.AppendPath(Storage::Details::UrlEncodePath(filePath));`
`111`	`111`
`112`	`112`	`if (parsedConnectionString.KeyCredential)`
`113`	`113`	`{`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {`
`46`	`46`	`{`
`47`	`47`	`auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);`
`48`	`48`	`auto fileSystemUri = std::move(parsedConnectionString.DataLakeServiceUri);`
`49`		`- fileSystemUri.AppendPath(fileSystemName);`
	`49`	`+ fileSystemUri.AppendPath(Storage::Details::UrlEncodePath(fileSystemName));`
`50`	`50`
`51`	`51`	`if (parsedConnectionString.KeyCredential)`
`52`	`52`	`{`
`@@ -163,15 +163,15 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {`
`163`	`163`	`PathClient FileSystemClient::GetPathClient(const std::string& path) const`
`164`	`164`	`{`
`165`	`165`	`auto builder = m_dfsUri;`
`166`		`- builder.AppendPath(path);`
	`166`	`+ builder.AppendPath(Storage::Details::UrlEncodePath(path));`
`167`	`167`	`return PathClient(builder, m_blobContainerClient.GetBlobClient(path), m_pipeline);`
`168`	`168`	`}`
`169`	`169`
`170`	`170`	`FileClient FileSystemClient::GetFileClient(const std::string& path) const`
`171`	`171`	`{`
`172`	`172`
`173`	`173`	`auto builder = m_dfsUri;`
`174`		`- builder.AppendPath(path);`
	`174`	`+ builder.AppendPath(Storage::Details::UrlEncodePath(path));`
`175`	`175`	`auto blobClient = m_blobContainerClient.GetBlobClient(path);`
`176`	`176`	`auto blockBlobClient = blobClient.GetBlockBlobClient();`
`177`	`177`	`return FileClient(`
`@@ -181,7 +181,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {`
`181`	`181`	`DirectoryClient FileSystemClient::GetDirectoryClient(const std::string& path) const`
`182`	`182`	`{`
`183`	`183`	`auto builder = m_dfsUri;`
`184`		`- builder.AppendPath(path);`
	`184`	`+ builder.AppendPath(Storage::Details::UrlEncodePath(path));`
`185`	`185`	`return DirectoryClient(builder, m_blobContainerClient.GetBlobClient(path), m_pipeline);`
`186`	`186`	`}`
`187`	`187`
Original file line number	Diff line number	Diff line change
`@@ -90,8 +90,8 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake {`
`90`	`90`	`{`
`91`	`91`	`auto parsedConnectionString = Azure::Storage::Details::ParseConnectionString(connectionString);`
`92`	`92`	`auto pathUri = std::move(parsedConnectionString.DataLakeServiceUri);`
`93`		`- pathUri.AppendPath(fileSystemName);`
`94`		`- pathUri.AppendPath(path);`
	`93`	`+ pathUri.AppendPath(Storage::Details::UrlEncodePath(fileSystemName));`
	`94`	`+ pathUri.AppendPath(Storage::Details::UrlEncodePath(path));`
`95`	`95`
`96`	`96`	`if (parsedConnectionString.KeyCredential)`
`97`	`97`	`{`