From 4bbeaaa560c4f880b3f8b5a60cccd0ef68c52f1b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 10:18:31 -0700 Subject: [PATCH] LR-EXP-LOCATION: an export subfolder never leaves the destination folder (#2571) The subfolder kept `/` and `..` from command parameters, and the Export dialog's field and saved presets were not cleaned at all, so `../..` wrote outside the chosen folder. clean_subfolder turns `\`, `:` and NUL into `_` and drops empty, `.` and `..` parts; it is applied in Placer::folder_for, which every export goes through, and to the parameter. Nested subfolders still work. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01K93HTyCiwYvbRGaAkjPvXn --- crates/engine/src/export.rs | 13 ++++++++++-- crates/engine/src/tests_export.rs | 34 +++++++++++++++++++++++++++++++ docs/whats-new.md | 8 ++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index bfeade59cf..bc6c875020 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -1102,7 +1102,7 @@ impl ExportOptions { naming: s("naming").map_or(d.naming, str::to_string), start_number: u("startNumber").map_or(d.start_number, |v| v.min(999_999) as u32), destination_mode: s("destinationMode").and_then(DestinationMode::parse).unwrap_or(d.destination_mode), - subfolder: s("subfolder").map_or(d.subfolder, |v| v.trim().replace(['\\', ':', '\0'], "_")), + subfolder: s("subfolder").map_or(d.subfolder, clean_subfolder), conflict: s("conflict").and_then(Conflict::parse).unwrap_or(d.conflict), tiff_compression: match s("tiffCompression").map(str::to_ascii_lowercase).as_deref() { Some("none") => TiffCompression::None, @@ -2241,6 +2241,14 @@ fn join_path(a: &str, b: &str) -> String { if a.is_empty() { b.to_string() } else { format!("{}/{b}", a.trim_end_matches('/')) } } +/// A subfolder that stays inside the destination folder (issue #2571): `\`, `:` and NUL become `_` +/// (so no drive, root or Windows separator), and empty, `.` and `..` parts are dropped; `a/b` still +/// nests. Applied where the folder is used, so presets and the Export dialog's field are covered too. +pub(crate) fn clean_subfolder(s: &str) -> String { + let s = s.trim().replace(['\\', ':', '\0'], "_"); + s.split('/').map(str::trim).filter(|p| !p.is_empty() && *p != "." && *p != "..").collect::>().join("/") +} + impl<'a> Placer<'a> { fn new( o: &'a ExportOptions, @@ -2270,7 +2278,8 @@ impl<'a> Placer<'a> { d.to_string() } }; - Ok(if self.o.subfolder.is_empty() { base } else { join_path(&base, &self.o.subfolder) }) + let sub = clean_subfolder(&self.o.subfolder); + Ok(if sub.is_empty() { base } else { join_path(&base, &sub) }) } /// Place photo `photo`'s export result (file `name`, its original in `source_dir`): write it diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index 4830490b5c..cc505d18f6 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -181,6 +181,40 @@ fn presets_conflicts_and_subfolders() { assert_eq!(written, ["out/Sub/LC01347.png"]); } +/// Issue #2571: a subfolder with `..`, a root or a drive wrote outside the chosen folder. It is +/// cleaned where the folder is used, so a preset or the Export dialog's field (which set it +/// without `from_json`) are covered as well as command parameters. +#[test] +fn a_subfolder_never_leaves_the_destination_folder() { + use crate::export::{Destination, export_batch}; + let mut s = Session::with_demo(); + let id = s.active().unwrap(); + let to = Destination { dir: "out".into(), exact: None }; + let written_to = |s: &mut Session, sub: &str| { + let o = ExportOptions { subfolder: sub.into(), naming: "x".into(), ..ExportOptions::from_json(&json!({"format": "png", "width": 16})) }; + let mut written: Vec = Vec::new(); + let mut write = |p: &str, _: &[u8]| { + written.push(p.to_string()); + Ok(()) + }; + export_batch(s, &[id], &o, &to, &mut write, &|_| false).unwrap(); + written + }; + for (sub, want) in [ + ("../../escaped", "out/escaped/x.png"), + ("..", "out/x.png"), + ("/etc", "out/etc/x.png"), + ("C:\\Windows", "out/C__Windows/x.png"), + ("a/./b/../c", "out/a/b/c/x.png"), + (" Client / Jobs ", "out/Client/Jobs/x.png"), + ] { + assert_eq!(written_to(&mut s, sub), [want], "{sub:?}"); + } + // command parameters get the same cleaning + let o = ExportOptions::from_json(&json!({"subfolder": "../up"})); + assert_eq!(o.subfolder, "up"); +} + #[test] fn prepared_exports_run_on_another_thread() { fn send(_: &T) {} diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2b..49dc6188e3 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,13 @@ # What's new in LightCraft +## 2026-10-22 + +### Export subfolders stay inside the export folder +- An export subfolder (typed in the Export dialog, saved in a preset or passed to `export`) can no longer lead + outside the chosen folder (issue #2571). `..` and `.` parts and a leading `/` are dropped, and `\` and `:` become + `_`, so a shared preset with `../..` in its subfolder writes inside your folder instead of elsewhere on disk. + Nested subfolders such as `Client/Jobs` still work. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges