feat: add archive support (zip, tar, 7z)#63
Merged
Merged
Conversation
- Core archive module: list, extract, create for ZIP, TAR (GZ/BZ2/XZ/ZST), 7Z - Security: path traversal prevention, symlink skip, OOM protection (temp files for XZ), setuid stripping - UI: F7 extract/create dialog, F12 archive menu, Enter/F3 preview archives in viewer - Viewer: archives displayed as formatted text listing (name, size, date) instead of hex - Batch operations with progress and cancellation - Dependencies: zip 8.6, bzip2 0.6, sevenz-rust 0.6, tar 0.4, flate2, zstd, lzma-rs
Reviewer's GuideImplements a new archive subsystem (ZIP, TAR* and 7z) with list/extract/create operations, wires it into the batch job engine and UI (dialogs, pickers, keybindings, viewer), and documents the feature and new dependencies, including safety measures against path traversal, symlinks, and unsafe permissions. Sequence diagram for archive extraction via F7 dialogsequenceDiagram
actor User
participant NormalInput as handle_f7_key
participant AppState
participant Dialog as handle_archive_extract_dialog
participant Batch as execute_batch_with_byte_progress
participant Ops as batch_extract_archive
participant Archive as archive::extract::extract_archive
User->>NormalInput: press_F7
NormalInput->>AppState: show_archive_dialog()
AppState->>AppState: state.mode = DialogKind::ArchiveExtract
User->>Dialog: Enter (confirm extract)
Dialog->>AppState: PendingAction::ExtractArchive
Dialog->>Batch: start_confirmed_action()
Batch->>Batch: execute_batch_with_byte_progress()
Batch->>Ops: batch_extract_archive()
Ops->>Archive: archive::extract::extract_archive(source, dest, progress_tx, cancel)
Archive-->>Ops: Result<(), ArchiveError>
Ops-->>Batch: BatchReport
Batch-->>AppState: update status/progress
Sequence diagram for archive preview in viewersequenceDiagram
actor User
participant NormalInput as handle_enter_key
participant ViewerLoader as ViewerState::open_background
participant ViewerState as ViewerState::open
participant ArchiveList as archive::list::list_archive
User->>NormalInput: Enter on archive
NormalInput->>ViewerLoader: ViewerState::open_background(path)
ViewerLoader->>ViewerState: open(path)
ViewerState->>ViewerState: should_open_as_text(...)
ViewerState->>ViewerState: open_as_archive_listing(path, file_size)
ViewerState->>ViewerState: format_archive_listing(path)
ViewerState->>ArchiveList: list_archive(path)
ArchiveList-->>ViewerState: Vec<ArchiveEntry>
ViewerState-->>ViewerLoader: ViewerState { view_mode = Text }
ViewerLoader-->>User: archive listing displayed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
archive_format_from_paththe extension handling is narrower than elsewhere (e.g.detect_format) and the README (e.g..tbzis supported by detection/docs but not accepted for creation), so it would be good to align all three to the same set of extensions to avoid user-visible inconsistencies. - The temp file names for TAR+XZ (
lc-xz-tar-<pid>andlc-xz-decompress-<pid>) are fixed per process and not per operation; consider incorporating a random suffix or using a proper temp file API to avoid collisions between concurrent archive operations and to reduce the risk of races. - Creating a
.7zarchive currently goes throughArchiveFormat::SevenZand ends increate_7zreturningUnsupportedFormat; it may be clearer to either prevent.7zfrom being offered as a creatable format in the dialog or surface a more specific error so users understand that 7z writing is not implemented.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `archive_format_from_path` the extension handling is narrower than elsewhere (e.g. `detect_format`) and the README (e.g. `.tbz` is supported by detection/docs but not accepted for creation), so it would be good to align all three to the same set of extensions to avoid user-visible inconsistencies.
- The temp file names for TAR+XZ (`lc-xz-tar-<pid>` and `lc-xz-decompress-<pid>`) are fixed per process and not per operation; consider incorporating a random suffix or using a proper temp file API to avoid collisions between concurrent archive operations and to reduce the risk of races.
- Creating a `.7z` archive currently goes through `ArchiveFormat::SevenZ` and ends in `create_7z` returning `UnsupportedFormat`; it may be clearer to either prevent `.7z` from being offered as a creatable format in the dialog or surface a more specific error so users understand that 7z writing is not implemented.
## Individual Comments
### Comment 1
<location path="src/input/dialogs.rs" line_range="496-505" />
<code_context>
+fn handle_archive_extract_dialog(
</code_context>
<issue_to_address>
**issue (bug_risk):** Archive extract dialog ignores button selection and always treats Enter as "OK".
This dialog relies on `state.dialog_selection` to visually switch between `[ OK ]` and `[ Cancel ]`, but `handle_archive_extract_dialog` never updates `dialog_selection` on arrow/tab keys and always treats `KeyCode::Enter` as "OK". That makes `[ Cancel ]` effectively unreachable except via `Esc`. Please update `dialog_selection` on left/right/Tab and branch on the selected button when handling Enter so that `Cancel` actually cancels, consistent with other dialogs.
</issue_to_address>
### Comment 2
<location path="src/input/dialogs.rs" line_range="537-533" />
<code_context>
+fn handle_archive_create_dialog(
</code_context>
<issue_to_address>
**issue (bug_risk):** Archive create dialog has the same selection/Enter behavior issue as the extract dialog.
As with the extract dialog, `handle_archive_create_dialog` ignores `state.dialog_selection` on `KeyCode::Enter`, so `[ Cancel ]` can only be triggered with `Esc`. Please align this with the other dialogs so Enter activates the currently selected button and the action matches the highlighted option.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive archive support to Libre Commander, enabling users to browse, extract, and create various archive formats (ZIP, TAR/GZ/BZ2/XZ/ZST, and 7Z) directly from the dual-panel interface. Key review feedback focuses on critical bugs and security vulnerabilities: a compilation error in the ZIP date-time conversion where is_multiple_of is used on a primitive integer, malformed ZIP path separators on Windows, and potential path traversal vulnerabilities if absolute paths are present in archive entries. Additionally, using fs::remove_dir_all on extraction failure could lead to accidental data loss of pre-existing user files, and concurrent operations could trigger race conditions or leak temporary files on disk due to non-unique temporary filenames.
Greptile SummaryAdds a complete archive subsystem — ZIP, TAR (gz/bz2/xz/zst), and 7Z — with list, extract, and create operations, F7/F12 key bindings, an extract/create dialog, and archive-listing in the viewer.
Confidence Score: 3/5Not safe to merge as-is: the extraction rollback deletes pre-existing user directories, the archive dialog freezes the TUI for large .tar.xz files, and concurrent XZ operations corrupt each other's temp files. Three independent defects affect data integrity or responsiveness on the changed paths. The extraction cleanup calls src/ops/archive/tar.rs, src/ops/archive/zip.rs, and src/ops/archive/sevenz.rs for the cleanup/rollback logic; src/input/normal.rs for the blocking listing call. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant InputHandler as input/normal.rs
participant Batch as ops/batch.rs
participant Archive as ops/archive/*
participant TempFS as Temp Filesystem
participant UI as ui/dialogs + viewer
User->>InputHandler: F7 (on archive) / F12
InputHandler->>Archive: list_archive() [BLOCKING on main thread]
Archive->>TempFS: decompress .xz to PID-named temp file
TempFS-->>Archive: entries
Archive-->>InputHandler: "Vec<ArchiveEntry>"
InputHandler->>UI: show ArchiveExtract/Create dialog
User->>UI: confirm extract/create
UI->>Batch: PendingAction::ExtractArchive / CreateArchive
Batch->>Batch: thread::scope (background thread)
Batch->>Archive: extract_archive / create_archive
Archive->>TempFS: write to PID-named temp (xz only)
Archive-->>Batch: Result
Batch-->>UI: BatchReport + progress updates
Note over InputHandler,Archive: list_archive blocks UI thread for .tar.xz
Note over Archive,TempFS: PID-only temp name collides on concurrent ops
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Fix OOM: XZ decompress/create use temp files instead of in-memory buffers - Fix silent TarGz fallback for unsupported formats (.rar, .deb) — now errors - Fix F5 inactive-panel archive branch — removed confusing behavior - Add dialog selection support (Left/Right toggle, Enter checks selection) - Change remove_dir_all to remove_dir in extraction cleanup (3 extractors) - Add atomic temp counter for unique filenames (AtomicUsize) - Add absolute path rejection in all 3 sanitize/validate functions - Add !entry.is_dir() checks in F7/F12/Enter handlers - Add Windows backslash normalization in zip create paths - Improve 7z creation error message (InvalidArchive instead of UnsupportedFormat) - Deduplicate apply_text_edit to use apply_dialog_text_edit - Remove all #[allow(clippy::too_many_lines/arguments)] suppressions - Remove dead has_non_dotdot check, restore accidentally deleted create_tar
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add core archive support (ZIP, TAR variants, 7z) with integrated UI dialogs, viewer preview, and batch operations.
New Features:
Enhancements:
Build:
Documentation: