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.

Sign up for GitHub

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

GH-42134: [C++][FS][Azure] Validate AzureOptions::{blob,dfs}_storage_scheme #42135

Merged
merged 2 commits into from
Jun 14, 2024
Merged

GH-42134: [C++][FS][Azure] Validate AzureOptions::{blob,dfs}_storage_scheme #42135

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
adca6f6
GH-42134: [C++][FS][Azure] Validate AzureOptions::{blob,dfs}_storage_…
kou Jun 13, 2024
b66e7de
Use valid scheme
kou Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cpp/src/arrow/filesystem/azurefs.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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>(
Expand Down
44 changes: 44 additions & 0 deletions cpp/src/arrow/filesystem/azurefs_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down Expand Up @@ -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:
Expand Down
Loading