lashman/pixstrip
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Fix 16 medium-severity bugs from audit

Browse Source 
CLI: add UTC suffix to timestamps, validate image extensions on
single-file input, canonicalize watch paths for reliable matching,
derive counter_enabled from template presence, warn when undo count
exceeds available batches.

Core: apply space/special-char transforms in template rename path,
warn on metadata preservation for unsupported formats, derive AVIF
speed from compress preset quality level.

GTK: use buffer size for apples-to-apples compress preview comparison,
shorten approximate format labels, cache file sizes to avoid repeated
syscalls on checkbox toggle, add batch-update guard to prevent O(n^2)
in select/deselect all, use widget names for reliable progress/log
lookup, add unique suffix for duplicate download filenames.
This commit is contained in:
lashman
2026-03-07 23:02:57 +02:00
parent 9ef33fa90f
commit 9fcbe237bd
7 changed files with 136 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
pixstrip-gtk/src/app.rs
View File

@@ -109,6 +109,7 @@ pub struct AppState {
pub wizard: Rc<RefCell<WizardState>>,
pub loaded_files: Rc<RefCell<Vec<std::path::PathBuf>>>,
pub excluded_files: Rc<RefCell<std::collections::HashSet<std::path::PathBuf>>>,
pub file_sizes: Rc<RefCell<std::collections::HashMap<std::path::PathBuf, u64>>>,
pub output_dir: Rc<RefCell<Option<std::path::PathBuf>>>,
pub job_config: Rc<RefCell<JobConfig>>,
pub detailed_mode: bool,
@@ -321,6 +322,7 @@ fn build_ui(app: &adw::Application) {
wizard: Rc::new(RefCell::new(WizardState::new())),
loaded_files: Rc::new(RefCell::new(Vec::new())),
excluded_files: Rc::new(RefCell::new(std::collections::HashSet::new())),
file_sizes: Rc::new(RefCell::new(std::collections::HashMap::new())),
output_dir: Rc::new(RefCell::new(None)),
detailed_mode: app_cfg.skill_level.is_advanced(),
batch_queue: Rc::new(RefCell::new(BatchQueue::default())),
@@ -1397,6 +1399,7 @@ fn update_images_count_label(ui: &WizardUi, count: usize) {
&loaded_box,
&ui.state.loaded_files,
&ui.state.excluded_files,
&ui.state.file_sizes,
);
}
}
@@ -2576,14 +2579,10 @@ fn update_progress_labels(nav_view: &adw::NavigationView, current: usize, total:
if let Some(page) = nav_view.visible_page() {
walk_widgets(&page.child(), &|widget| {
if let Some(label) = widget.downcast_ref::<gtk::Label>() {
if label.css_classes().iter().any(|c| c == "heading")
&& (label.label().contains("images") || label.label().contains("0 /"))
{
if label.widget_name() == "processing-count-label" {
label.set_label(&format!("{} / {} images", current, total));
}
if label.css_classes().iter().any(|c| c == "dim-label")
&& (label.label().contains("Estimating") || label.label().contains("ETA") || label.label().contains("Almost") || label.label().contains("Current"))
{
if label.widget_name() == "processing-eta-label" {
if current < total {
label.set_label(&format!("{} - {}", eta, file));
} else {
@@ -2599,8 +2598,7 @@ fn add_log_entry(nav_view: &adw::NavigationView, current: usize, total: usize, f
if let Some(page) = nav_view.visible_page() {
walk_widgets(&page.child(), &|widget| {
if let Some(bx) = widget.downcast_ref::<gtk::Box>()
&& bx.spacing() == 2
&& bx.orientation() == gtk::Orientation::Vertical
&& bx.widget_name() == "processing-log-box"
{
let entry = gtk::Label::builder()
.label(format!("[{}/{}] {} - Done", current, total, file))

3
pixstrip-gtk/src/processing.rs
View File

@@ -24,6 +24,7 @@ pub fn build_processing_page() -> adw::NavigationPage {
.css_classes(["heading"])
.halign(gtk::Align::Start)
.build();
progress_label.set_widget_name("processing-count-label");
let progress_bar = gtk::ProgressBar::builder()
.fraction(0.0)
@@ -39,6 +40,7 @@ pub fn build_processing_page() -> adw::NavigationPage {
.css_classes(["dim-label"])
.halign(gtk::Align::Start)
.build();
eta_label.set_widget_name("processing-eta-label");
// Activity log
let log_group = adw::PreferencesGroup::builder()
@@ -55,6 +57,7 @@ pub fn build_processing_page() -> adw::NavigationPage {
.orientation(gtk::Orientation::Vertical)
.spacing(2)
.build();
log_box.set_widget_name("processing-log-box");
log_scrolled.set_child(Some(&log_box));
log_group.add(&log_scrolled);

16
pixstrip-gtk/src/steps/step_compress.rs
View File

@@ -937,8 +937,6 @@ enum PreviewResult {
}
fn generate_preview(path: &std::path::Path, comp: PreviewCompression) -> PreviewResult {
let original_size = std::fs::metadata(path).map(|m| m.len()).unwrap_or(0);
let img = match image::open(path) {
Ok(img) => img,
Err(e) => return PreviewResult::Error(e.to_string()),
@@ -959,6 +957,10 @@ fn generate_preview(path: &std::path::Path, comp: PreviewCompression) -> Preview
return PreviewResult::Error("Failed to encode original".into());
}
// Use the preview buffer size for comparison (not on-disk size)
// so both original and compressed refer to the same downscaled image
let original_size = orig_buf.len() as u64;
// Encode compressed in the requested format
let mut comp_buf = Vec::new();
let format_label;
@@ -1008,9 +1010,9 @@ fn generate_preview(path: &std::path::Path, comp: PreviewCompression) -> Preview
}
}
PreviewCompression::WebP(quality) => {
// image 0.25 only has lossless WebP encoding, so approximate with
// JPEG at equivalent quality for the visual preview
format_label = format!("WebP Q{} (approx)", quality);
// image crate only has lossless WebP encoding, so approximate with
// JPEG at equivalent quality for the visual preview (size estimate only)
format_label = format!("~WebP Q{}", quality);
let cursor = std::io::Cursor::new(&mut comp_buf);
let encoder = image::codecs::jpeg::JpegEncoder::new_with_quality(cursor, quality);
let rgb = preview_img.to_rgb8();
@@ -1027,8 +1029,8 @@ fn generate_preview(path: &std::path::Path, comp: PreviewCompression) -> Preview
}
}
PreviewCompression::Avif(quality) => {
// AVIF encoding not available in image crate - approximate with JPEG
format_label = format!("AVIF Q{} (approx)", quality);
// AVIF encoding not available in image crate - approximate with JPEG (size estimate only)
format_label = format!("~AVIF Q{}", quality);
let cursor = std::io::Cursor::new(&mut comp_buf);
let encoder = image::codecs::jpeg::JpegEncoder::new_with_quality(cursor, quality);
let rgb = preview_img.to_rgb8();

85
pixstrip-gtk/src/steps/step_images.rs
View File

@@ -36,6 +36,7 @@ pub fn build_images_page(state: &AppState) -> adw::NavigationPage {
{
let loaded_files = state.loaded_files.clone();
let excluded = state.excluded_files.clone();
let sizes = state.file_sizes.clone();
let stack_ref = stack.clone();
let subfolder_choice = subfolder_choice.clone();
drop_target.connect_drop(move |target, value, _x, _y| {
@@ -49,7 +50,7 @@ pub fn build_images_page(state: &AppState) -> adw::NavigationPage {
add_images_flat(&path, &mut files);
let count = files.len();
drop(files);
refresh_grid(&stack_ref, &loaded_files, &excluded, count);
refresh_grid(&stack_ref, &loaded_files, &excluded, &sizes, count);
} else {
let choice = *subfolder_choice.borrow();
match choice {
@@ -58,24 +59,25 @@ pub fn build_images_page(state: &AppState) -> adw::NavigationPage {
add_images_from_dir(&path, &mut files);
let count = files.len();
drop(files);
refresh_grid(&stack_ref, &loaded_files, &excluded, count);
refresh_grid(&stack_ref, &loaded_files, &excluded, &sizes, count);
}
Some(false) => {
let mut files = loaded_files.borrow_mut();
add_images_flat(&path, &mut files);
let count = files.len();
drop(files);
refresh_grid(&stack_ref, &loaded_files, &excluded, count);
refresh_grid(&stack_ref, &loaded_files, &excluded, &sizes, count);
}
None => {
let mut files = loaded_files.borrow_mut();
add_images_flat(&path, &mut files);
let count = files.len();
drop(files);
refresh_grid(&stack_ref, &loaded_files, &excluded, count);
refresh_grid(&stack_ref, &loaded_files, &excluded, &sizes, count);
let loaded_files = loaded_files.clone();
let excluded = excluded.clone();
let sz = sizes.clone();
let stack_ref = stack_ref.clone();
let subfolder_choice = subfolder_choice.clone();
let dir_path = path.clone();
@@ -88,6 +90,7 @@ pub fn build_images_page(state: &AppState) -> adw::NavigationPage {
&dir_path,
&loaded_files,
&excluded,
&sz,
&stack_ref,
&subfolder_choice,
);
@@ -103,7 +106,7 @@ pub fn build_images_page(state: &AppState) -> adw::NavigationPage {
}
let count = files.len();
drop(files);
refresh_grid(&stack_ref, &loaded_files, &excluded, count);
refresh_grid(&stack_ref, &loaded_files, &excluded, &sizes, count);
return true;
}
}
@@ -118,6 +121,7 @@ pub fn build_images_page(state: &AppState) -> adw::NavigationPage {
{
let loaded_files = state.loaded_files.clone();
let excluded = state.excluded_files.clone();
let sizes = state.file_sizes.clone();
let stack_ref = stack.clone();
uri_drop.connect_drop(move |_target, value, _x, _y| {
if let Ok(text) = value.get::<glib::GString>() {
@@ -141,6 +145,7 @@ pub fn build_images_page(state: &AppState) -> adw::NavigationPage {
if is_image_url {
let loaded = loaded_files.clone();
let excl = excluded.clone();
let sz = sizes.clone();
let sr = stack_ref.clone();
// Download in background thread
let (tx, rx) = std::sync::mpsc::channel::<Option<std::path::PathBuf>>();
@@ -159,7 +164,7 @@ pub fn build_images_page(state: &AppState) -> adw::NavigationPage {
}
let count = files.len();
drop(files);
refresh_grid(&sr, &loaded, &excl, count);
refresh_grid(&sr, &loaded, &excl, &sz, count);
glib::ControlFlow::Break
}
Ok(None) => glib::ControlFlow::Break,
@@ -259,6 +264,7 @@ fn show_subfolder_prompt(
dir: &std::path::Path,
loaded_files: &Rc<RefCell<Vec<PathBuf>>>,
excluded: &Rc<RefCell<HashSet<PathBuf>>>,
file_sizes: &Rc<RefCell<std::collections::HashMap<PathBuf, u64>>>,
stack: &gtk::Stack,
subfolder_choice: &Rc<RefCell<Option<bool>>>,
) {
@@ -273,6 +279,7 @@ fn show_subfolder_prompt(
let loaded_files = loaded_files.clone();
let excluded = excluded.clone();
let file_sizes = file_sizes.clone();
let stack = stack.clone();
let subfolder_choice = subfolder_choice.clone();
let dir = dir.to_path_buf();
@@ -284,7 +291,7 @@ fn show_subfolder_prompt(
add_images_from_subdirs(&dir, &mut files);
let count = files.len();
drop(files);
refresh_grid(&stack, &loaded_files, &excluded, count);
refresh_grid(&stack, &loaded_files, &excluded, &file_sizes, count);
}
});
@@ -302,6 +309,7 @@ fn refresh_grid(
stack: &gtk::Stack,
loaded_files: &Rc<RefCell<Vec<PathBuf>>>,
excluded: &Rc<RefCell<HashSet<PathBuf>>>,
file_sizes: &Rc<RefCell<std::collections::HashMap<PathBuf, u64>>>,
count: usize,
) {
if count > 0 {
@@ -310,7 +318,7 @@ fn refresh_grid(
stack.set_visible_child_name("empty");
}
if let Some(loaded_widget) = stack.child_by_name("loaded") {
rebuild_grid_model(&loaded_widget, loaded_files, excluded);
rebuild_grid_model(&loaded_widget, loaded_files, excluded, file_sizes);
}
}
@@ -319,15 +327,25 @@ pub fn rebuild_grid_model(
widget: &gtk::Widget,
loaded_files: &Rc<RefCell<Vec<PathBuf>>>,
excluded: &Rc<RefCell<HashSet<PathBuf>>>,
file_sizes: &Rc<RefCell<std::collections::HashMap<PathBuf, u64>>>,
) {
let files = loaded_files.borrow();
let excluded_set = excluded.borrow();
let count = files.len();
let included_count = files.iter().filter(|p| !excluded_set.contains(*p)).count();
// Populate size cache for any new files
{
let mut sizes = file_sizes.borrow_mut();
for p in files.iter() {
sizes.entry(p.clone()).or_insert_with(|| {
std::fs::metadata(p).map(|m| m.len()).unwrap_or(0)
});
}
}
let sizes = file_sizes.borrow();
let total_size: u64 = files.iter()
.filter(|p| !excluded_set.contains(*p))
.filter_map(|p| std::fs::metadata(p).ok())
.map(|m| m.len())
.map(|p| sizes.get(p).copied().unwrap_or(0))
.sum();
let size_str = format_size(total_size);
@@ -386,15 +404,16 @@ fn update_count_label(
widget: &gtk::Widget,
loaded_files: &Rc<RefCell<Vec<PathBuf>>>,
excluded: &Rc<RefCell<HashSet<PathBuf>>>,
file_sizes: &Rc<RefCell<std::collections::HashMap<PathBuf, u64>>>,
) {
let files = loaded_files.borrow();
let excluded_set = excluded.borrow();
let sizes = file_sizes.borrow();
let count = files.len();
let included_count = files.iter().filter(|p| !excluded_set.contains(*p)).count();
let total_size: u64 = files.iter()
.filter(|p| !excluded_set.contains(*p))
.filter_map(|p| std::fs::metadata(p).ok())
.map(|m| m.len())
.map(|p| sizes.get(p).copied().unwrap_or(0))
.sum();
let size_str = format_size(total_size);
update_heading_label(widget, count, included_count, &size_str);
@@ -531,6 +550,9 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
.spacing(0)
.build();
// Guard flag to prevent O(n^2) update cascade during batch checkbox operations
let batch_updating: Rc<std::cell::Cell<bool>> = Rc::new(std::cell::Cell::new(false));
// Toolbar
let toolbar = gtk::Box::builder()
.orientation(gtk::Orientation::Horizontal)
@@ -675,6 +697,8 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
{
let excluded = state.excluded_files.clone();
let loaded = state.loaded_files.clone();
let cached_sizes = state.file_sizes.clone();
let batch_flag = batch_updating.clone();
factory.connect_bind(move |_factory, list_item| {
let Some(list_item) = list_item.downcast_ref::<gtk::ListItem>() else { return };
let Some(item) = list_item.item().and_downcast::<ImageItem>() else { return };
@@ -733,6 +757,8 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
// Wire checkbox toggle
let excl = excluded.clone();
let loaded_ref = loaded.clone();
let sizes_ref = cached_sizes.clone();
let batch_guard = batch_flag.clone();
let file_path = path.clone();
let handler_id = check.connect_toggled(move |btn| {
{
@@ -743,12 +769,16 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
excl.insert(file_path.clone());
}
}
// Skip per-toggle label update during batch select/deselect
if batch_guard.get() {
return;
}
// Update count label
if let Some(parent) = btn.ancestor(gtk::Stack::static_type())
&& let Some(stack) = parent.downcast_ref::<gtk::Stack>()
&& let Some(loaded_widget) = stack.child_by_name("loaded")
{
update_count_label(&loaded_widget, &loaded_ref, &excl);
update_count_label(&loaded_widget, &loaded_ref, &excl, &sizes_ref);
}
});
// Store handler id so we can disconnect in unbind
@@ -817,14 +847,18 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
{
let excl = state.excluded_files.clone();
let loaded = state.loaded_files.clone();
let sizes = state.file_sizes.clone();
let batch_flag = batch_updating.clone();
select_all_button.connect_clicked(move |btn| {
excl.borrow_mut().clear();
if let Some(parent) = btn.ancestor(gtk::Stack::static_type())
&& let Some(stack) = parent.downcast_ref::<gtk::Stack>()
&& let Some(loaded_widget) = stack.child_by_name("loaded")
{
batch_flag.set(true);
set_all_checkboxes_in(&loaded_widget, true);
update_count_label(&loaded_widget, &loaded, &excl);
batch_flag.set(false);
update_count_label(&loaded_widget, &loaded, &excl, &sizes);
}
});
}
@@ -833,6 +867,8 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
{
let excl = state.excluded_files.clone();
let loaded = state.loaded_files.clone();
let sizes = state.file_sizes.clone();
let batch_flag = batch_updating.clone();
deselect_all_button.connect_clicked(move |btn| {
{
let files = loaded.borrow();
@@ -845,8 +881,10 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
&& let Some(stack) = parent.downcast_ref::<gtk::Stack>()
&& let Some(loaded_widget) = stack.child_by_name("loaded")
{
batch_flag.set(true);
set_all_checkboxes_in(&loaded_widget, false);
update_count_label(&loaded_widget, &loaded, &excl);
batch_flag.set(false);
update_count_label(&loaded_widget, &loaded, &excl, &sizes);
}
});
}
@@ -930,7 +968,22 @@ fn download_image_url(url: &str) -> Option<std::path::PathBuf> {
.unwrap_or("downloaded.jpg");
let filename = if sanitized.is_empty() { "downloaded.jpg" } else { sanitized };
let dest = temp_dir.join(filename);
// Avoid overwriting previous downloads with the same filename
let dest = {
let base = temp_dir.join(filename);
if base.exists() {
let stem = std::path::Path::new(filename).file_stem()
.and_then(|s| s.to_str()).unwrap_or("downloaded");
let ext = std::path::Path::new(filename).extension()
.and_then(|e| e.to_str()).unwrap_or("jpg");
let ts = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_millis()).unwrap_or(0);
temp_dir.join(format!("{}_{}.{}", stem, ts, ext))
} else {
base
}
};
// Use GIO for the download (synchronous, runs in background thread)
let gfile = gtk::gio::File::for_uri(url);