[bug] S3 provider should limit max key size to 1024 #31

CHANGELOG.md

@@ -11,6 +11,7 @@
 * ~~\#28 \[bug\] Address slow directory responses in S3 mounts for deep nested directories~~
 * \#29 \[bug\] S3 error responses are not being logged
 * ~~\#30 \[bug\] Sia json error responses are not logged~~
+* ~~\#31 \[bug\] S3 provider should limit max key size to 1024~~
 
 ### Changes from v2.0.2-rc
 

repertory/librepertory/include/types/repertory.hpp

@@ -200,6 +200,7 @@ enum class api_error {
   item_exists,
   item_not_found,
   more_data,
+  name_too_long,
   no_disk_space,
   not_implemented,
   not_supported,
@@ -216,31 +217,33 @@ enum class api_error {
 
 [[nodiscard]] auto api_error_from_string(std::string_view str) -> api_error;
 
-[[nodiscard]] auto
-api_error_to_string(const api_error &error) -> const std::string &;
+[[nodiscard]] auto api_error_to_string(const api_error &error)
+    -> const std::string &;
 
 enum class database_type {
   rocksdb,
   sqlite,
 };
-[[nodiscard]] auto database_type_from_string(
-    std::string type,
-    database_type default_type = database_type::rocksdb) -> database_type;
-
 [[nodiscard]] auto
-database_type_to_string(const database_type &type) -> std::string;
+database_type_from_string(std::string type,
+                          database_type default_type = database_type::rocksdb)
+    -> database_type;
+
+[[nodiscard]] auto database_type_to_string(const database_type &type)
+    -> std::string;
 
 enum class download_type {
   default_,
   direct,
   ring_buffer,
 };
-[[nodiscard]] auto download_type_from_string(
-    std::string type,
-    download_type default_type = download_type::default_) -> download_type;
-
 [[nodiscard]] auto
-download_type_to_string(const download_type &type) -> std::string;
+download_type_from_string(std::string type,
+                          download_type default_type = download_type::default_)
+    -> download_type;
+
+[[nodiscard]] auto download_type_to_string(const download_type &type)
+    -> std::string;
 
 enum class exit_code : std::int32_t {
   success = 0,

repertory/librepertory/src/providers/s3/s3_provider.cpp

@@ -39,6 +39,19 @@
 #include "utils/string.hpp"
 #include "utils/time.hpp"
 
+namespace {
+[[nodiscard]] auto set_request_path(auto &request, const auto &cfg,
+                                    const std::string &object_name)
+    -> repertory::api_error {
+  request.path = object_name;
+  if ((cfg.bucket + request.path).subst(0U).size() > 1024U) {
+    return repertory::api_error::name_too_long;
+  }
+
+  return repertory::api_error::success;
+}
+} // namespace
+
 namespace repertory {
 s3_provider::s3_provider(app_config &config, i_http_comm &comm)
     : base_provider(config, comm) {}
@@ -118,12 +131,17 @@ auto s3_provider::create_directory_impl(const std::string &api_path,
   curl::requests::http_put_file put_file{};
   put_file.allow_timeout = true;
   put_file.aws_service = "aws:amz:" + cfg.region + ":s3";
-  put_file.path = object_name + '/';
   put_file.response_handler = [&response_data](auto &&data,
+
                                                long /*response_code*/) {
     response_data = std::string(data.begin(), data.end());
   };
 
+  auto res = set_request_path(put_file, cfg, object_name + '/');
+  if (res != api_error::success) {
+    return res;
+  }
+
   long response_code{};
   if (not get_comm().make_request(put_file, response_code, stop_requested)) {
     utils::error::raise_api_path_error(function_name, api_path,
@@ -221,12 +239,17 @@ auto s3_provider::create_path_directories(const std::string &api_path,
         curl::requests::http_put_file put_file{};
         put_file.allow_timeout = true;
         put_file.aws_service = "aws:amz:" + cfg.region + ":s3";
-        put_file.path = (is_encrypted ? cur_key : cur_path) + '/';
         put_file.response_handler = [&response_data](auto &&data,
                                                      long /*response_code*/) {
           response_data = std::string(data.begin(), data.end());
         };
 
+        res = set_request_path(put_file, cfg,
+                               (is_encrypted ? cur_key : cur_path) + '/');
+        if (res != api_error::success) {
+          return res;
+        }
+
         stop_type stop_requested{false};
         long response_code{};
         if (not get_comm().make_request(put_file, response_code,
@@ -655,12 +678,16 @@ auto s3_provider::get_object_info(bool directory, const std::string &api_path,
     curl::requests::http_head head{};
     head.allow_timeout = true;
     head.aws_service = "aws:amz:" + cfg.region + ":s3";
-    head.path = directory ? object_name + '/' : object_name;
     head.response_headers = http_headers{};
     head.response_handler = [&response_data](auto &&data,
                                              long /*response_code*/) {
       response_data = std::string(data.begin(), data.end());
     };
+    res = set_request_path(put_file, cfg,
+                           directory ? object_name + '/' : object_name);
+    if (res != api_error::success) {
+      return res;
+    }
 
     stop_type stop_requested{false};
     long response_code{};
@@ -813,7 +840,6 @@ auto s3_provider::read_file_bytes(const std::string &api_path, std::size_t size,
         curl::requests::http_get get{};
         get.aws_service = "aws:amz:" + cfg.region + ":s3";
         get.headers["response-content-type"] = "binary/octet-stream";
-        get.path = object_name;
         get.range = {{
             read_offset,
             read_offset + read_size - 1U,
@@ -822,6 +848,10 @@ auto s3_provider::read_file_bytes(const std::string &api_path, std::size_t size,
                                               long /*response_code*/) {
           read_buffer = response_data;
         };
+        res = set_request_path(get, cfg, object_name);
+        if (res != api_error::success) {
+          return res;
+        }
 
         long response_code{};
         const auto notify_retry = [&]() {
@@ -912,11 +942,15 @@ auto s3_provider::remove_directory_impl(const std::string &api_path)
   curl::requests::http_delete del{};
   del.allow_timeout = true;
   del.aws_service = "aws:amz:" + cfg.region + ":s3";
-  del.path = object_name + '/';
   del.response_handler = [&response_data](auto &&data, long /*response_code*/) {
     response_data = std::string(data.begin(), data.end());
   };
 
+  auto res = set_request_path(del, cfg, object_name + '/');
+  if (res != api_error::success) {
+    return res;
+  }
+
   long response_code{};
   stop_type stop_requested{};
   if (not get_comm().make_request(del, response_code, stop_requested)) {
@@ -959,10 +993,13 @@ auto s3_provider::remove_file_impl(const std::string &api_path) -> api_error {
   curl::requests::http_delete del{};
   del.allow_timeout = true;
   del.aws_service = "aws:amz:" + cfg.region + ":s3";
-  del.path = object_name;
   del.response_handler = [&response_data](auto &&data, long /*response_code*/) {
     response_data = std::string(data.begin(), data.end());
   };
+  auto res = set_request_path(del, cfg, object_name);
+  if (res != api_error::success) {
+    return res;
+  }
 
   long response_code{};
   stop_type stop_requested{};
@@ -1036,13 +1073,17 @@ auto s3_provider::upload_file_impl(const std::string &api_path,
   std::string response_data;
   curl::requests::http_put_file put_file{};
   put_file.aws_service = "aws:amz:" + cfg.region + ":s3";
-  put_file.path = object_name;
   put_file.response_handler = [&response_data](auto &&data,
                                                long /*response_code*/) {
     response_data = std::string(data.begin(), data.end());
   };
   put_file.source_path = source_path;
 
+  auto res = set_request_path(put_file, cfg, object_name);
+  if (res != api_error::success) {
+    return res;
+  }
+
   if (is_encrypted && file_size > 0U) {
     static stop_type no_stop{false};
 

repertory/librepertory/src/types/repertory.cpp

@@ -25,8 +25,8 @@
 #include "utils/string.hpp"
 
 namespace repertory {
-auto database_type_from_string(std::string type,
-                               database_type default_type) -> database_type {
+auto database_type_from_string(std::string type, database_type default_type)
+    -> database_type {
   type = utils::string::to_lower(utils::string::trim(type));
   if (type == "rocksdb") {
     return database_type::rocksdb;
@@ -50,8 +50,8 @@ auto database_type_to_string(const database_type &type) -> std::string {
   }
 }
 
-auto download_type_from_string(std::string type,
-                               download_type default_type) -> download_type {
+auto download_type_from_string(std::string type, download_type default_type)
+    -> download_type {
   type = utils::string::to_lower(utils::string::trim(type));
   if (type == "default") {
     return download_type::default_;
@@ -112,6 +112,7 @@ static const std::unordered_map<api_error, std::string> LOOKUP = {
     {api_error::item_exists, "item_exists"},
     {api_error::item_not_found, "item_not_found"},
     {api_error::more_data, "more_data"},
+    {api_error::name_too_long, "name_too_long"},
     {api_error::no_disk_space, "no_disk_space"},
     {api_error::not_implemented, "not_implemented"},
     {api_error::not_supported, "not_supported"},

repertory/librepertory/src/utils/unix/unix_utils.cpp

@@ -58,6 +58,8 @@ auto from_api_error(const api_error &err) -> int {
     return -EINVAL;
   case api_error::item_not_found:
     return -ENOENT;
+  case api_error::name_too_long:
+    return -ENAMETOOLONG;
   case api_error::out_of_memory:
     return -ENOMEM;
   case api_error::no_disk_space:
@@ -128,6 +130,8 @@ auto to_api_error(int err) -> api_error {
     return api_error::file_in_use;
   case EINVAL:
     return api_error::invalid_operation;
+  case ENAMETOOLONG:
+    return api_error::name_too_long;
   case ENOENT:
     return api_error::item_not_found;
   case ENOMEM:

repertory/librepertory/src/utils/windows/windows_utils.cpp

@@ -65,6 +65,8 @@ auto from_api_error(const api_error &e) -> NTSTATUS {
     return STATUS_CLIENT_SERVER_PARAMETERS_INVALID;
   case api_error::item_not_found:
     return STATUS_OBJECT_NAME_NOT_FOUND;
+  case api_error::name_too_long:
+    return STATUS_NAME_TOO_LONG;
   case api_error::no_disk_space:
     return STATUS_DEVICE_INSUFFICIENT_RESOURCES;
   case api_error::os_error: