Fix overflow, race condition, and format bugs
This commit is contained in:
@@ -176,16 +176,6 @@ impl PipelineExecutor {
|
||||
.unwrap_or("unknown")
|
||||
.to_string();
|
||||
|
||||
let current = completed_ref.fetch_add(1, Ordering::Relaxed) + 1;
|
||||
|
||||
let _ = tx_clone.send(ProgressUpdate {
|
||||
current,
|
||||
total,
|
||||
current_file: file_name.clone(),
|
||||
succeeded_so_far: succeeded_ref.load(Ordering::Relaxed),
|
||||
failed_so_far: failed_ref.load(Ordering::Relaxed),
|
||||
});
|
||||
|
||||
let loader = ImageLoader::new();
|
||||
let encoder = OutputEncoder::with_options(EncoderOptions {
|
||||
progressive_jpeg: job.progressive_jpeg,
|
||||
@@ -207,13 +197,23 @@ impl PipelineExecutor {
|
||||
Err(e) => {
|
||||
failed_ref.fetch_add(1, Ordering::Relaxed);
|
||||
if let Ok(mut errs) = errors_ref.lock() {
|
||||
errs.push((file_name, e.to_string()));
|
||||
errs.push((file_name.clone(), e.to_string()));
|
||||
}
|
||||
if pause_on_error {
|
||||
pause_flag.store(true, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Send progress after processing so counts are consistent
|
||||
let current = completed_ref.fetch_add(1, Ordering::Relaxed) + 1;
|
||||
let _ = tx_clone.send(ProgressUpdate {
|
||||
current,
|
||||
total,
|
||||
current_file: file_name,
|
||||
succeeded_so_far: succeeded_ref.load(Ordering::Relaxed),
|
||||
failed_so_far: failed_ref.load(Ordering::Relaxed),
|
||||
});
|
||||
});
|
||||
});
|
||||
// Drop sender so the receiver loop ends
|
||||
@@ -493,8 +493,8 @@ impl PipelineExecutor {
|
||||
} => match output_format {
|
||||
ImageFormat::Jpeg => jpeg_quality.unwrap_or(85),
|
||||
ImageFormat::Png => png_level.unwrap_or(6),
|
||||
ImageFormat::WebP => webp_quality.map(|q| q as u8).unwrap_or(80),
|
||||
ImageFormat::Avif => avif_quality.map(|q| q as u8).unwrap_or(50),
|
||||
ImageFormat::WebP => webp_quality.map(|q| q.clamp(0.0, 100.0) as u8).unwrap_or(80),
|
||||
ImageFormat::Avif => avif_quality.map(|q| q.clamp(0.0, 100.0) as u8).unwrap_or(50),
|
||||
_ => 85,
|
||||
},
|
||||
});
|
||||
@@ -599,22 +599,33 @@ impl PipelineExecutor {
|
||||
// KeepAll: copy everything back from source.
|
||||
// Privacy/Custom: copy metadata back, then selectively strip certain tags.
|
||||
// StripAll: do nothing (already stripped by re-encoding).
|
||||
// Note: little_exif only supports JPEG and TIFF metadata manipulation.
|
||||
if let Some(ref meta_config) = job.metadata {
|
||||
let format_supports_exif = matches!(
|
||||
output_format,
|
||||
ImageFormat::Jpeg | ImageFormat::Tiff
|
||||
);
|
||||
match meta_config {
|
||||
crate::operations::MetadataConfig::KeepAll => {
|
||||
if !copy_metadata_from_source(&source.path, &output_path) {
|
||||
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
|
||||
if format_supports_exif {
|
||||
if !copy_metadata_from_source(&source.path, &output_path) {
|
||||
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
|
||||
}
|
||||
}
|
||||
// For non-JPEG/TIFF formats, metadata is lost during re-encoding
|
||||
// and cannot be restored. This is a known limitation.
|
||||
}
|
||||
crate::operations::MetadataConfig::StripAll => {
|
||||
// Already stripped by re-encoding - nothing to do
|
||||
}
|
||||
_ => {
|
||||
// Privacy or Custom: copy all metadata back, then strip unwanted tags
|
||||
if !copy_metadata_from_source(&source.path, &output_path) {
|
||||
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
|
||||
if format_supports_exif {
|
||||
if !copy_metadata_from_source(&source.path, &output_path) {
|
||||
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
|
||||
}
|
||||
strip_selective_metadata(&output_path, meta_config);
|
||||
}
|
||||
strip_selective_metadata(&output_path, meta_config);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -697,8 +708,17 @@ 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()
|
||||
// If canonicalize fails (output file doesn't exist yet),
|
||||
// canonicalize parent directories and compare with filename appended
|
||||
let resolve = |p: &std::path::Path| -> Option<std::path::PathBuf> {
|
||||
let parent = p.parent()?;
|
||||
let name = p.file_name()?;
|
||||
Some(parent.canonicalize().ok()?.join(name))
|
||||
};
|
||||
match (resolve(a), resolve(b)) {
|
||||
(Some(ra), Some(rb)) => ra == rb,
|
||||
_ => a.as_os_str() == b.as_os_str(),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -732,7 +752,12 @@ fn find_unique_path(path: &std::path::Path) -> std::path::PathBuf {
|
||||
.create_new(true)
|
||||
.open(&candidate)
|
||||
{
|
||||
Ok(_) => return candidate,
|
||||
Ok(mut f) => {
|
||||
// Write a marker byte so cleanup_placeholder (which only removes
|
||||
// 0-byte files) won't delete our reservation before the real write
|
||||
let _ = std::io::Write::write_all(&mut f, b"~");
|
||||
return candidate;
|
||||
}
|
||||
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
|
||||
Err(_) => continue,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user