Browse Source

Handle image crashes (#530)

* Don't crash when checking images

* Don't crash when checking images

* Don't crash when checking images

* HERE

* REME

* REE
pull/534/head
Rafał Mikrut 5 months ago committed by GitHub
parent
commit
de964ce40d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 9
      Cargo.toml
  2. 55
      czkawka_core/src/broken_files.rs
  3. 9
      czkawka_core/src/duplicate.rs
  4. 52
      czkawka_core/src/similar_images.rs

9
Cargo.toml

@ -4,5 +4,10 @@ members = [
"czkawka_cli",
"czkawka_gui",
]
#[profile.release]
#lto = true
[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"

55
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<FileEntry> = 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<BTreeMap<String,
fn check_extension_avaibility(file_name_lowercase: &str) -> 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
];

9
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<String, Vec<FileEntry>> = Default::default();
let mut errors: Vec<String> = 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();

52
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<PathBuf> = 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<u8>)> = 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::<FileEntry>::new);
self.image_hashes.get_mut(buf).unwrap().push(file_entry.clone());
}

Loading…
Cancel
Save