Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 10 additions & 11 deletions cpp/src/arrow/filesystem/s3fs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -271,17 +271,16 @@ Result<S3Options> S3Options::FromUri(const Uri& uri, std::string* out_path) {
options.ConfigureDefaultCredentials();
}

auto it = options_map.find("region");
if (it != options_map.end()) {
options.region = it->second;
}
it = options_map.find("scheme");
if (it != options_map.end()) {
options.scheme = it->second;
}
it = options_map.find("endpoint_override");
if (it != options_map.end()) {
options.endpoint_override = it->second;
for (const auto& kv : options_map) {
Comment thread
pitrou marked this conversation as resolved.
if (kv.first == "region") {
options.region = kv.second;
} else if (kv.first == "scheme") {
options.scheme = kv.second;
} else if (kv.first == "endpoint_override") {
options.endpoint_override = kv.second;
} else {
return Status::Invalid("Unexpected query parameter in S3 URI: '", kv.first, "'");
}
}

return options;
Expand Down
3 changes: 3 additions & 0 deletions cpp/src/arrow/filesystem/s3fs_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ TEST_F(S3OptionsTest, FromUri) {

// Missing bucket name
ASSERT_RAISES(Invalid, S3Options::FromUri("s3:///foo/bar/", &path));

// Invalid option
ASSERT_RAISES(Invalid, S3Options::FromUri("s3://mybucket/?xxx=zzz", &path));
}

TEST_F(S3OptionsTest, FromAccessKey) {
Expand Down
17 changes: 13 additions & 4 deletions cpp/src/arrow/util/uri.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,21 @@ std::string TextRangeToString(const UriTextRangeStructA& range) {
bool IsTextRangeSet(const UriTextRangeStructA& range) { return range.first != nullptr; }

#ifdef _WIN32
bool IsDriveSpec(util::string_view s) {
bool IsDriveSpec(const util::string_view s) {
return (s.length() >= 2 && s[1] == ':' &&
((s[0] >= 'A' && s[0] <= 'Z') || (s[0] >= 'a' && s[0] <= 'z')));
}
#endif

std::string UriUnescape(const util::string_view s) {
std::string result(s);
if (!result.empty()) {
auto end = uriUnescapeInPlaceA(&result[0]);
result.resize(end - &result[0]);
}
return result;
}

} // namespace

std::string UriEscape(const std::string& s) {
Expand Down Expand Up @@ -125,9 +134,9 @@ std::string Uri::username() const {
auto userpass = TextRangeToView(impl_->uri_.userInfo);
auto sep_pos = userpass.find_first_of(':');
if (sep_pos == util::string_view::npos) {
return std::string(userpass);
return UriUnescape(userpass);
} else {
return std::string(userpass.substr(0, sep_pos));
return UriUnescape(userpass.substr(0, sep_pos));
}
}

Expand All @@ -137,7 +146,7 @@ std::string Uri::password() const {
if (sep_pos == util::string_view::npos) {
return std::string();
} else {
return std::string(userpass.substr(sep_pos + 1));
return UriUnescape(userpass.substr(sep_pos + 1));
}
}

Expand Down
13 changes: 13 additions & 0 deletions cpp/src/arrow/util/uri_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,19 @@ TEST(Uri, ParseUserPass) {
ASSERT_EQ(uri.host(), "localhost");
ASSERT_EQ(uri.username(), "someuser");
ASSERT_EQ(uri.password(), "somepass");

// With %-encoding
ASSERT_OK(uri.Parse("http://some%20user%2Fname:somepass@localhost"));
ASSERT_EQ(uri.scheme(), "http");
ASSERT_EQ(uri.host(), "localhost");
ASSERT_EQ(uri.username(), "some user/name");
ASSERT_EQ(uri.password(), "somepass");

ASSERT_OK(uri.Parse("http://some%20user%2Fname:some%20pass%2Fword@localhost"));
ASSERT_EQ(uri.scheme(), "http");
ASSERT_EQ(uri.host(), "localhost");
ASSERT_EQ(uri.username(), "some user/name");
ASSERT_EQ(uri.password(), "some pass/word");
}

TEST(Uri, FileScheme) {
Expand Down