Merge pull request #37 from bbockelm/service_url_slash

Correctly handle the case with trailing slash in the service URL
PelicanPlatform · Jul 23, 2024 · f6accba · f6accba
2 parents 6d3ca14 + 7d61731
commit f6accba
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 1 deletion.
diff --git a/src/S3Commands.cc b/src/S3Commands.cc
@@ -100,10 +100,22 @@ bool AmazonRequest::parseURL(const std::string &url, std::string &bucket_path,
 
 	if (m_style == "path") {
 		host = substring(url, hostStartIdx, resourceStartIdx);
-		path = substring(url, resourceStartIdx) + "/" + bucket + "/" + object;
+		auto resourcePrefix = substring(url, resourceStartIdx);
+		if (resourcePrefix[resourcePrefix.size() - 1] == '/') {
+			resourcePrefix =
+				substring(resourcePrefix, 0, resourcePrefix.size() - 1);
+		}
+		if (bucket.empty()) {
+			path = resourcePrefix + object;
+			bucket_path = resourcePrefix + object.substr(0, object.find('/'));
+		} else {
+			path = resourcePrefix + "/" + bucket + "/" + object;
+			bucket_path = resourcePrefix + "/" + bucket;
+		}
 	} else {
 		host = bucket + "." + substring(url, hostStartIdx, resourceStartIdx);
 		path = substring(url, resourceStartIdx) + object;
+		bucket_path = "/";
 	}
 
 	return true;

diff --git a/test/s3_tests.cc b/test/s3_tests.cc
@@ -162,6 +162,23 @@ s3.end
 	}
 };
 
+// Regression test for when the service_url ends in a `/`
+class FileSystemS3PathBucketSlash : public FileSystemFixtureBase {
+  protected:
+	virtual std::string GetConfig() override {
+		return R"(
+s3.begin
+s3.path_name        /test
+s3.service_name     s3.amazonaws.com
+s3.region           us-east-1
+s3.bucket_name      genome-browser
+s3.service_url      https://s3.us-east-1.amazonaws.com/
+s3.url_style        path
+s3.end
+)";
+	}
+};
+
 void TestDirectoryContents(S3FileSystem &fs, const std::string &dirname) {
 	std::unique_ptr<XrdOssDF> dir(fs.newDir());
 	ASSERT_TRUE(dir);
@@ -279,6 +296,18 @@ TEST_F(FileSystemS3PathNoBucket, List) {
 	TestDirectoryContents(fs, "/test/genome-browser/cells/tabula-sapiens/");
 }
 
+TEST_F(FileSystemS3PathBucketSlash, Stat) {
+	S3FileSystem fs(m_log.get(), m_configfn.c_str(), nullptr);
+	struct stat buff;
+	auto rv = fs.Stat("/test/cells/tabula-sapiens/cellbrowser.json.bak", &buff);
+	ASSERT_EQ(rv, 0) << "Failed to stat AWS bucket (" << strerror(-rv) << ")";
+}
+
+TEST_F(FileSystemS3PathBucketSlash, List) {
+	S3FileSystem fs(m_log.get(), m_configfn.c_str(), nullptr);
+	TestDirectoryContents(fs, "/test/cells/tabula-sapiens");
+}
+
 int main(int argc, char **argv) {
 	::testing::InitGoogleTest(&argc, argv);
 	return RUN_ALL_TESTS();