rgw/sfs: Fix list-objects check for common-prefixes

Fixes a list-objects issue that happened when calculating if a query
should be truncated or not when having common prefixes.

The proposed code change is using the last valid entry (entry being
either the last object name or common prefix) as the next marker for
the call that checks if there are more items.

That way we are taking into account items that were already added to the
results and we're not repeating them. (Also the truncation is
calculated fine).

Fixes: https://github.com/aquarist-labs/s3gw/issues/838
Signed-off-by: Xavi Garcia <[email protected]>

qa/rgw/store/sfs/tests/test-sfs-list-objects.py

@@ -108,6 +108,7 @@ def check_list_response_prefix(self, list_resp, objects_expected, prefix, common
                 self.assertTrue(common_prefix["Prefix"] in common_prefixes)
         else:
             self.assertFalse("CommonPrefixes" in list_resp)
+        self.assertFalse(list_resp["IsTruncated"])
 
 
     def test_list_objects_no_prefix(self):
@@ -260,6 +261,29 @@ def test_list_objects_with_delimiter(self):
         ]
         self.check_list_response_prefix(objs_ret, expected_objects, 'directory/', expected_common_prefixes)
 
+    def test_list_objects_with_delimiter_object_name_is_greater(self):
+        bucket_name = self.get_random_bucket_name()
+        self.s3_client.create_bucket(Bucket=bucket_name)
+        self.assert_bucket_exists(bucket_name)
+        # objects to upload
+        objects = [
+            'b/test',
+            'test',
+            'test1',
+            'test2'
+        ]
+        for obj in objects:
+            self.upload_file_and_check(bucket_name, obj)
+        # list all objects with prefix equal to 'prefix'
+        objs_ret = self.s3_client.list_objects(Bucket=bucket_name, Prefix='', Delimiter = '/')
+        expected_objects = [
+            'test', 'test1', 'test2'
+        ]
+        expected_common_prefixes = [
+            'b/'
+        ]
+        self.check_list_response_prefix(objs_ret, expected_objects, '', expected_common_prefixes)
+
 if __name__ == '__main__':
     if len(sys.argv) == 2:
         address_port = sys.argv.pop()

src/rgw/driver/sfs/bucket.cc

@@ -244,7 +244,7 @@ int SFSBucket::list(
 
   if (!params.delim.empty()) {
     std::vector<rgw_bucket_dir_entry> new_results;
-    list.roll_up_common_prefixes(
+    auto next_marker = list.roll_up_common_prefixes(
         params.prefix, params.delim, results.objs, results.common_prefixes,
         new_results
     );
@@ -254,7 +254,7 @@ int SFSBucket::list(
     // anything after the prefix.
     if (!results.common_prefixes.empty()) {
       std::vector<rgw_bucket_dir_entry> objects_after;
-      std::string query = std::prev(results.common_prefixes.end())->first;
+      std::string query = next_marker;
       query.append(
           sfs::S3_MAX_OBJECT_NAME_BYTES - query.size(),
           std::numeric_limits<char>::max()

src/rgw/driver/sfs/sqlite/sqlite_list.cc

@@ -155,17 +155,18 @@ bool SQLiteList::versions(
   return true;
 }
 
-void SQLiteList::roll_up_common_prefixes(
+std::string SQLiteList::roll_up_common_prefixes(
     const std::string& find_after_prefix, const std::string& delimiter,
     const std::vector<rgw_bucket_dir_entry>& objects,
     std::map<std::string, bool>& out_common_prefixes,
     std::vector<rgw_bucket_dir_entry>& out_objects
 ) const {
   const size_t find_after_pos = find_after_prefix.length();
   const size_t delim_len = delimiter.length();
+  std::string next_marker = "";
   if (delimiter.empty()) {
     out_objects = objects;
-    return;
+    return "";
   }
   const std::string* prefix{nullptr};  // Last added prefix
   for (size_t i = 0; i < objects.size(); i++) {
@@ -181,12 +182,16 @@ void SQLiteList::roll_up_common_prefixes(
         prefix = &out_common_prefixes
                       .emplace(name.substr(0, delim_pos + delim_len), true)
                       .first->first;
+        next_marker = *prefix;
         continue;
       }
     }
     // Not found -> next
     out_objects.push_back(objects[i]);
+    next_marker = objects[i].key.name;
   }
+
+  return next_marker;
 }
 
 }  // namespace rgw::sal::sfs::sqlite

src/rgw/driver/sfs/sqlite/sqlite_list.h

@@ -52,7 +52,8 @@ class SQLiteList {
   // `delimiter` position.
   //
   // Copy all other `objects` to `out_objects`.
-  void roll_up_common_prefixes(
+  // Returns the next marker to be used in a subsequent call to list objects
+  std::string roll_up_common_prefixes(
       const std::string& find_after_prefix, const std::string& delimiter,
       const std::vector<rgw_bucket_dir_entry>& objects,
       std::map<std::string, bool>& out_common_prefixes,