This is an automated email from the ASF dual-hosted git repository.
raulcd pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 7a53fd7f13 GH-50204: [C++] Fix S3FileSystem::MakeUri dropping the
bucket on Windows MinGW (#50205)
7a53fd7f13 is described below
commit 7a53fd7f13c6cbf3dbb3d06a2fb8c64258ee0633
Author: Raúl Cumplido <[email protected]>
AuthorDate: Thu Jun 18 09:16:12 2026 +0200
GH-50204: [C++] Fix S3FileSystem::MakeUri dropping the bucket on Windows
MinGW (#50205)
### Rationale for this change
`S3FileSystem::MakeUri` wasn't exercised on CI only on `s3fs-module-test`
and on Linux. It currently fails for MinGW.
### What changes are included in this PR?
Properly parse URI on both Linux/Windows MSVC and MinGW environments. Added
tests for coverage.
### Are these changes tested?
Yes, new tests have been added.
### Are there any user-facing changes?
No
* GitHub Issue: #50204
Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Raúl Cumplido <[email protected]>
---
cpp/src/arrow/filesystem/s3fs.cc | 18 +++++++++++++-----
cpp/src/arrow/filesystem/s3fs_test.cc | 29 +++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/cpp/src/arrow/filesystem/s3fs.cc b/cpp/src/arrow/filesystem/s3fs.cc
index 0c15f6f184..970ee9d71e 100644
--- a/cpp/src/arrow/filesystem/s3fs.cc
+++ b/cpp/src/arrow/filesystem/s3fs.cc
@@ -3052,13 +3052,21 @@ Result<std::string> S3FileSystem::MakeUri(std::string
path) const {
if (path.length() <= 1 || path[0] != '/') {
return Status::Invalid("MakeUri requires an absolute, non-root path, got
", path);
}
- ARROW_ASSIGN_OR_RAISE(auto uri, util::UriFromAbsolutePath(path));
+ ARROW_ASSIGN_OR_RAISE(auto uri_from_path, util::UriFromAbsolutePath(path));
+ constexpr std::string_view kFileScheme = "file://";
+ std::string_view uri_view(uri_from_path);
+ if (uri_view.starts_with(kFileScheme)) {
+ uri_view.remove_prefix(kFileScheme.size());
+ }
+ if (uri_view.starts_with("/")) {
+ // Remove leading slash if present
+ uri_view.remove_prefix(1);
+ }
+ std::string uri = "s3://";
if (!options().GetAccessKey().empty()) {
- uri = "s3://" + options().GetAccessKey() + ":" + options().GetSecretKey()
+ "@" +
- uri.substr("file:///"s.size());
- } else {
- uri = "s3" + uri.substr("file"s.size());
+ uri += options().GetAccessKey() + ":" + options().GetSecretKey() + "@";
}
+ uri += std::string(uri_view);
uri += "?";
uri += "region=" + util::UriEscape(options().region);
uri += "&";
diff --git a/cpp/src/arrow/filesystem/s3fs_test.cc
b/cpp/src/arrow/filesystem/s3fs_test.cc
index 6be20603a8..52c2621249 100644
--- a/cpp/src/arrow/filesystem/s3fs_test.cc
+++ b/cpp/src/arrow/filesystem/s3fs_test.cc
@@ -481,6 +481,35 @@ TEST_F(S3FileSystemRegionTest, EnvironmentVariable) {
}
#endif
+////////////////////////////////////////////////////////////////////////////
+// S3FileSystem make URI test
+
+class S3FileSystemMakeUri : public AwsTestMixin {};
+
+TEST_F(S3FileSystemMakeUri, MakeUriWithCredentials) {
+ S3Options options;
+ options.ConfigureAccessKey("minio", "miniopass");
+ options.region = "us-east-1";
+ ASSERT_OK_AND_ASSIGN(auto fs, S3FileSystem::Make(options));
+ ASSERT_OK_AND_ASSIGN(auto uri,
fs->MakeUri("/bucket/somedir/subdir/subfile"));
+ EXPECT_EQ(uri,
+ "s3://minio:miniopass@bucket/somedir/subdir/subfile"
+ "?region=us-east-1&scheme=https&endpoint_override="
+ "&allow_bucket_creation=0&allow_bucket_deletion=0");
+}
+
+TEST_F(S3FileSystemMakeUri, MakeUriWithoutCredentials) {
+ S3Options options;
+ options.ConfigureAnonymousCredentials();
+ options.region = "us-east-1";
+ ASSERT_OK_AND_ASSIGN(auto fs, S3FileSystem::Make(options));
+ ASSERT_OK_AND_ASSIGN(auto uri,
fs->MakeUri("/bucket/somedir/subdir/subfile"));
+ EXPECT_EQ(uri,
+ "s3://bucket/somedir/subdir/subfile"
+ "?region=us-east-1&scheme=https&endpoint_override="
+ "&allow_bucket_creation=0&allow_bucket_deletion=0");
+}
+
////////////////////////////////////////////////////////////////////////////
// Basic test for the Minio test server.
