apacheGH-42134: [C++][FS][Azure] Validate AzureOptions::{blob,dfs}_st…

…orage_scheme (apache#42135) ### Rationale for this change This is for showing user-friendly error message for invalid `{blob,dfs}_storage_scheme`. ### What changes are included in this PR? Validate `{blob,dfs}_storage_scheme` before we use them in `Make{Blob,DataLake}ServiceClient()`. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: apache#42134 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
CurtHagenlocher · Jun 14, 2024 · d078d5c · d078d5c
1 parent 02f461f
commit d078d5c
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 2 deletions.
diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc
@@ -366,6 +366,10 @@ Result<std::unique_ptr<Blobs::BlobServiceClient>> AzureOptions::MakeBlobServiceC
   if (account_name.empty()) {
     return Status::Invalid("AzureOptions doesn't contain a valid account name");
   }
+  if (!(blob_storage_scheme == "http" || blob_storage_scheme == "https")) {
+    return Status::Invalid("AzureOptions::blob_storage_scheme must be http or https: ",
+                           blob_storage_scheme);
+  }
   switch (credential_kind_) {
     case CredentialKind::kAnonymous:
       return std::make_unique<Blobs::BlobServiceClient>(AccountBlobUrl(account_name));
@@ -393,6 +397,10 @@ AzureOptions::MakeDataLakeServiceClient() const {
   if (account_name.empty()) {
     return Status::Invalid("AzureOptions doesn't contain a valid account name");
   }
+  if (!(dfs_storage_scheme == "http" || dfs_storage_scheme == "https")) {
+    return Status::Invalid("AzureOptions::dfs_storage_scheme must be http or https: ",
+                           dfs_storage_scheme);
+  }
   switch (credential_kind_) {
     case CredentialKind::kAnonymous:
       return std::make_unique<DataLake::DataLakeServiceClient>(

diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc
@@ -731,6 +731,38 @@ class TestAzureOptions : public ::testing::Test {
                                "unknown=invalid",
                                nullptr));
   }
+
+  void TestMakeBlobServiceClientInvalidAccountName() {
+    AzureOptions options;
+    ASSERT_RAISES_WITH_MESSAGE(
+        Invalid, "Invalid: AzureOptions doesn't contain a valid account name",
+        options.MakeBlobServiceClient());
+  }
+
+  void TestMakeBlobServiceClientInvalidBlobStorageScheme() {
+    AzureOptions options;
+    options.account_name = "user";
+    options.blob_storage_scheme = "abfs";
+    ASSERT_RAISES_WITH_MESSAGE(
+        Invalid, "Invalid: AzureOptions::blob_storage_scheme must be http or https: abfs",
+        options.MakeBlobServiceClient());
+  }
+
+  void TestMakeDataLakeServiceClientInvalidAccountName() {
+    AzureOptions options;
+    ASSERT_RAISES_WITH_MESSAGE(
+        Invalid, "Invalid: AzureOptions doesn't contain a valid account name",
+        options.MakeDataLakeServiceClient());
+  }
+
+  void TestMakeDataLakeServiceClientInvalidDfsStorageScheme() {
+    AzureOptions options;
+    options.account_name = "user";
+    options.dfs_storage_scheme = "abfs";
+    ASSERT_RAISES_WITH_MESSAGE(
+        Invalid, "Invalid: AzureOptions::dfs_storage_scheme must be http or https: abfs",
+        options.MakeDataLakeServiceClient());
+  }
 };
 
 TEST_F(TestAzureOptions, FromUriBlobStorage) { TestFromUriBlobStorage(); }
@@ -764,6 +796,18 @@ TEST_F(TestAzureOptions, FromUriDfsStorageAuthority) { TestFromUriDfsStorageAuth
 TEST_F(TestAzureOptions, FromUriInvalidQueryParameter) {
   TestFromUriInvalidQueryParameter();
 }
+TEST_F(TestAzureOptions, MakeBlobServiceClientInvalidAccountName) {
+  TestMakeBlobServiceClientInvalidAccountName();
+}
+TEST_F(TestAzureOptions, MakeBlobServiceClientInvalidBlobStorageScheme) {
+  TestMakeBlobServiceClientInvalidBlobStorageScheme();
+}
+TEST_F(TestAzureOptions, MakeDataLakeServiceClientInvalidAccountName) {
+  TestMakeDataLakeServiceClientInvalidAccountName();
+}
+TEST_F(TestAzureOptions, MakeDataLakeServiceClientInvalidDfsStorageScheme) {
+  TestMakeDataLakeServiceClientInvalidDfsStorageScheme();
+}
 
 class TestAzureFileSystem : public ::testing::Test {
  protected:

diff --git a/python/pyarrow/tests/test_fs.py b/python/pyarrow/tests/test_fs.py
@@ -1450,8 +1450,8 @@ def test_azurefs_options(pickle_module):
     fs3 = AzureFileSystem(account_name='fake-account', account_key='fakeaccount',
                           blob_storage_authority='fake-blob-authority',
                           dfs_storage_authority='fake-dfs-authority',
-                          blob_storage_scheme='fake-blob-scheme',
-                          dfs_storage_scheme='fake-dfs-scheme')
+                          blob_storage_scheme='https',
+                          dfs_storage_scheme='https')
     assert isinstance(fs3, AzureFileSystem)
     assert pickle_module.loads(pickle_module.dumps(fs3)) == fs3
     assert fs3 != fs2