fix list objects ordering to follow AWS S3

Signed-off-by: Utkarsh Srivastava <[email protected]>

src/sdk/namespace_fs.js

@@ -18,6 +18,7 @@ const stream_utils = require('../util/stream_utils');
 const buffer_utils = require('../util/buffer_utils');
 const size_utils = require('../util/size_utils');
 const native_fs_utils = require('../util/native_fs_utils');
+const js_utils = require('../util/js_utils');
 const ChunkFS = require('../util/chunk_fs');
 const LRUCache = require('../util/lru_cache');
 const nb_native = require('../util/nb_native');
@@ -91,14 +92,14 @@ const XATTR_METADATA_IGNORE_LIST = [
 ];
 
 /**
+ * sort_entries_by_name uses byte order instead of
+ * UTF-8 ordering to determine sort order.
  * @param {fs.Dirent} a
  * @param {fs.Dirent} b
  * @returns {1|-1|0}
  */
 function sort_entries_by_name(a, b) {
-    if (a.name < b.name) return -1;
-    if (a.name > b.name) return 1;
-    return 0;
+    return Buffer.compare(Buffer.from(a.name, 'utf8'), Buffer.from(b.name, 'utf8'));
 }
 
 function _get_version_id_by_stat({ino, mtimeNsBigint}) {
@@ -685,9 +686,10 @@ class NamespaceFS {
                     // Since versions are arranged next to latest object in the latest first order,
                     // no need to find the sorted last index. Push the ".versions/#VERSION_OBJECT" as
                     // they are in order
-                    if (results.length && r.key < results[results.length - 1].key &&
+                    const r_key_buf = Buffer.from(r.key, 'utf8');
+                    if (results.length && Buffer.compare(r_key_buf, Buffer.from(results[results.length - 1].key, 'utf8')) === -1 &&
                         !this._is_hidden_version_path(r.key)) {
-                        pos = _.sortedLastIndexBy(results, r, a => a.key);
+                        pos = js_utils.sortedLastIndexBy(results, curr => Buffer.from(curr.key, 'utf8').compare(r_key_buf) === -1);
                     } else {
                         pos = results.length;
                     }

src/test/unit_tests/jest_tests/js_utils.test.js

@@ -0,0 +1,79 @@
+/* Copyright (C) 2024 NooBaa */
+'use strict';
+
+const { sortedLastIndexBy } = require("../../../util/js_utils");
+
+describe('test js_utils.js', () => {
+    describe('sortedLastIndexBy', () => {
+        it('should correctly find position for number array', () => {
+            const test_table = [{
+                array: [1, 3, 4, 5],
+                target: 0,
+                expected_position: 0,
+            }, {
+                array: [1, 3, 4, 5],
+                target: 2,
+                expected_position: 1,
+            }, {
+                array: [1, 3, 4, 5],
+                target: 6,
+                expected_position: 4,
+            }];
+
+            for (const entry of test_table) {
+                expect(sortedLastIndexBy(entry.array, curr => curr < entry.target)).toBe(entry.expected_position);
+            }
+        });
+
+        it('should correctly find position for string array', () => {
+            const test_table = [{
+                array: ["a", "b", "c", "d"],
+                target: "A",
+                expected_position: 0,
+            }, {
+                array: ["a", "b", "d", "e"],
+                target: "c",
+                expected_position: 2,
+            }, {
+                array: ["a", "b", "c", "d"],
+                target: "z",
+                expected_position: 4,
+            }];
+
+            for (const entry of test_table) {
+                expect(sortedLastIndexBy(entry.array, curr => curr < entry.target)).toBe(entry.expected_position);
+            }
+        });
+
+        it('should correctly find position for utf8 string (buffer) array', () => {
+            // Editors might not render characters in this array properly but that's not a mistake,
+            // it's intentional to use such characters - sourced from:
+            // https://forum.moonwalkinc.com/t/determining-s3-listing-order/116
+            const array = ["file_ꦏ_1.txt", "file__2.txt", "file__3.txt", "file_𐎣_4.txt"];
+            const array_of_bufs = array.map(item => Buffer.from(item, 'utf8'));
+
+            const test_table = [{
+                array: array_of_bufs,
+                target: array_of_bufs[0],
+                expected_position: 0,
+            }, {
+                array: array_of_bufs,
+                target: array_of_bufs[2],
+                expected_position: 2,
+            }, {
+                array: array_of_bufs,
+                target: array_of_bufs[3],
+                expected_position: 3,
+            }, {
+                array: array_of_bufs,
+                target: Buffer.from("file_𐎣_40.txt", 'utf8'),
+                expected_position: 4,
+            }];
+
+            for (const entry of test_table) {
+                expect(sortedLastIndexBy(entry.array, curr => curr.compare(entry.target) === -1)).toBe(entry.expected_position);
+            }
+        });
+    });
+});
+

src/util/js_utils.js

@@ -237,6 +237,30 @@ function omit_symbol(maybe_obj, sym) {
     return _.omit(obj, sym);
 }
 
+/**
+ * sortedLastIndexBy is like lodash's sortedLastIndexBy except that
+ * instead of taking a iteratee, it takes a function `less` which should
+ * return `true` when `curr < value`.
+ * @param {Array<any>} array 
+ * @param {(curr: any) => Boolean} less 
+ * @returns 
+ */
+function sortedLastIndexBy(array, less) {
+    let low = 0;
+    let high = array === null ? low : array.length;
+
+    while (low < high) {
+        const mid = Math.floor(low + ((high - low) / 2));
+        if (less(array[mid])) {
+            low = mid + 1;
+        } else {
+            high = mid;
+        }
+    }
+
+    return high;
+}
+
 exports.self_bind = self_bind;
 exports.array_push_all = array_push_all;
 exports.array_push_keep_latest = array_push_keep_latest;
@@ -250,3 +274,4 @@ exports.inspect_lazy = inspect_lazy;
 exports.make_array = make_array;
 exports.map_get_or_create = map_get_or_create;
 exports.omit_symbol = omit_symbol;
+exports.sortedLastIndexBy = sortedLastIndexBy;