Fix 12 medium-severity bugs across all crates

- Escape backslashes in Nautilus preset names preventing Python injection
- Fix tiled watermarks starting at (spacing,spacing) instead of (0,0)
- Fix text watermark width overestimation (1.0x to 0.6x multiplier)
- Fix output_dpi forcing re-encoding for metadata-only presets
- Fix AVIF/WebP compression detection comparing against wrong preset values
- Add shared batch_updating guard for Ctrl+A/Ctrl+Shift+A select actions
- Fix overwrite conflict check ignoring preserve_directory_structure
- Add changes_filename()/changes_extension() for smarter overwrite checks
- Fix watch folder hardcoding "Blog Photos" preset
- Fix undo dropping history for partially-trashed batches
- Fix skipped files inflating size statistics
- Make CLI watch config writes atomic
This commit is contained in:
2026-03-07 23:35:32 +02:00
parent 1a174d40a7
commit 7e5d19ab03
8 changed files with 77 additions and 30 deletions

View File

@@ -573,25 +573,27 @@ fn cmd_undo(count: usize) {
entry.total, entry.input_dir entry.total, entry.input_dir
); );
let mut batch_trashed = 0; let mut remaining_files = Vec::new();
for file_path in &entry.output_files { for file_path in &entry.output_files {
let path = PathBuf::from(file_path); let path = PathBuf::from(file_path);
if path.exists() { if path.exists() {
match trash::delete(&path) { match trash::delete(&path) {
Ok(()) => { Ok(()) => {
batch_trashed += 1;
total_trashed += 1; total_trashed += 1;
} }
Err(e) => { Err(e) => {
eprintln!(" Failed to trash {}: {}", path.display(), e); eprintln!(" Failed to trash {}: {}", path.display(), e);
remaining_files.push(file_path.clone());
} }
} }
} }
} }
// Keep entry in history if no files were trashed // Keep entry with remaining files if some could not be trashed
if batch_trashed == 0 { if !remaining_files.is_empty() {
failed_entries.push(entry); let mut kept = entry;
kept.output_files = remaining_files;
failed_entries.push(kept);
} }
} }
@@ -665,7 +667,7 @@ fn cmd_watch_add(path: &str, preset_name: &str, recursive: bool) {
} }
match serde_json::to_string_pretty(&watches) { match serde_json::to_string_pretty(&watches) {
Ok(json) => { Ok(json) => {
if let Err(e) = std::fs::write(&watches_path, json) { if let Err(e) = pixstrip_core::storage::atomic_write(&watches_path, &json) {
eprintln!("Failed to write watch config: {}", e); eprintln!("Failed to write watch config: {}", e);
std::process::exit(1); std::process::exit(1);
} }
@@ -743,7 +745,7 @@ fn cmd_watch_remove(path: &str) {
match serde_json::to_string_pretty(&watches) { match serde_json::to_string_pretty(&watches) {
Ok(json) => { Ok(json) => {
if let Err(e) = std::fs::write(&watches_path, json) { if let Err(e) = pixstrip_core::storage::atomic_write(&watches_path, &json) {
eprintln!("Failed to write watch config: {}", e); eprintln!("Failed to write watch config: {}", e);
std::process::exit(1); std::process::exit(1);
} }

View File

@@ -398,7 +398,7 @@ impl PipelineExecutor {
let output_path = match job.overwrite_behavior { let output_path = match job.overwrite_behavior {
crate::operations::OverwriteAction::Skip if output_path.exists() => { crate::operations::OverwriteAction::Skip if output_path.exists() => {
return Ok((input_size, 0, std::path::PathBuf::new())); return Ok((0, 0, std::path::PathBuf::new()));
} }
crate::operations::OverwriteAction::AutoRename if output_path.exists() => { crate::operations::OverwriteAction::AutoRename if output_path.exists() => {
find_unique_path(&output_path) find_unique_path(&output_path)
@@ -563,7 +563,7 @@ impl PipelineExecutor {
crate::operations::OverwriteAction::Skip => { crate::operations::OverwriteAction::Skip => {
if output_path.exists() { if output_path.exists() {
// Return 0 bytes written - file was skipped // Return 0 bytes written - file was skipped
return Ok((input_size, 0, std::path::PathBuf::new())); return Ok((0, 0, std::path::PathBuf::new()));
} }
output_path output_path
} }

View File

@@ -151,8 +151,8 @@ fn install_nautilus() -> Result<()> {
\x20 item.connect('activate', self._on_preset, '{}', files)\n\ \x20 item.connect('activate', self._on_preset, '{}', files)\n\
\x20 submenu.append_item(item)\n\n", \x20 submenu.append_item(item)\n\n",
name.replace(' ', "_"), name.replace(' ', "_"),
name.replace('\'', "\\'"), name.replace('\\', "\\\\").replace('\'', "\\'"),
name.replace('\'', "\\'"), name.replace('\\', "\\\\").replace('\'', "\\'"),
)); ));
} }

View File

@@ -120,6 +120,11 @@ impl ConvertConfig {
} }
} }
} }
/// Returns true if this conversion will change at least some file extensions.
pub fn changes_extension(&self) -> bool {
!matches!(self, Self::KeepOriginal)
}
} }
// --- Compress --- // --- Compress ---
@@ -318,6 +323,18 @@ pub struct RenameConfig {
fn default_counter_position() -> u32 { 3 } fn default_counter_position() -> u32 { 3 }
impl RenameConfig { impl RenameConfig {
/// Returns true if this rename config would actually change any filename.
pub fn changes_filename(&self) -> bool {
!self.prefix.is_empty()
|| !self.suffix.is_empty()
|| self.counter_enabled
|| !self.regex_find.is_empty()
|| self.case_mode > 0
|| self.replace_spaces > 0
|| self.special_chars > 0
|| self.template.is_some()
}
/// Pre-compile the regex for batch use. Call once before a loop of apply_simple_compiled. /// Pre-compile the regex for batch use. Call once before a loop of apply_simple_compiled.
pub fn compile_regex(&self) -> Option<regex::Regex> { pub fn compile_regex(&self) -> Option<regex::Regex> {
rename::compile_rename_regex(&self.regex_find) rename::compile_rename_regex(&self.regex_find)

View File

@@ -198,7 +198,7 @@ fn render_text_to_image(
opacity: f32, opacity: f32,
) -> image::RgbaImage { ) -> image::RgbaImage {
let scale = ab_glyph::PxScale::from(font_size); let scale = ab_glyph::PxScale::from(font_size);
let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 1.0) as u32).saturating_add(4).min(16384); let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as u32).saturating_add(4).min(16384);
let text_height = ((font_size.min(1000.0) * 1.4) as u32).saturating_add(4).min(4096); let text_height = ((font_size.min(1000.0) * 1.4) as u32).saturating_add(4).min(4096);
let alpha = (opacity * 255.0).clamp(0.0, 255.0) as u8; let alpha = (opacity * 255.0).clamp(0.0, 255.0) as u8;
@@ -296,7 +296,7 @@ fn apply_text_watermark(
} else { } else {
// No rotation - draw text directly (faster) // No rotation - draw text directly (faster)
let scale = ab_glyph::PxScale::from(font_size); let scale = ab_glyph::PxScale::from(font_size);
let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 1.0) as u32).saturating_add(4).min(16384); let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as u32).saturating_add(4).min(16384);
let text_height = ((font_size.min(1000.0) * 1.4) as u32).saturating_add(4).min(4096); let text_height = ((font_size.min(1000.0) * 1.4) as u32).saturating_add(4).min(4096);
let text_dims = Dimensions { let text_dims = Dimensions {
width: text_width, width: text_width,
@@ -351,9 +351,9 @@ fn apply_tiled_text_watermark(
let tw = tile.width(); let tw = tile.width();
let th = tile.height(); let th = tile.height();
let mut y: i64 = spacing as i64; let mut y: i64 = 0;
while y < ih as i64 { while y < ih as i64 {
let mut x: i64 = spacing as i64; let mut x: i64 = 0;
while x < iw as i64 { while x < iw as i64 {
image::imageops::overlay(&mut rgba, &tile, x, y); image::imageops::overlay(&mut rgba, &tile, x, y);
x += tw as i64 + spacing as i64; x += tw as i64 + spacing as i64;
@@ -366,12 +366,12 @@ fn apply_tiled_text_watermark(
let alpha = (opacity * 255.0).clamp(0.0, 255.0) as u8; let alpha = (opacity * 255.0).clamp(0.0, 255.0) as u8;
let draw_color = Rgba([color[0], color[1], color[2], alpha]); let draw_color = Rgba([color[0], color[1], color[2], alpha]);
let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 1.0) as i64 + 4).min(16384); let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as i64 + 4).min(16384);
let text_height = ((font_size.min(1000.0) * 1.4) as i64 + 4).min(4096); let text_height = ((font_size.min(1000.0) * 1.4) as i64 + 4).min(4096);
let mut y = spacing as i64; let mut y: i64 = 0;
while y < ih as i64 { while y < ih as i64 {
let mut x = spacing as i64; let mut x: i64 = 0;
while x < iw as i64 { while x < iw as i64 {
draw_text_mut(&mut rgba, draw_color, x as i32, y as i32, scale, &font, text); draw_text_mut(&mut rgba, draw_color, x as i32, y as i32, scale, &font, text);
x += text_width + spacing as i64; x += text_width + spacing as i64;
@@ -413,9 +413,9 @@ fn apply_tiled_image_watermark(
let mut base = img.into_rgba8(); let mut base = img.into_rgba8();
let (iw, ih) = (base.width(), base.height()); let (iw, ih) = (base.width(), base.height());
let mut ty = spacing; let mut ty = 0u32;
while ty < ih { while ty < ih {
let mut tx = spacing; let mut tx = 0u32;
while tx < iw { while tx < iw {
for oy in 0..oh { for oy in 0..oh {
for ox in 0..ow { for ox in 0..ow {

View File

@@ -67,7 +67,7 @@ impl Preset {
crate::operations::CompressConfig::Preset(p) => p.avif_speed(), crate::operations::CompressConfig::Preset(p) => p.avif_speed(),
_ => 6, _ => 6,
}).unwrap_or(6), }).unwrap_or(6),
output_dpi: 72, output_dpi: 0,
} }
} }

View File

@@ -115,6 +115,8 @@ pub struct AppState {
pub detailed_mode: bool, pub detailed_mode: bool,
pub batch_queue: Rc<RefCell<BatchQueue>>, pub batch_queue: Rc<RefCell<BatchQueue>>,
pub expanded_sections: Rc<RefCell<std::collections::HashMap<String, bool>>>, pub expanded_sections: Rc<RefCell<std::collections::HashMap<String, bool>>>,
/// Guard flag to suppress per-checkbox callbacks during bulk select/deselect
pub batch_updating: Rc<std::cell::Cell<bool>>,
} }
impl AppState { impl AppState {
@@ -327,6 +329,7 @@ fn build_ui(app: &adw::Application) {
detailed_mode: app_cfg.skill_level.is_advanced(), detailed_mode: app_cfg.skill_level.is_advanced(),
batch_queue: Rc::new(RefCell::new(BatchQueue::default())), batch_queue: Rc::new(RefCell::new(BatchQueue::default())),
expanded_sections: Rc::new(RefCell::new(sess_state.expanded_sections.clone())), expanded_sections: Rc::new(RefCell::new(sess_state.expanded_sections.clone())),
batch_updating: Rc::new(std::cell::Cell::new(false)),
job_config: Rc::new(RefCell::new(JobConfig { job_config: Rc::new(RefCell::new(JobConfig {
preset_mode: false, preset_mode: false,
resize_enabled: if remember { sess_state.resize_enabled.unwrap_or(true) } else { true }, resize_enabled: if remember { sess_state.resize_enabled.unwrap_or(true) } else { true },
@@ -942,7 +945,9 @@ fn setup_window_actions(window: &adw::ApplicationWindow, ui: &WizardUi) {
&& let Some(stack) = page.child().and_downcast::<gtk::Stack>() && let Some(stack) = page.child().and_downcast::<gtk::Stack>()
&& let Some(loaded) = stack.child_by_name("loaded") && let Some(loaded) = stack.child_by_name("loaded")
{ {
ui.state.batch_updating.set(true);
crate::steps::step_images::set_all_checkboxes_in(&loaded, true); crate::steps::step_images::set_all_checkboxes_in(&loaded, true);
ui.state.batch_updating.set(false);
let files = ui.state.loaded_files.borrow(); let files = ui.state.loaded_files.borrow();
let count = files.len(); let count = files.len();
drop(files); drop(files);
@@ -969,7 +974,9 @@ fn setup_window_actions(window: &adw::ApplicationWindow, ui: &WizardUi) {
&& let Some(stack) = page.child().and_downcast::<gtk::Stack>() && let Some(stack) = page.child().and_downcast::<gtk::Stack>()
&& let Some(loaded) = stack.child_by_name("loaded") && let Some(loaded) = stack.child_by_name("loaded")
{ {
ui.state.batch_updating.set(true);
crate::steps::step_images::set_all_checkboxes_in(&loaded, false); crate::steps::step_images::set_all_checkboxes_in(&loaded, false);
ui.state.batch_updating.set(false);
let files = ui.state.loaded_files.borrow(); let files = ui.state.loaded_files.borrow();
let count = files.len(); let count = files.len();
drop(files); drop(files);
@@ -1799,11 +1806,14 @@ fn run_processing(_window: &adw::ApplicationWindow, ui: &WizardUi) {
// Check if user has customized per-format quality values beyond the preset defaults // Check if user has customized per-format quality values beyond the preset defaults
let preset_jpeg = cfg.quality_preset.jpeg_quality(); let preset_jpeg = cfg.quality_preset.jpeg_quality();
let preset_webp = cfg.quality_preset.webp_quality(); let preset_webp = cfg.quality_preset.webp_quality();
let preset_avif = cfg.quality_preset.avif_quality();
let preset_avif_speed = cfg.quality_preset.avif_speed();
let preset_webp_effort = cfg.quality_preset.webp_effort();
let has_custom = cfg.jpeg_quality != preset_jpeg let has_custom = cfg.jpeg_quality != preset_jpeg
|| cfg.webp_quality != preset_webp as u8 || cfg.webp_quality != preset_webp as u8
|| cfg.avif_quality != preset_webp as u8 || cfg.avif_quality != preset_avif
|| cfg.avif_speed != 6 || cfg.avif_speed != preset_avif_speed
|| cfg.webp_effort != 4; || cfg.webp_effort != preset_webp_effort;
if has_custom { if has_custom {
job.compress = Some(pixstrip_core::operations::CompressConfig::Custom { job.compress = Some(pixstrip_core::operations::CompressConfig::Custom {
@@ -1967,8 +1977,9 @@ fn run_processing(_window: &adw::ApplicationWindow, ui: &WizardUi) {
} }
// Check for existing output files when "Ask" overwrite behavior is set. // Check for existing output files when "Ask" overwrite behavior is set.
// Skip check if rename or format conversion is active (output names will differ). // Skip check if rename or format conversion will actually change output names.
let has_rename_or_convert = job.rename.is_some() || job.convert.is_some(); let has_rename_or_convert = job.rename.as_ref().is_some_and(|r| r.changes_filename())
|| job.convert.as_ref().is_some_and(|c| c.changes_extension());
if ask_overwrite && !has_rename_or_convert { if ask_overwrite && !has_rename_or_convert {
let output_dir = ui.state.output_dir.borrow().clone() let output_dir = ui.state.output_dir.borrow().clone()
.unwrap_or_else(|| { .unwrap_or_else(|| {
@@ -1980,7 +1991,19 @@ fn run_processing(_window: &adw::ApplicationWindow, ui: &WizardUi) {
let conflicts: Vec<String> = files.iter() let conflicts: Vec<String> = files.iter()
.filter_map(|f| { .filter_map(|f| {
let name = f.file_name()?; let name = f.file_name()?;
let out_path = output_dir.join(name); let out_path = if job.preserve_directory_structure {
if let Ok(rel) = f.strip_prefix(&job.input_dir) {
if let Some(parent) = rel.parent() {
output_dir.join(parent).join(name)
} else {
output_dir.join(name)
}
} else {
output_dir.join(name)
}
} else {
output_dir.join(name)
};
if out_path.exists() { Some(name.to_string_lossy().into()) } else { None } if out_path.exists() { Some(name.to_string_lossy().into()) } else { None }
}) })
.take(10) .take(10)
@@ -3464,9 +3487,14 @@ fn build_watch_folder_panel() -> gtk::Box {
if let Ok(file) = result if let Ok(file) = result
&& let Some(path) = file.path() && let Some(path) = file.path()
{ {
// Use first available preset name
let default_preset = pixstrip_core::preset::Preset::all_builtins()
.first()
.map(|p| p.name.clone())
.unwrap_or_else(|| "Blog Photos".to_string());
let new_folder = pixstrip_core::watcher::WatchFolder { let new_folder = pixstrip_core::watcher::WatchFolder {
path: path.clone(), path: path.clone(),
preset_name: "Blog Photos".to_string(), preset_name: default_preset,
recursive: false, recursive: false,
active: true, active: true,
}; };

View File

@@ -550,8 +550,8 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
.spacing(0) .spacing(0)
.build(); .build();
// Guard flag to prevent O(n^2) update cascade during batch checkbox operations // Use the shared batch_updating flag from AppState
let batch_updating: Rc<std::cell::Cell<bool>> = Rc::new(std::cell::Cell::new(false)); let batch_updating = state.batch_updating.clone();
// Toolbar // Toolbar
let toolbar = gtk::Box::builder() let toolbar = gtk::Box::builder()