Merge PR #2035: Reject out-of-range start in photo.rename and photo.renamePreview

crates/engine/src/cmd/manage.rs

@@ -9,8 +9,13 @@ use crate::Result;
 
 fn rename_args(s: &crate::Session, p: &Value, c: &str) -> Result<(Vec<lightcraft_catalog::PhotoId>, String, usize)> {
     let template = str_param(p, "template").ok_or_else(|| bad(c, "missing `template`"))?.to_string();
-    let start = p.get("start").and_then(Value::as_u64).unwrap_or(1) as usize;
-    Ok((s.targets(p)?, template, start))
+    let start = p.get("start").and_then(Value::as_u64).unwrap_or(1);
+    let ids = s.targets(p)?;
+    // every number of the sequence `start..start + n` must fit
+    let last = start.checked_add((ids.len() as u64).saturating_sub(1));
+    let start = last.and_then(|l| usize::try_from(l).ok()).and_then(|_| usize::try_from(start).ok());
+    let start = start.ok_or_else(|| bad(c, "start out of range"))?;
+    Ok((ids, template, start))
 }
 
 /// A named set of colour-label names (red, yellow, green, blue, purple; empty = the colour's name).

crates/engine/src/rename.rs

@@ -916,7 +916,8 @@ fn plan_rename_core(
 ) -> Vec<RenamePlan> {
     let mut taken: HashSet<String> = HashSet::new(); // lower-case target paths/names claimed in this batch
     let mut out = Vec::new();
-    for (seq, p) in (start..).zip(photos) {
+    for (i, p) in photos.iter().enumerate() {
+        let seq = start.saturating_add(i);
         let want = expand(template, p, seq);
         let (stem, ext) = split_ext(&want);
         let (stem, ext) = (stem.to_string(), ext.to_string());

crates/engine/src/tests_organize.rs

@@ -336,3 +336,23 @@ fn deleting_an_album_folder_preserves_photos_and_restores_its_subtree() {
     drop(reopened);
     let _ = std::fs::remove_dir_all(dir);
 }
+
+/// A huge `start` is a validation error, not an arithmetic overflow in the numbering.
+#[test]
+fn rename_huge_start_is_a_bad_param() {
+    let mut s = Session::with_demo();
+    let ids: Vec<u64> = s.catalog.photos().take(3).map(|p| p.id.0).collect();
+    let names_before: Vec<String> = s.catalog.photos().map(|p| p.file_name.clone()).collect();
+    for cmd in ["photo.renamePreview", "photo.rename"] {
+        let e = s.execute(cmd, &json!({"template": "{seq}", "start": u64::MAX, "ids": ids})).unwrap_err();
+        assert!(e.to_string().contains("start out of range"), "{cmd}: {e}");
+    }
+    let names_after: Vec<String> = s.catalog.photos().map(|p| p.file_name.clone()).collect();
+    assert_eq!(names_before, names_after, "a rejected rename changes nothing");
+    // the last start whose whole sequence fits is accepted, one more is not
+    let fits = usize::MAX as u64 - 2;
+    let r = s.execute("photo.renamePreview", &json!({"template": "{seq}", "start": fits, "ids": ids}));
+    assert!(r.is_ok(), "{r:?}");
+    let r = s.execute("photo.renamePreview", &json!({"template": "{seq}", "start": fits + 1, "ids": ids}));
+    assert!(r.is_err(), "{r:?}");
+}