diff --git a/Cargo.toml b/Cargo.toml index 2300c5b..ade44d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,5 +4,10 @@ members = [ "czkawka_cli", "czkawka_gui", ] -#[profile.release] -#lto = true \ No newline at end of file +[profile.release] +# Panic = "abort" will crash entire app when processing invalid image file. +# Since crash happens in external library, this is only way to handle this(I think). +panic = "unwind" + +# LTO setting is disabled by default, because release mode is usually needed to develop app and compilation with LTO would take a lot of time +#lto = "fat" \ No newline at end of file diff --git a/czkawka_core/src/broken_files.rs b/czkawka_core/src/broken_files.rs index 8fe31e5..b1f33cf 100644 --- a/czkawka_core/src/broken_files.rs +++ b/czkawka_core/src/broken_files.rs @@ -7,7 +7,7 @@ use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::Arc; use std::thread::sleep; use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use std::{fs, mem, thread}; +use std::{fs, mem, panic, thread}; use crossbeam_channel::Receiver; use directories_next::ProjectDirs; @@ -403,30 +403,40 @@ impl BrokenFiles { }; //// PROGRESS THREAD END let mut vec_file_entry: Vec = non_cached_files_to_check - .par_iter() - .map(|file_entry| { + .into_par_iter() + .map(|(_, mut file_entry)| { atomic_file_counter.fetch_add(1, Ordering::Relaxed); if stop_receiver.is_some() && stop_receiver.unwrap().try_recv().is_ok() { check_was_breaked.store(true, Ordering::Relaxed); return None; } - let file_entry = file_entry.1; match file_entry.type_of_file { TypeOfFile::Image => { - match image::open(&file_entry.path) { - Ok(_) => Some(None), - Err(t) => { - let error_string = t.to_string(); - // This error is a problem with image library, remove check when https://github.com/image-rs/jpeg-decoder/issues/130 will be fixed - if !error_string.contains("spectral selection is not allowed in non-progressive scan") { - let mut file_entry = file_entry.clone(); - file_entry.error_string = error_string; - Some(Some(file_entry)) - } else { - Some(None) + let file_entry_clone = file_entry.clone(); + + let result = panic::catch_unwind(|| { + match image::open(&file_entry.path) { + Ok(_) => Some(None), + Err(t) => { + let error_string = t.to_string(); + // This error is a problem with image library, remove check when https://github.com/image-rs/jpeg-decoder/issues/130 will be fixed + if !error_string.contains("spectral selection is not allowed in non-progressive scan") { + file_entry.error_string = error_string; + Some(Some(file_entry)) + } else { + Some(None) + } } - } // Something is wrong with image + } + }); + + // If image crashed during opening, we just skip checking its hash and go on + if let Ok(image_result) = result { + image_result + } else { + println!("Image-rs library crashed when opening \"{:?}\" image, please check if problem happens with latest image-rs version(this can be checked via https://github.com/qarmin/ImageOpening tool) and if it is not reported, please report bug here - https://github.com/image-rs/image/issues", file_entry_clone.path); + Some(Some(file_entry_clone)) } } TypeOfFile::ArchiveZip => match fs::File::open(&file_entry.path) { @@ -434,9 +444,7 @@ impl BrokenFiles { Ok(_) => Some(None), Err(e) => { // TODO Maybe filter out unnecessary types of errors - let error_string = e.to_string(); - let mut file_entry = file_entry.clone(); - file_entry.error_string = error_string; + file_entry.error_string = e.to_string(); Some(Some(file_entry)) } }, @@ -447,9 +455,7 @@ impl BrokenFiles { Ok(file) => match rodio::Decoder::new(BufReader::new(file)) { Ok(_) => Some(None), Err(e) => { - let error_string = e.to_string(); - let mut file_entry = file_entry.clone(); - file_entry.error_string = error_string; + file_entry.error_string = e.to_string(); Some(Some(file_entry)) } }, @@ -747,10 +753,7 @@ fn load_cache_from_file(text_messages: &mut Messages) -> Option TypeOfFile { // Checking allowed image extensions let allowed_image_extensions = [ - ".jpg", ".jpeg", ".png", /*, ".bmp"*/ - /*".tiff", ".tif",*/ ".tga", ".ff", /*, ".gif"*/ - // Gif will be reenabled in image-rs 0.24 - ".jif", ".jfi", /*, ".ico"*/ + ".jpg", ".jpeg", ".png", ".bmp", ".tiff", ".tif", ".tga", ".ff", ".gif", ".jif", ".jfi", ".ico", // Ico and bmp crashes are not fixed yet /*".webp",*/ ".avif", // Webp is not really supported in image crate ]; diff --git a/czkawka_core/src/duplicate.rs b/czkawka_core/src/duplicate.rs index 6333e7d..e4985c1 100644 --- a/czkawka_core/src/duplicate.rs +++ b/czkawka_core/src/duplicate.rs @@ -1061,22 +1061,21 @@ impl DuplicateFinder { } full_hash_results = non_cached_files_to_check - .par_iter() + .into_par_iter() .map(|(size, vec_file_entry)| { let mut hashmap_with_hash: BTreeMap> = Default::default(); let mut errors: Vec = Vec::new(); let mut buffer = [0u8; 1024 * 128]; atomic_file_counter.fetch_add(vec_file_entry.len(), Ordering::Relaxed); - for file_entry in vec_file_entry { + for mut file_entry in vec_file_entry { if stop_receiver.is_some() && stop_receiver.unwrap().try_recv().is_ok() { check_was_breaked.store(true, Ordering::Relaxed); return None; } - match hash_calculation(&mut buffer, file_entry, &check_type, u64::MAX) { + match hash_calculation(&mut buffer, &file_entry, &check_type, u64::MAX) { Ok(hash_string) => { - let mut file_entry = file_entry.clone(); file_entry.hash = hash_string.clone(); hashmap_with_hash.entry(hash_string.clone()).or_insert_with(Vec::new); hashmap_with_hash.get_mut(hash_string.as_str()).unwrap().push(file_entry); @@ -1084,7 +1083,7 @@ impl DuplicateFinder { Err(s) => errors.push(s), } } - Some((*size, hashmap_with_hash, errors)) + Some((size, hashmap_with_hash, errors)) }) .while_some() .collect(); diff --git a/czkawka_core/src/similar_images.rs b/czkawka_core/src/similar_images.rs index 9324934..1f6790e 100644 --- a/czkawka_core/src/similar_images.rs +++ b/czkawka_core/src/similar_images.rs @@ -3,6 +3,7 @@ use std::fs::OpenOptions; use std::fs::{File, Metadata}; use std::io::Write; use std::io::*; +use std::panic; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::Arc; @@ -269,10 +270,8 @@ impl SimilarImages { let mut folders_to_check: Vec = Vec::with_capacity(1024 * 2); // This should be small enough too not see to big difference and big enough to store most of paths without needing to resize vector self.allowed_extensions.extend_allowed_extensions(&[ - ".jpg", ".jpeg", ".png", /*, ".bmp"*/ - /*".tiff", ".tif",*/ ".tga", ".ff", /*, ".gif"*/ - ".jif", ".jfi", /*, ".webp"*/ - ]); // webp cannot be seen in preview, gif needs to be enabled after releasing image crate 0.24.0, bmp needs to be fixed in image crate + ".jpg", ".jpeg", ".png", ".bmp", ".tiff", ".tif", ".tga", ".ff", ".jif", ".jfi", /*, ".webp", ".gif", ".ico"*/ + ]); // webp cannot be seen in preview, gif is not too good to checking preview because is animated, ico is just ico and is not too usable // Add root folders for finding for id in &self.directories.included_directories { @@ -527,29 +526,38 @@ impl SimilarImages { }; //// PROGRESS THREAD END let mut vec_file_entry: Vec<(FileEntry, Vec)> = non_cached_files_to_check - .par_iter() - .map(|file_entry| { + .into_par_iter() + .map(|(_s, mut file_entry)| { atomic_file_counter.fetch_add(1, Ordering::Relaxed); if stop_receiver.is_some() && stop_receiver.unwrap().try_recv().is_ok() { // This will not break return None; } - let mut file_entry = file_entry.1.clone(); - let image = match image::open(file_entry.path.clone()) { - Ok(t) => t, - // Err(_inspected) => return Some(None), // Something is wrong with image, - // For broken images empty hash is used, because without it will try to resecan files each time when it is called(missing cache file is responsible for it) - // This may cause problems(very rarely), when e.g. file was not available due lack of permissions, but it is available now - Err(_inspected) => { - let mut buf = Vec::new(); - for _i in 0..(self.hash_size * self.hash_size / 8) { - buf.push(0); - } - file_entry.hash = buf.clone(); - return Some(Some((file_entry, buf))); + let image; + + let result = panic::catch_unwind(|| { + match image::open(file_entry.path.clone()) { + Ok(t) => Ok(t), + // Err(_inspected) => return Some(None), // Something is wrong with image, + // For broken images empty hash is used, because without it will try to resecan files each time when it is called(missing cache file is responsible for it) + // This may cause problems(very rarely), when e.g. file was not available due lack of permissions, but it is available now + Err(_inspected) => Err(()), } - }; + }); + + // If image crashed during opening, we just skip checking its hash and go on + if let Ok(image_result) = result { + if let Ok(image2) = image_result { + image = image2; + } else { + return Some(Some((file_entry, Vec::new()))); + } + } else { + println!("Image-rs library crashed when opening \"{:?}\" image, please check if problem happens with latest image-rs version(this can be checked via https://github.com/qarmin/ImageOpening tool) and if it is not reported, please report bug here - https://github.com/image-rs/image/issues", file_entry.path); + return Some(Some((file_entry, Vec::new()))); + } + let dimensions = image.dimensions(); file_entry.dimensions = format!("{}x{}", dimensions.0, dimensions.1); @@ -587,7 +595,7 @@ impl SimilarImages { // All valid entries are used to create bktree used to check for hash similarity for (file_entry, buf) in &vec_file_entry { // Only use to comparing, non broken hashes(all 0 or 255 hashes means that algorithm fails to decode them because e.g. contains a log of alpha channel) - if !(buf.iter().all(|e| *e == 0) || buf.iter().all(|e| *e == 255)) { + if !(buf.is_empty() || buf.iter().all(|e| *e == 0) || buf.iter().all(|e| *e == 255)) { self.image_hashes.entry(buf.clone()).or_insert_with(Vec::::new); self.image_hashes.get_mut(buf).unwrap().push(file_entry.clone()); }