fix(filemeta): guard get_idx bound and fix sorts_before tie-break (#4509)

crates/filemeta/src/filemeta.rs

@@ -166,7 +166,7 @@ impl FileMeta {
     }
 
     fn get_idx(&self, idx: usize) -> Result<FileMetaVersion> {
-        if idx > self.versions.len() {
+        if idx >= self.versions.len() {
             return Err(Error::FileNotFound);
         }
 
@@ -1140,6 +1140,33 @@ mod test {
         assert_eq!(fm, newfm)
     }
 
+    #[test]
+    fn test_get_idx_out_of_bounds_returns_error_without_panic() {
+        let mut fm = FileMeta::new();
+
+        let (m, n) = (3, 2);
+        for i in 0..3 {
+            let mut fi = FileInfo::new(i.to_string().as_str(), m, n);
+            fi.version_id = Some(Uuid::from_u128(i + 1));
+            fi.mod_time = Some(OffsetDateTime::now_utc());
+            fm.add_version(fi).unwrap();
+        }
+
+        let len = fm.versions.len();
+        assert_eq!(len, 3);
+
+        // In-bounds indices resolve.
+        assert!(fm.get_idx(0).is_ok());
+        assert!(fm.get_idx(len - 1).is_ok());
+
+        // idx == len must return FileNotFound rather than panic on the
+        // out-of-bounds slice index, matching set_idx's guard.
+        match fm.get_idx(len) {
+            Err(Error::FileNotFound) => {}
+            other => panic!("expected FileNotFound for idx == len, got {other:?}"),
+        }
+    }
+
     #[test]
     fn test_marshal_metaobject() {
         let obj = MetaObject {

crates/filemeta/src/filemeta/version.rs

@@ -904,7 +904,13 @@ impl FileMetaVersionHeader {
             return self.mod_time > o.mod_time;
         }
 
-        match self.mod_time.cmp(&o.mod_time) {
+        // The following doesn't make too much sense, but we want sort to be consistent nonetheless.
+        // Prefer lower types
+        if self.version_type != o.version_type {
+            return self.version_type < o.version_type;
+        }
+        // Consistent sort on signature
+        match self.signature.cmp(&o.signature) {
             Ordering::Greater => {
                 return true;
             }
@@ -913,13 +919,7 @@ impl FileMetaVersionHeader {
             }
             _ => {}
         }
-
+        // Consistent sort on version_id
-        // The following doesn't make too much sense, but we want sort to be consistent nonetheless.
-        // Prefer lower types
-        if self.version_type != o.version_type {
-            return self.version_type < o.version_type;
-        }
-        // Consistent sort on signature
         match self.version_id.cmp(&o.version_id) {
             Ordering::Greater => {
                 return true;
@@ -3949,6 +3949,49 @@ mod tests {
         assert!(!a.sorts_before(&a.clone()));
     }
 
+    #[test]
+    fn version_header_sorts_before_breaks_signature_tie_before_version_id() {
+        // Two same-type Object headers with equal mod_time but differing
+        // signature and version_id. Matching MinIO's sortsBefore, the higher
+        // signature wins the tie ahead of the version_id comparison, even when
+        // version_id would order the two the other way.
+        let lower_sig_higher_id = FileMetaVersionHeader {
+            version_id: Some(Uuid::from_u128(2)),
+            mod_time: Some(sample_mod_time()),
+            version_type: VersionType::Object,
+            signature: [0x00, 0x00, 0x00, 0x01],
+            ..Default::default()
+        };
+        let higher_sig_lower_id = FileMetaVersionHeader {
+            version_id: Some(Uuid::from_u128(1)),
+            signature: [0x00, 0x00, 0x00, 0x02],
+            ..lower_sig_higher_id.clone()
+        };
+
+        assert!(higher_sig_lower_id.sorts_before(&lower_sig_higher_id));
+        assert!(!lower_sig_higher_id.sorts_before(&higher_sig_lower_id));
+    }
+
+    #[test]
+    fn version_header_sorts_before_version_id_only_breaks_signature_tie() {
+        // Equal mod_time, version_type, and signature: only then does version_id
+        // decide, and the higher version_id sorts first.
+        let a = FileMetaVersionHeader {
+            version_id: Some(Uuid::from_u128(1)),
+            mod_time: Some(sample_mod_time()),
+            version_type: VersionType::Object,
+            signature: [0x0a, 0x0b, 0x0c, 0x0d],
+            ..Default::default()
+        };
+        let b = FileMetaVersionHeader {
+            version_id: Some(Uuid::from_u128(2)),
+            ..a.clone()
+        };
+
+        assert!(b.sorts_before(&a));
+        assert!(!a.sorts_before(&b));
+    }
+
     #[test]
     fn merge_file_meta_versions_survives_garbage_header_stream() {
         let valid = FileMetaShallowVersion {