mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-28 03:57:57 -06:00
engine: refuse negative indices instead of targeting element 0
`version.*`, `pointColor.delete` and `redeye.catchlight`/`redeye.delete` parsed `index` with `f64_req(..)? as usize`. Rust's float-to-int cast saturates, so `index: -1` became 0 and the bounds checks never saw a negative: a caller sending `index: -1` silently deleted, restored or renamed version 0 (or the first point-colour sample / red-eye correction) and got `ok` back. `history.restore` already guards this and `spot_index` uses `as_u64`. New `index_req` helper in `cmd/mod.rs` (the same finite, non-negative guard) is used by the five sites. Behaviour for valid indices is unchanged. Regression test `negative_indices_are_refused` creates a version, then asserts `version.delete/restore/rename` with `index: -1` error and version 0 is untouched (before the fix the first one deletes it).
This commit is contained in:
@@ -5,7 +5,7 @@ use lightcraft_develop::{DevelopSettings, Preset, ProcessVersion, Section, Setti
|
||||
use lightcraft_geom::{CropGeometry, CropHandle, Point, Rect, crop_fit_angle, drag_crop};
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use super::{CommandSpec, always, bad, bool_or, cmd, f64_or, f64_req, has_active, has_clipboard, has_selection, ok, str_param};
|
||||
use super::{CommandSpec, always, bad, bool_or, cmd, f64_or, f64_req, has_active, has_clipboard, has_selection, index_req, ok, str_param};
|
||||
use crate::{Result, Session, media::SourceLevel};
|
||||
|
||||
fn active(s: &Session, c: &str) -> Result<PhotoId> {
|
||||
@@ -144,7 +144,7 @@ fn user_preset<'a>(s: &'a mut Session, p: &Value, c: &str) -> Result<&'a mut Pre
|
||||
/// Change version `p.index` of the active photo (`f` gets it and the current settings).
|
||||
fn edit_version(s: &mut Session, p: &Value, c: &str, label: &str, f: impl FnOnce(&mut Version, std::sync::Arc<DevelopSettings>)) -> Result<Value> {
|
||||
let id = active(s, c)?;
|
||||
let i = f64_req(p, "index", c)? as usize;
|
||||
let i = index_req(p, "index", c)?;
|
||||
let ph = s.catalog.photo(id).ok_or_else(|| bad(c, "no photo"))?;
|
||||
let mut versions = ph.versions.clone();
|
||||
let v = versions.get_mut(i).ok_or_else(|| bad(c, "no such version"))?;
|
||||
@@ -1060,14 +1060,14 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
}),
|
||||
cmd!("version.restore", "Restore Version", [], None, "{index}", has_active, |s, p| {
|
||||
let id = active(s, "version.restore")?;
|
||||
let i = f64_req(p, "index", "version.restore")? as usize;
|
||||
let i = index_req(p, "index", "version.restore")?;
|
||||
let v = s.catalog.photo(id).and_then(|ph| ph.versions.get(i).cloned()).ok_or_else(|| bad("version.restore", "no such version"))?;
|
||||
s.set_develop(id, (*v.settings).clone(), &format!("Restore {}", v.name))?;
|
||||
ok()
|
||||
}),
|
||||
cmd!("version.delete", "Delete Version", [], None, "{index}", has_active, |s, p| {
|
||||
let id = active(s, "version.delete")?;
|
||||
let i = f64_req(p, "index", "version.delete")? as usize;
|
||||
let i = index_req(p, "index", "version.delete")?;
|
||||
let mut versions = s.catalog.photo(id).map(|ph| ph.versions.clone()).unwrap_or_default();
|
||||
if i >= versions.len() {
|
||||
return Err(bad("version.delete", "no such version"));
|
||||
|
||||
Reference in New Issue
Block a user