diff --git a/src/S3Commands.cc b/src/S3Commands.cc index 1d37086..b48d1e4 100644 --- a/src/S3Commands.cc +++ b/src/S3Commands.cc @@ -557,6 +557,47 @@ bool AmazonS3Head::SendRequest() { return SendS3Request(noPayloadAllowed, 0, true); } +void AmazonS3Head::parseResponse() { + if (m_parsedResponse) { + return; + } + m_parsedResponse = true; + + const std::string &headers = getResultString(); + std::string line; + size_t current_newline = 0; + size_t next_newline = std::string::npos; + size_t last_character = headers.size(); + while (current_newline != std::string::npos && + current_newline != last_character - 1) { + next_newline = headers.find("\r\n", current_newline + 2); + line = substring(headers, current_newline + 2, next_newline); + + size_t colon = line.find(":"); + if (colon != std::string::npos && colon != line.size()) { + auto attr = substring(line, 0, colon); + auto value = substring(line, colon + 1); + trim(value); + toLower(attr); + + if (attr == "content-length") { + m_size = std::stol(value); + } else if (attr == "last-modified") { + struct tm t; + char *eos = strptime(value.c_str(), "%a, %d %b %Y %T %Z", &t); + if (eos == &value.c_str()[value.size()]) { + auto epoch = timegm(&t); + if (epoch != -1) { + m_last_modified = epoch; + } + } + } + } + + current_newline = next_newline; + } +} + // --------------------------------------------------------------------------- bool AmazonS3List::SendRequest(const std::string &continuationToken) { diff --git a/src/S3Commands.hh b/src/S3Commands.hh index 3e72d2e..8f60133 100644 --- a/src/S3Commands.hh +++ b/src/S3Commands.hh @@ -283,10 +283,21 @@ class AmazonS3Head final : public AmazonRequest { virtual bool SendRequest(); - off_t getSize() const { return m_size; } + off_t getSize() { + parseResponse(); + return m_size; + } + time_t getLastModified() { + parseResponse(); + return m_last_modified; + } private: + void parseResponse(); + + bool m_parsedResponse{false}; off_t m_size{0}; + time_t m_last_modified{0}; }; struct S3ObjectInfo { diff --git a/src/S3File.cc b/src/S3File.cc index fa6a56f..81871e0 100644 --- a/src/S3File.cc +++ b/src/S3File.cc @@ -104,6 +104,9 @@ int S3File::Open(const char *path, int Oflag, mode_t Mode, XrdOucEnv &env) { if (ai->getS3BucketName().empty()) { return -EINVAL; } + if (object == "") { + return -ENOENT; + } m_ai = *ai; m_object = object; @@ -167,53 +170,23 @@ int S3File::Fstat(struct stat *buff) { } } - std::string headers = head.getResultString(); - - std::string line; - size_t current_newline = 0; - size_t next_newline = std::string::npos; - size_t last_character = headers.size(); - while (current_newline != std::string::npos && - current_newline != last_character - 1) { - next_newline = headers.find("\r\n", current_newline + 2); - line = substring(headers, current_newline + 2, next_newline); - - size_t colon = line.find(":"); - if (colon != std::string::npos && colon != line.size()) { - std::string attr = substring(line, 0, colon); - std::string value = substring(line, colon + 1); - trim(value); - toLower(attr); - - if (attr == "content-length") { - this->content_length = std::stol(value); - } else if (attr == "last-modified") { - struct tm t; - char *eos = strptime(value.c_str(), "%a, %d %b %Y %T %Z", &t); - if (eos == &value.c_str()[value.size()]) { - time_t epoch = timegm(&t); - if (epoch != -1) { - this->last_modified = epoch; - } - } - } - } + content_length = head.getSize(); + last_modified = head.getLastModified(); - current_newline = next_newline; + if (buff) { + memset(buff, '\0', sizeof(struct stat)); + buff->st_mode = 0600 | S_IFREG; + buff->st_nlink = 1; + buff->st_uid = 1; + buff->st_gid = 1; + buff->st_size = content_length; + buff->st_mtime = last_modified; + buff->st_atime = 0; + buff->st_ctime = 0; + buff->st_dev = 0; + buff->st_ino = 0; } - memset(buff, '\0', sizeof(struct stat)); - buff->st_mode = 0600 | S_IFREG; - buff->st_nlink = 1; - buff->st_uid = 1; - buff->st_gid = 1; - buff->st_size = this->content_length; - buff->st_mtime = this->last_modified; - buff->st_atime = 0; - buff->st_ctime = 0; - buff->st_dev = 0; - buff->st_ino = 0; - return 0; } diff --git a/src/S3FileSystem.cc b/src/S3FileSystem.cc index c428a2f..ae016fe 100644 --- a/src/S3FileSystem.cc +++ b/src/S3FileSystem.cc @@ -41,6 +41,9 @@ #include #include +bool S3FileSystem::m_dir_marker = true; +std::string S3FileSystem::m_dir_marker_name = ".pelican_dir_marker"; + S3FileSystem::S3FileSystem(XrdSysLogger *lp, const char *configfn, XrdOucEnv *envP) : m_env(envP), m_log(lp, "s3_") { @@ -200,18 +203,37 @@ XrdOssDF *S3FileSystem::newFile(const char *user) { return new S3File(m_log, this); } +// +// Stat a path within the S3 bucket as if it were a hierarchical +// path. +// +// Note that S3 is _not_ a hierarchy and may contain objects that +// can't be represented inside XRootD. In that case, we just return +// an -ENOENT. +// +// For example, consider a setup with two objects: +// +// - /foo/bar.txt +// - /foo +// +// In this case, `Stat` of `/foo` will return a file so walking the +// bucket will miss `/foo/bar.txt` +// +// We will also return an ENOENT for objects with a trailing `/`. So, +// if there's a single object in the bucket: +// +// - /foo/bar.txt/ +// +// then a `Stat` of `/foo/bar.txt` and `/foo/bar.txt/` will both return +// `-ENOENT`. int S3FileSystem::Stat(const char *path, struct stat *buff, int opts, XrdOucEnv *env) { m_log.Log(XrdHTTPServer::Debug, "Stat", "Stat'ing path", path); - std::string localPath = path; - - if (std::count(localPath.begin(), localPath.end(), '/') == 1) { - localPath = localPath + "/"; - } std::string exposedPath, object; - auto rv = parsePath(localPath.c_str(), exposedPath, object); + auto rv = parsePath(path, exposedPath, object); if (rv != 0) { + m_log.Log(XrdHTTPServer::Debug, "Stat", "Failed to parse path:", path); return rv; } auto ai = getS3AccessInfo(exposedPath, object); @@ -225,14 +247,100 @@ int S3FileSystem::Stat(const char *path, struct stat *buff, int opts, } trimslashes(object); + if (object == "") { + if (m_dir_marker) { + // We even do the `Stat` for `/` despite the fact we always + // return the same directory object. This way, we test for + // permission denied or other errors with the S3 instance. + object = m_dir_marker_name; + } else { + if (buff) { + memset(buff, '\0', sizeof(struct stat)); + buff->st_mode = 0700 | S_IFDIR; + buff->st_nlink = 0; + buff->st_uid = 1; + buff->st_gid = 1; + buff->st_size = 4096; + buff->st_mtime = buff->st_atime = buff->st_ctime = 0; + buff->st_dev = 0; + buff->st_ino = 1; + } + return 0; + } + } + + // First, check to see if the file name is an object. If it's + // a 404 response, then we will assume it may be a directory. + AmazonS3Head headCommand = AmazonS3Head(*ai, object, m_log); + auto res = headCommand.SendRequest(); + if (res) { + if (buff) { + memset(buff, '\0', sizeof(struct stat)); + if (object == m_dir_marker_name) { + buff->st_mode = 0700 | S_IFDIR; + buff->st_size = 4096; + buff->st_nlink = 0; + } else { + buff->st_mode = 0600 | S_IFREG; + buff->st_size = headCommand.getSize(); + buff->st_nlink = 1; + } + buff->st_uid = buff->st_gid = 1; + buff->st_mtime = buff->st_atime = buff->st_ctime = 0; + buff->st_dev = 0; + buff->st_ino = 1; + } + return 0; + } else { + auto httpCode = headCommand.getResponseCode(); + if (httpCode == 0) { + if (m_log.getMsgMask() & XrdHTTPServer::Info) { + std::stringstream ss; + ss << "Failed to stat path " << path + << "; error: " << headCommand.getErrorMessage() + << " (code=" << headCommand.getErrorCode() << ")"; + m_log.Log(XrdHTTPServer::Info, "Stat", ss.str().c_str()); + } + return -EIO; + } + if (httpCode == 404) { + if (object == m_dir_marker_name) { + if (buff) { + memset(buff, '\0', sizeof(struct stat)); + buff->st_mode = 0700 | S_IFDIR; + buff->st_nlink = 0; + buff->st_uid = 1; + buff->st_gid = 1; + buff->st_size = 4096; + buff->st_mtime = buff->st_atime = buff->st_ctime = 0; + buff->st_dev = 0; + buff->st_ino = 1; + } + return 0; + } + object = object + "/"; + } else { + if (m_log.getMsgMask() & XrdHTTPServer::Info) { + std::stringstream ss; + ss << "Failed to stat path " << path << "; response code " + << httpCode; + m_log.Log(XrdHTTPServer::Info, "Stat", ss.str().c_str()); + } + return httpCode == 403 ? -EACCES : -EIO; + } + } + + // List the object name as a pseudo-directory. Limit the results + // back to a single item (we're just looking to see if there's a + // common prefix here). AmazonS3List listCommand(*ai, object, 1, m_log); - auto res = listCommand.SendRequest(""); + res = listCommand.SendRequest(""); if (!res) { auto httpCode = listCommand.getResponseCode(); if (httpCode == 0) { if (m_log.getMsgMask() & XrdHTTPServer::Info) { std::stringstream ss; - ss << "Failed to stat path " << localPath + ss << "Failed to stat path " << path << "; error: " << listCommand.getErrorMessage() << " (code=" << listCommand.getErrorCode() << ")"; m_log.Log(XrdHTTPServer::Info, "Stat", ss.str().c_str()); @@ -241,15 +349,13 @@ int S3FileSystem::Stat(const char *path, struct stat *buff, int opts, } else { if (m_log.getMsgMask() & XrdHTTPServer::Info) { std::stringstream ss; - ss << "Failed to stat path " << localPath << "; response code " + ss << "Failed to stat path " << path << "; response code " << httpCode; m_log.Log(XrdHTTPServer::Info, "Stat", ss.str().c_str()); } switch (httpCode) { case 404: return -ENOENT; - case 500: - return -EIO; case 403: return -EPERM; default: @@ -275,61 +381,37 @@ int S3FileSystem::Stat(const char *path, struct stat *buff, int opts, m_log.Log(XrdHTTPServer::Debug, "Stat", ss.str().c_str()); } - if (object.empty()) { - memset(buff, '\0', sizeof(struct stat)); - buff->st_mode = 0700 | S_IFDIR; - buff->st_nlink = 0; - buff->st_uid = 1; - buff->st_gid = 1; - buff->st_size = 4096; - buff->st_mtime = buff->st_atime = buff->st_ctime = 0; - buff->st_dev = 0; - buff->st_ino = 1; - return 0; - } - + // Recall we queried for 'object name' + '/'; as in, 'foo/' + // instead of 'foo'. + // If there's an object name with a trailing '/', then we + // aren't able to open it or otherwise represent it within + // XRootD. Hence, we just pretend it doesn't exist. bool foundObj = false; - size_t objSize = 0; for (const auto &obj : objInfo) { if (obj.m_key == object) { foundObj = true; - objSize = obj.m_size; break; } } if (foundObj) { - memset(buff, '\0', sizeof(struct stat)); - buff->st_mode = 0600 | S_IFREG; - buff->st_nlink = 1; - buff->st_uid = buff->st_gid = 1; - buff->st_size = objSize; - buff->st_mtime = buff->st_atime = buff->st_ctime = 0; - buff->st_dev = 0; - buff->st_ino = 1; - return 0; + return -ENOENT; } - auto desiredPrefix = object + "/"; - bool foundPrefix = false; - for (const auto &prefix : commonPrefixes) { - if (prefix == desiredPrefix) { - foundPrefix = true; - break; - } - } - if (!foundPrefix) { + if (!objInfo.size() && !commonPrefixes.size()) { return -ENOENT; } - memset(buff, '\0', sizeof(struct stat)); - buff->st_mode = 0700 | S_IFDIR; - buff->st_nlink = 0; - buff->st_uid = 1; - buff->st_gid = 1; - buff->st_size = 4096; - buff->st_mtime = buff->st_atime = buff->st_ctime = 0; - buff->st_dev = 0; - buff->st_ino = 1; + if (buff) { + memset(buff, '\0', sizeof(struct stat)); + buff->st_mode = 0700 | S_IFDIR; + buff->st_nlink = 0; + buff->st_uid = 1; + buff->st_gid = 1; + buff->st_size = 4096; + buff->st_mtime = buff->st_atime = buff->st_ctime = 0; + buff->st_dev = 0; + buff->st_ino = 1; + } return 0; } @@ -379,7 +461,8 @@ int S3FileSystem::parsePath(const char *fullPath, std::string &exposedPath, // Objects names may contain path separators. ++pathComponents; if (pathComponents == p.end()) { - return -ENOENT; + object = ""; + return 0; } std::filesystem::path objectPath = *pathComponents++; diff --git a/src/S3FileSystem.hh b/src/S3FileSystem.hh index 95c639e..a7c45e2 100644 --- a/src/S3FileSystem.hh +++ b/src/S3FileSystem.hh @@ -141,6 +141,17 @@ class S3FileSystem : public XrdOss { XrdOucEnv *m_env; XrdSysError m_log; + // The filesystem logic can test for an empty object to see if there's + // authorized access to the bucket. This relies on said object not + // existing -- a reasonable assumption but not foolproof. Hence, we have + // a boolean (currently not configurable) to disable the behavior. + // Note: in the future, if we want to create an "empty" directory, we could + // just create an empty object. + static bool m_dir_marker; + + // The name of the empty object for directory existence. + static std::string m_dir_marker_name; + bool handle_required_config(const char *desired_name, const std::string &source); std::map> m_s3_access_map; diff --git a/test/s3_unit_tests.cc b/test/s3_unit_tests.cc index c6bae9f..a39ab89 100644 --- a/test/s3_unit_tests.cc +++ b/test/s3_unit_tests.cc @@ -347,14 +347,21 @@ TEST_F(FileSystemS3Fixture, StatRoot) { ASSERT_EQ(fs.Stat("/test/", &buf, 0, nullptr), 0); ASSERT_EQ(buf.st_mode & S_IFDIR, S_IFDIR); + ASSERT_EQ(fs.Stat("//test/", &buf, 0, nullptr), 0); + ASSERT_EQ(buf.st_mode & S_IFDIR, S_IFDIR); + + ASSERT_EQ(fs.Stat("//test", &buf, 0, nullptr), 0); + ASSERT_EQ(buf.st_mode & S_IFDIR, S_IFDIR); + + ASSERT_EQ(fs.Stat("/test//", &buf, 0, nullptr), 0); + ASSERT_EQ(buf.st_mode & S_IFDIR, S_IFDIR); + ASSERT_EQ(fs.Stat("/test/statroot.txt", &buf, 0, nullptr), 0); ASSERT_EQ(buf.st_mode & S_IFREG, S_IFREG); } TEST_F(FileSystemS3Fixture, NestedDir) { - // TODO: uncommenting the line below will trigger the bug described in issue - // #63. Enable it once a fix for that is merged. - // WritePattern("/test/one.txt", 100'000, 'a', 32'768, true); + WritePattern("/test/one.txt", 100'000, 'a', 32'768, true); WritePattern("/test/one/two/statroot.txt", 100'000, 'a', 32'768, true); XrdSysLogger log; @@ -368,6 +375,31 @@ TEST_F(FileSystemS3Fixture, NestedDir) { ASSERT_EQ(buf.st_mode & S_IFDIR, S_IFDIR); } +TEST_F(FileSystemS3Fixture, InvalidObject) { + // Test various configurations of S3 buckets that lead + // to undefined situations in our filesystem-like translation, + // just to ensure we have our specified behavior. + XrdSysLogger log; + S3FileSystem fs(&log, m_configfn.c_str(), nullptr); + + // Object nested "inside" a directory. + WritePattern("/test/nested/foo", 1'024, 'a', 1'024, true); + WritePattern("/test/nested/foo/foo.txt", 1'024, 'a', 1'024, true); + + struct stat buf; + ASSERT_EQ(fs.Stat("/test/nested/foo", &buf, 0, nullptr), 0); + ASSERT_EQ(buf.st_mode & S_IFREG, S_IFREG); + ASSERT_EQ(buf.st_size, 1'024); + + ASSERT_EQ(fs.Stat("/test/nested/foo/foo.txt", &buf, 0, nullptr), 0); + ASSERT_EQ(buf.st_mode & S_IFREG, S_IFREG); + ASSERT_EQ(buf.st_size, 1'024); + + // Object with a trailing slash in name + WritePattern("/test/trailing/", 1'024, 'a', 1'024, true); + ASSERT_EQ(fs.Stat("/test/trailing/", &buf, 0, nullptr), -ENOENT); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();