diff --git a/pixstrip-cli/src/main.rs b/pixstrip-cli/src/main.rs index c7d69b1..7c1d1cc 100644 --- a/pixstrip-cli/src/main.rs +++ b/pixstrip-cli/src/main.rs @@ -419,37 +419,8 @@ fn cmd_process(args: CmdProcessArgs) { } } - // Save to history + // Save to history - use actual output paths from the executor let history = HistoryStore::new(); - let output_ext = match job.convert { - Some(ConvertConfig::SingleFormat(fmt)) => fmt.extension(), - _ => "", - }; - let output_files: Vec = source_files - .iter() - .enumerate() - .map(|(i, f)| { - let stem = f.file_stem().and_then(|s| s.to_str()).unwrap_or("file"); - let ext = if output_ext.is_empty() { - f.extension().and_then(|e| e.to_str()).unwrap_or("jpg") - } else { - output_ext - }; - let name = if let Some(ref rename) = job.rename { - if let Some(ref tmpl) = rename.template { - pixstrip_core::operations::rename::apply_template( - tmpl, stem, ext, rename.counter_start.saturating_add(i as u32), None, - ) - } else { - rename.apply_simple(stem, ext, (i as u32).saturating_add(1)) - } - } else { - format!("{}.{}", stem, ext) - }; - output_dir.join(name).to_string_lossy().into() - }) - .collect(); - if let Err(e) = history.add(pixstrip_core::storage::HistoryEntry { timestamp: chrono_timestamp(), input_dir: input_dir.canonicalize().unwrap_or_else(|_| input_dir.to_path_buf()).to_string_lossy().into(), @@ -461,7 +432,7 @@ fn cmd_process(args: CmdProcessArgs) { total_input_bytes: result.total_input_bytes, total_output_bytes: result.total_output_bytes, elapsed_ms: result.elapsed_ms, - output_files, + output_files: result.output_files, }, 50, 30) { eprintln!("Warning: failed to save history (undo may not work): {}", e); } @@ -777,6 +748,8 @@ fn cmd_watch_start() { let (tx, rx) = std::sync::mpsc::channel(); let mut watchers = Vec::new(); + // Collect output directories so we can skip files inside them (prevent infinite loop) + let mut output_dirs: Vec = Vec::new(); for watch in &active { let watcher = pixstrip_core::watcher::FolderWatcher::new(); @@ -784,19 +757,41 @@ fn cmd_watch_start() { eprintln!("Failed to start watching {}: {}", watch.path.display(), e); continue; } + output_dirs.push(watch.path.join("processed")); watchers.push((watcher, watch.preset_name.clone())); } + // Drop the original sender so the channel closes when all watcher threads exit + drop(tx); + + if watchers.is_empty() { + eprintln!("Failed to start any watchers. Check that watch folders exist and are accessible."); + return; + } + // Process incoming files for event in &rx { match event { pixstrip_core::watcher::WatchEvent::NewImage(path) => { + // Skip files inside output directories to prevent infinite processing loop + if output_dirs.iter().any(|d| path.starts_with(d)) { + continue; + } + println!("New image: {}", path.display()); + + // Wait briefly for file to be fully written + std::thread::sleep(std::time::Duration::from_millis(500)); + // Find which watcher owns this path and use its preset let matched = active.iter() .find(|w| path.starts_with(&w.path)) .map(|w| w.preset_name.clone()); - if let Some(preset_name) = matched.as_deref().or_else(|| watchers.first().map(|(_, n)| n.as_str())) { + if matched.is_none() { + eprintln!(" Warning: no matching watch folder for {}, skipping", path.display()); + continue; + } + if let Some(preset_name) = matched.as_deref() { let Some(preset) = find_preset(preset_name) else { eprintln!(" Preset '{}' not found, skipping", preset_name); continue; diff --git a/pixstrip-core/src/encoder.rs b/pixstrip-core/src/encoder.rs index 89e658d..a44e8a0 100644 --- a/pixstrip-core/src/encoder.rs +++ b/pixstrip-core/src/encoder.rs @@ -86,8 +86,8 @@ impl OutputEncoder { if self.options.output_dpi > 0 { comp.set_pixel_density(mozjpeg::PixelDensity { unit: mozjpeg::PixelDensityUnit::Inches, - x: self.options.output_dpi as u16, - y: self.options.output_dpi as u16, + x: self.options.output_dpi.min(65535) as u16, + y: self.options.output_dpi.min(65535) as u16, }); } diff --git a/pixstrip-core/src/executor.rs b/pixstrip-core/src/executor.rs index 109547c..b64bc49 100644 --- a/pixstrip-core/src/executor.rs +++ b/pixstrip-core/src/executor.rs @@ -33,6 +33,8 @@ pub struct BatchResult { pub total_output_bytes: u64, pub errors: Vec<(String, String)>, pub elapsed_ms: u64, + /// Actual output file paths written by the executor (only successfully written files). + pub output_files: Vec, } pub struct PipelineExecutor { @@ -98,6 +100,7 @@ impl PipelineExecutor { total_output_bytes: 0, errors: Vec::new(), elapsed_ms: 0, + output_files: Vec::new(), }); } @@ -122,6 +125,7 @@ impl PipelineExecutor { let output_bytes = AtomicU64::new(0); let cancelled = AtomicBool::new(false); let errors: Mutex> = Mutex::new(Vec::new()); + let written_files: Mutex> = Mutex::new(Vec::new()); // Channel for progress updates from worker threads to the callback let (tx, rx) = std::sync::mpsc::channel::(); @@ -141,6 +145,7 @@ impl PipelineExecutor { let output_bytes_ref = &output_bytes; let cancelled_ref = &cancelled; let errors_ref = &errors; + let written_ref = &written_files; let worker = scope.spawn(move || { pool.install(|| { @@ -189,10 +194,15 @@ impl PipelineExecutor { }); match Self::process_single_static(job, source, &loader, &encoder, idx) { - Ok((in_size, out_size)) => { + Ok((in_size, out_size, out_path)) => { succeeded_ref.fetch_add(1, Ordering::Relaxed); input_bytes_ref.fetch_add(in_size, Ordering::Relaxed); output_bytes_ref.fetch_add(out_size, Ordering::Relaxed); + if !out_path.as_os_str().is_empty() { + if let Ok(mut wf) = written_ref.lock() { + wf.push(out_path.display().to_string()); + } + } } Err(e) => { failed_ref.fetch_add(1, Ordering::Relaxed); @@ -231,6 +241,7 @@ impl PipelineExecutor { total_output_bytes: output_bytes.load(Ordering::Relaxed), errors: errors.into_inner().unwrap_or_default(), elapsed_ms: start.elapsed().as_millis() as u64, + output_files: written_files.into_inner().unwrap_or_default(), }) } @@ -256,6 +267,7 @@ impl PipelineExecutor { total_output_bytes: 0, errors: Vec::new(), elapsed_ms: 0, + output_files: Vec::new(), }; for (i, source) in job.sources.iter().enumerate() { @@ -291,10 +303,13 @@ impl PipelineExecutor { }); match Self::process_single_static(job, source, &loader, &encoder, i) { - Ok((input_size, output_size)) => { + Ok((input_size, output_size, out_path)) => { result.succeeded += 1; result.total_input_bytes += input_size; result.total_output_bytes += output_size; + if !out_path.as_os_str().is_empty() { + result.output_files.push(out_path.display().to_string()); + } } Err(e) => { result.failed += 1; @@ -316,14 +331,24 @@ impl PipelineExecutor { loader: &ImageLoader, encoder: &OutputEncoder, index: usize, - ) -> std::result::Result<(u64, u64), PixstripError> { + ) -> std::result::Result<(u64, u64, std::path::PathBuf), PixstripError> { let input_size = std::fs::metadata(&source.path) .map(|m| m.len()) .unwrap_or(0); + // Metadata stripping via little_exif only works for JPEG/TIFF. + // For other formats, force the pixel path so re-encoding strips metadata. + let metadata_needs_reencode = job.metadata.as_ref().is_some_and(|m| { + !matches!(m, crate::operations::MetadataConfig::KeepAll) + && !matches!( + source.original_format, + Some(ImageFormat::Jpeg) | Some(ImageFormat::Tiff) + ) + }); + // Fast path: if no pixel processing needed (rename-only or rename+metadata), // just copy the file instead of decoding/re-encoding. - if !job.needs_pixel_processing() { + if !job.needs_pixel_processing() && !metadata_needs_reencode { let output_path = if let Some(ref rename) = job.rename { let stem = source.path.file_stem().and_then(|s| s.to_str()).unwrap_or("output"); let ext = source.path.extension().and_then(|e| e.to_str()).unwrap_or("jpg"); @@ -333,8 +358,12 @@ impl PipelineExecutor { .ok() .and_then(|r| r.with_guessed_format().ok()) .and_then(|r| r.into_dimensions().ok()); + // Apply regex find/replace on stem (matching the pixel path) + let working_stem = crate::operations::rename::apply_regex_replace( + stem, &rename.regex_find, &rename.regex_replace, + ); let new_name = crate::operations::rename::apply_template_full( - template, stem, ext, + template, &working_stem, ext, rename.counter_start.saturating_add(index as u32), dims, Some(ext), Some(&source.path), None, ); @@ -348,18 +377,26 @@ impl PipelineExecutor { } else { new_name }; - job.output_dir.join(new_name) + job.output_dir.join(sanitize_filename(&new_name)) } else { let new_name = rename.apply_simple(stem, ext, (index as u32).saturating_add(1)); - job.output_dir.join(new_name) + job.output_dir.join(sanitize_filename(&new_name)) } } else { job.output_path_for(source, None) }; + // Prevent overwriting the source file + if paths_are_same(&source.path, &output_path) { + return Err(PixstripError::Processing { + operation: "output".into(), + reason: format!("output path is the same as source: {}", source.path.display()), + }); + } + let output_path = match job.overwrite_behavior { crate::operations::OverwriteAction::Skip if output_path.exists() => { - return Ok((input_size, 0)); + return Ok((input_size, 0, std::path::PathBuf::new())); } crate::operations::OverwriteAction::AutoRename if output_path.exists() => { find_unique_path(&output_path) @@ -368,10 +405,16 @@ impl PipelineExecutor { }; if let Some(parent) = output_path.parent() { - std::fs::create_dir_all(parent).map_err(PixstripError::Io)?; + std::fs::create_dir_all(parent).map_err(|e| { + cleanup_placeholder(&output_path); + PixstripError::Io(e) + })?; } - std::fs::copy(&source.path, &output_path).map_err(PixstripError::Io)?; + std::fs::copy(&source.path, &output_path).map_err(|e| { + cleanup_placeholder(&output_path); + PixstripError::Io(e) + })?; // Metadata handling on the copy if let Some(ref meta_config) = job.metadata { @@ -391,7 +434,7 @@ impl PipelineExecutor { } let output_size = std::fs::metadata(&output_path).map(|m| m.len()).unwrap_or(0); - return Ok((input_size, output_size)); + return Ok((input_size, output_size, output_path)); } // Load image @@ -494,21 +537,29 @@ impl PipelineExecutor { } else { new_name }; - job.output_dir.join(new_name) + job.output_dir.join(sanitize_filename(&new_name)) } else { let new_name = rename.apply_simple(stem, ext, (index as u32).saturating_add(1)); - job.output_dir.join(new_name) + job.output_dir.join(sanitize_filename(&new_name)) } } else { job.output_path_for(source, Some(output_format)) }; + // Prevent overwriting the source file + if paths_are_same(&source.path, &output_path) { + return Err(PixstripError::Processing { + operation: "output".into(), + reason: format!("output path is the same as source: {}", source.path.display()), + }); + } + // Handle overwrite behavior let output_path = match job.overwrite_behavior { crate::operations::OverwriteAction::Skip => { if output_path.exists() { // Return 0 bytes written - file was skipped - return Ok((input_size, 0)); + return Ok((input_size, 0, std::path::PathBuf::new())); } output_path } @@ -524,16 +575,25 @@ impl PipelineExecutor { // Ensure output directory exists if let Some(parent) = output_path.parent() { - std::fs::create_dir_all(parent).map_err(PixstripError::Io)?; + std::fs::create_dir_all(parent).map_err(|e| { + cleanup_placeholder(&output_path); + PixstripError::Io(e) + })?; } // Watermark (after compress settings determined, before encode) if let Some(ref config) = job.watermark { - img = apply_watermark(img, config)?; + img = apply_watermark(img, config).map_err(|e| { + cleanup_placeholder(&output_path); + e + })?; } // Encode and save - encoder.encode_to_file(&img, &output_path, output_format, quality)?; + encoder.encode_to_file(&img, &output_path, output_format, quality).map_err(|e| { + cleanup_placeholder(&output_path); + e + })?; // Metadata handling: re-encoding strips all EXIF by default. // KeepAll: copy everything back from source. @@ -563,7 +623,7 @@ impl PipelineExecutor { .map(|m| m.len()) .unwrap_or(0); - Ok((input_size, output_size)) + Ok((input_size, output_size, output_path)) } } @@ -614,14 +674,44 @@ fn auto_orient_from_exif( 2 => img.fliph(), // Flipped horizontal 3 => img.rotate180(), // Rotated 180 4 => img.flipv(), // Flipped vertical - 5 => img.rotate90().fliph(), // Transposed + 5 => img.fliph().rotate270(), // Transposed 6 => img.rotate90(), // Rotated 90 CW - 7 => img.rotate270().fliph(), // Transverse + 7 => img.fliph().rotate90(), // Transverse 8 => img.rotate270(), // Rotated 270 CW _ => img, } } +/// Strip path separators and parent-directory components from a user-provided filename. +/// Prevents path traversal attacks via rename templates (e.g., "../../etc/passwd"). +fn sanitize_filename(name: &str) -> String { + let path = std::path::Path::new(name); + path.file_name() + .and_then(|f| f.to_str()) + .unwrap_or("output") + .to_string() +} + +/// Check if two paths refer to the same file (resolves symlinks and relative paths). +fn paths_are_same(a: &std::path::Path, b: &std::path::Path) -> bool { + match (a.canonicalize(), b.canonicalize()) { + (Ok(ca), Ok(cb)) => ca == cb, + _ => { + // If canonicalize fails (file doesn't exist yet), compare components directly + a.as_os_str() == b.as_os_str() + } + } +} + +/// Remove a 0-byte placeholder file created by find_unique_path on error. +fn cleanup_placeholder(path: &std::path::Path) { + if let Ok(meta) = std::fs::metadata(path) { + if meta.len() == 0 { + let _ = std::fs::remove_file(path); + } + } +} + fn find_unique_path(path: &std::path::Path) -> std::path::PathBuf { let stem = path .file_stem() @@ -633,13 +723,21 @@ fn find_unique_path(path: &std::path::Path) -> std::path::PathBuf { .unwrap_or("bin"); let parent = path.parent().unwrap_or_else(|| std::path::Path::new(".")); + // Use O_CREAT | O_EXCL via create_new() to atomically reserve the path, + // preventing TOCTOU races when multiple threads auto-rename concurrently. for i in 1u32..10000 { let candidate = parent.join(format!("{}_{}.{}", stem, i, ext)); - if !candidate.exists() { - return candidate; + match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&candidate) + { + Ok(_) => return candidate, + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(_) => continue, } } - // Extremely unlikely fallback + // Extremely unlikely fallback - use timestamp for uniqueness parent.join(format!("{}_{}.{}", stem, std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis()) diff --git a/pixstrip-core/src/operations/adjustments.rs b/pixstrip-core/src/operations/adjustments.rs index cdc0dd6..7f2618d 100644 --- a/pixstrip-core/src/operations/adjustments.rs +++ b/pixstrip-core/src/operations/adjustments.rs @@ -68,12 +68,12 @@ fn crop_to_aspect_ratio(img: DynamicImage, w_ratio: f64, h_ratio: f64) -> Dynami let (crop_w, crop_h) = if current_ratio > target_ratio { // Image is wider than target, crop width - let new_w = (ih as f64 * target_ratio) as u32; - (new_w.min(iw), ih) + let new_w = (ih as f64 * target_ratio).round().max(1.0) as u32; + (new_w.min(iw).max(1), ih) } else { // Image is taller than target, crop height - let new_h = (iw as f64 / target_ratio) as u32; - (iw, new_h.min(ih)) + let new_h = (iw as f64 / target_ratio).round().max(1.0) as u32; + (iw, new_h.min(ih).max(1)) }; let x = iw.saturating_sub(crop_w) / 2; @@ -193,9 +193,11 @@ fn apply_sepia(img: DynamicImage) -> DynamicImage { } fn add_canvas_padding(img: DynamicImage, padding: u32) -> DynamicImage { + // Cap padding to prevent unreasonable memory allocation + let padding = padding.min(10_000); let (w, h) = (img.width(), img.height()); - let new_w = w.saturating_add(padding.saturating_mul(2)); - let new_h = h.saturating_add(padding.saturating_mul(2)); + let new_w = w.saturating_add(padding.saturating_mul(2)).min(65_535); + let new_h = h.saturating_add(padding.saturating_mul(2)).min(65_535); let mut canvas = RgbaImage::from_pixel(new_w, new_h, Rgba([255, 255, 255, 255])); diff --git a/pixstrip-core/src/operations/rename.rs b/pixstrip-core/src/operations/rename.rs index 2bcbd28..bd83623 100644 --- a/pixstrip-core/src/operations/rename.rs +++ b/pixstrip-core/src/operations/rename.rs @@ -31,6 +31,7 @@ pub fn apply_template_full( if let Some(end) = result[start..].find('}') { let spec = &result[start + 9..start + end]; if let Ok(padding) = spec.parse::() { + let padding = padding.min(10); let counter_str = format!("{:0>width$}", counter, width = padding); result = format!("{}{}{}", &result[..start], counter_str, &result[start + end + 1..]); } diff --git a/pixstrip-core/src/operations/watermark.rs b/pixstrip-core/src/operations/watermark.rs index 629bad3..55980be 100644 --- a/pixstrip-core/src/operations/watermark.rs +++ b/pixstrip-core/src/operations/watermark.rs @@ -363,12 +363,8 @@ fn apply_tiled_image_watermark( reason: format!("Failed to load watermark image: {}", e), })?; - let wm_width = (watermark.width() as f32 * scale) as u32; - let wm_height = (watermark.height() as f32 * scale) as u32; - - if wm_width == 0 || wm_height == 0 { - return Ok(img); - } + let wm_width = ((watermark.width() as f32 * scale) as u32).clamp(1, 16384); + let wm_height = ((watermark.height() as f32 * scale) as u32).clamp(1, 16384); let mut watermark = watermark.resize_exact(wm_width, wm_height, image::imageops::FilterType::Lanczos3); if let Some(rot) = rotation { @@ -429,13 +425,9 @@ fn apply_image_watermark( reason: format!("Failed to load watermark image: {}", e), })?; - // Scale the watermark - let wm_width = (watermark.width() as f32 * scale) as u32; - let wm_height = (watermark.height() as f32 * scale) as u32; - - if wm_width == 0 || wm_height == 0 { - return Ok(img); - } + // Scale the watermark (capped to prevent OOM on extreme scale values) + let wm_width = ((watermark.width() as f32 * scale) as u32).clamp(1, 16384); + let wm_height = ((watermark.height() as f32 * scale) as u32).clamp(1, 16384); let mut watermark = watermark.resize_exact( wm_width, diff --git a/pixstrip-core/src/pipeline.rs b/pixstrip-core/src/pipeline.rs index 40616da..de2b3e6 100644 --- a/pixstrip-core/src/pipeline.rs +++ b/pixstrip-core/src/pipeline.rs @@ -80,6 +80,7 @@ impl ProcessingJob { || self.convert.is_some() || self.compress.is_some() || self.watermark.is_some() + || self.output_dpi > 0 } pub fn output_path_for( diff --git a/pixstrip-gtk/src/app.rs b/pixstrip-gtk/src/app.rs index cddf722..0da0d9c 100644 --- a/pixstrip-gtk/src/app.rs +++ b/pixstrip-gtk/src/app.rs @@ -211,9 +211,9 @@ pub fn build_app() -> adw::Application { .map(|p| p.display().to_string()) .collect::>() .join("\n"); - action.downcast_ref::() - .unwrap() - .activate(Some(&paths_str.to_variant())); + if let Some(simple) = action.downcast_ref::() { + simple.activate(Some(&paths_str.to_variant())); + } } } }); @@ -617,6 +617,13 @@ fn build_ui(app: &adw::Application) { state.expanded_sections = app_state_for_close.expanded_sections.borrow().clone(); let _ = session.save(&state); + + // Clean up temporary download directory + let temp_downloads = std::env::temp_dir().join("pixstrip-downloads"); + if temp_downloads.exists() { + let _ = std::fs::remove_dir_all(&temp_downloads); + } + glib::Propagation::Proceed }); } @@ -2043,7 +2050,7 @@ fn continue_processing( file, } => { if let Some(ref bar) = progress_bar { - let frac = current as f64 / total as f64; + let frac = if total > 0 { (current as f64 / total as f64).clamp(0.0, 1.0) } else { 0.0 }; bar.set_fraction(frac); bar.set_text(Some(&format!("{}/{} - {}", current, total, file))); bar.update_property(&[ @@ -2122,17 +2129,8 @@ fn show_results( .map(|p| p.display().to_string()) .unwrap_or_default(); - // Collect actual output files from the output directory - let output_files: Vec = if let Some(ref dir) = *ui.state.output_dir.borrow() { - std::fs::read_dir(dir) - .into_iter() - .flatten() - .filter_map(|e| e.ok()) - .map(|e| e.path().display().to_string()) - .collect() - } else { - vec![] - }; + // Use actual output file paths from the executor (only successfully written files) + let output_files: Vec = result.output_files.clone(); let _ = history.add(pixstrip_core::storage::HistoryEntry { timestamp: format!( @@ -2496,9 +2494,12 @@ fn calculate_eta(start: &std::time::Instant, current: usize, total: usize) -> St if current == 0 { return "Estimating time remaining...".into(); } + if current >= total { + return "Almost done...".into(); + } let elapsed = start.elapsed().as_secs_f64(); let per_image = elapsed / current as f64; - let remaining = (total - current) as f64 * per_image; + let remaining = (total.saturating_sub(current)) as f64 * per_image; if remaining < 1.0 { "Almost done...".into() } else { diff --git a/pixstrip-gtk/src/steps/mod.rs b/pixstrip-gtk/src/steps/mod.rs index 582d901..0f721f6 100644 --- a/pixstrip-gtk/src/steps/mod.rs +++ b/pixstrip-gtk/src/steps/mod.rs @@ -16,7 +16,7 @@ use gtk::prelude::*; pub fn full_text_list_factory() -> gtk::SignalListItemFactory { let factory = gtk::SignalListItemFactory::new(); factory.connect_setup(|_, item| { - let item = item.downcast_ref::().unwrap(); + let Some(item) = item.downcast_ref::() else { return }; let label = gtk::Label::builder() .xalign(0.0) .margin_start(8) @@ -27,7 +27,7 @@ pub fn full_text_list_factory() -> gtk::SignalListItemFactory { item.set_child(Some(&label)); }); factory.connect_bind(|_, item| { - let item = item.downcast_ref::().unwrap(); + let Some(item) = item.downcast_ref::() else { return }; if let Some(obj) = item.item() { if let Some(string_obj) = obj.downcast_ref::() { if let Some(label) = item.child().and_downcast_ref::() { diff --git a/pixstrip-gtk/src/steps/step_adjustments.rs b/pixstrip-gtk/src/steps/step_adjustments.rs index e84c1ef..4ec63eb 100644 --- a/pixstrip-gtk/src/steps/step_adjustments.rs +++ b/pixstrip-gtk/src/steps/step_adjustments.rs @@ -282,7 +282,7 @@ pub fn build_adjustments_page(state: &AppState) -> adw::NavigationPage { return; } - let idx = pidx.get().min(loaded.len() - 1); + let idx = pidx.get().min(loaded.len().saturating_sub(1)); pidx.set(idx); let path = loaded[idx].clone(); let name = path.file_name().and_then(|n| n.to_str()).unwrap_or("image"); diff --git a/pixstrip-gtk/src/steps/step_images.rs b/pixstrip-gtk/src/steps/step_images.rs index 4c11a66..fd8df97 100644 --- a/pixstrip-gtk/src/steps/step_images.rs +++ b/pixstrip-gtk/src/steps/step_images.rs @@ -883,20 +883,25 @@ fn download_image_url(url: &str) -> Option { let temp_dir = std::env::temp_dir().join("pixstrip-downloads"); std::fs::create_dir_all(&temp_dir).ok()?; - // Extract filename from URL + // Extract and sanitize filename from URL to prevent path traversal let url_path = url.split('?').next().unwrap_or(url); - let filename = url_path + let raw_name = url_path .rsplit('/') .next() - .unwrap_or("downloaded.jpg") - .to_string(); + .unwrap_or("downloaded.jpg"); + let sanitized = std::path::Path::new(raw_name) + .file_name() + .and_then(|f| f.to_str()) + .unwrap_or("downloaded.jpg"); + let filename = if sanitized.is_empty() { "downloaded.jpg" } else { sanitized }; - let dest = temp_dir.join(&filename); + let dest = temp_dir.join(filename); // Use GIO for the download (synchronous, runs in background thread) let gfile = gtk::gio::File::for_uri(url); let stream = gfile.read(gtk::gio::Cancellable::NONE).ok()?; + const MAX_DOWNLOAD_BYTES: usize = 100 * 1024 * 1024; // 100 MB let mut buf = Vec::new(); loop { let bytes = stream.read_bytes(8192, gtk::gio::Cancellable::NONE).ok()?; @@ -904,6 +909,9 @@ fn download_image_url(url: &str) -> Option { break; } buf.extend_from_slice(&bytes); + if buf.len() > MAX_DOWNLOAD_BYTES { + return None; + } } if buf.is_empty() { diff --git a/pixstrip-gtk/src/steps/step_rename.rs b/pixstrip-gtk/src/steps/step_rename.rs index f9b3f18..da3e575 100644 --- a/pixstrip-gtk/src/steps/step_rename.rs +++ b/pixstrip-gtk/src/steps/step_rename.rs @@ -384,7 +384,8 @@ pub fn build_rename_page(state: &AppState) -> adw::NavigationPage { tr.set_text(&var_text); tr.set_position(var_text.chars().count() as i32); } else { - let pos = tr.position() as usize; + let char_count = current.chars().count(); + let pos = (tr.position().max(0) as usize).min(char_count); let byte_pos = current.char_indices() .nth(pos) .map(|(i, _)| i) @@ -392,7 +393,7 @@ pub fn build_rename_page(state: &AppState) -> adw::NavigationPage { let mut new_text = current.clone(); new_text.insert_str(byte_pos, &var_text); tr.set_text(&new_text); - tr.set_position((pos + var_text.chars().count()) as i32); + tr.set_position((pos.saturating_add(var_text.chars().count())) as i32); } }); diff --git a/pixstrip-gtk/src/steps/step_resize.rs b/pixstrip-gtk/src/steps/step_resize.rs index 4996331..26c5a3f 100644 --- a/pixstrip-gtk/src/steps/step_resize.rs +++ b/pixstrip-gtk/src/steps/step_resize.rs @@ -445,12 +445,14 @@ pub fn build_resize_page(state: &AppState) -> adw::NavigationPage { std::thread::spawn(move || { let result = (|| -> Option> { let img = image::open(&path).ok()?; - let target_w = if render_tw > 0 { render_tw } else { img.width() }; + let target_w = if render_tw > 0 { render_tw } else { img.width().max(1) }; let target_h = if render_th > 0 { render_th - } else { + } else if img.width() > 0 { let scale = target_w as f64 / img.width() as f64; - (img.height() as f64 * scale).round() as u32 + (img.height() as f64 * scale).round().max(1.0) as u32 + } else { + target_w }; let resized = if mode == 0 && render_th > 0 { // Exact: stretch to exact dimensions diff --git a/pixstrip-gtk/src/steps/step_watermark.rs b/pixstrip-gtk/src/steps/step_watermark.rs index a0a25b4..3e74a29 100644 --- a/pixstrip-gtk/src/steps/step_watermark.rs +++ b/pixstrip-gtk/src/steps/step_watermark.rs @@ -434,7 +434,7 @@ pub fn build_watermark_page(state: &AppState) -> adw::NavigationPage { return; } - let idx = pidx.get().min(loaded.len() - 1); + let idx = pidx.get().min(loaded.len().saturating_sub(1)); pidx.set(idx); let path = loaded[idx].clone(); let name = path.file_name().and_then(|n| n.to_str()).unwrap_or("image");