diff --git a/crates/engine/src/export/params.rs b/crates/engine/src/export/params.rs index 795d8f3672..34397fad3c 100644 --- a/crates/engine/src/export/params.rs +++ b/crates/engine/src/export/params.rs @@ -1,4 +1,5 @@ use super::options::{Conflict, DestinationMode, ExportFormat, HdrCurve, MetadataPolicy, Resize, ResizeMode, SharpenAmount, SharpenFor}; +use super::place::clean_subfolder; use super::prepare::DngCompression; use super::watermark::{Anchor, Watermark}; use super::{ExportOptions, JxlColour, JxlEffort, JxlOptions, OutputDepth, OutputSpace, TiffCompression}; @@ -429,7 +430,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, diff --git a/crates/engine/src/export/place.rs b/crates/engine/src/export/place.rs index d223a3a791..c36f5e26bc 100644 --- a/crates/engine/src/export/place.rs +++ b/crates/engine/src/export/place.rs @@ -60,6 +60,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(super) 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> { pub(super) fn new( o: &'a ExportOptions, @@ -89,7 +97,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 27268e463a..531f1d652c 100644 --- a/crates/engine/src/tests/export.rs +++ b/crates/engine/src/tests/export.rs @@ -189,6 +189,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 31e08c6008..8d040d8892 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -362,6 +362,12 @@ ## 2026-10-23 +### 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. + ### Videos in the grid are no longer sent to the image decoders - A video in the library (an MP4 or MOV brought in from a Lightroom catalog) no longer asks the image decoders for a grid thumbnail (issue #2682). They can't read one, so each video failed with "unrecognised image format", showed a