File Manager: Add Compress & Extract archive support#2630
Draft
mgutt wants to merge 132 commits intounraid:masterfrom
Draft
File Manager: Add Compress & Extract archive support#2630mgutt wants to merge 132 commits intounraid:masterfrom
mgutt wants to merge 132 commits intounraid:masterfrom
Conversation
- Add Compress option to folder and file context menus - Add Extract option to archive files (zip, tar, gz, etc.) - Create compress/extract dialog templates in Templates.php - Implement doAction cases 16 (compress) and 17 (extract) - Add auto-detection for archive format based on path (zstd for appdata, zip for shares) - Add updateArchiveName() helper to sync archive name with format selection - Add validation for compress/extract actions in Start button handler
…lementation - Control.php: Add format and archive_name parameters for compress action - file_manager: Implement case 16 (compress) with support for zip, tar, tar.gz, tar.zst - file_manager: Implement case 17 (extract) with auto-detection of archive type - Support for multiple archive formats: zip, tar, tar.gz, tar.bz2, tar.xz, tar.zst, 7z, rar - Add progress tracking via tee to status file - Add warning texts for compress/extract actions in Browse.page
…button, add debug logging
…cross extract calls, add debug logging
- Implement parse_zip_progress() to parse structured log format - Create zip_wrapper script for real-time zip progress parsing - Integrate zip_wrapper into compress case 16 - Parse DOT|timestamp entries to calculate speed and ETA - Display progress: Completed: X%, Archive: YMB, Speed: ZMB/s, ETA: H:MM:SS - Keep tar formats using simple status display for now
- Remove unused $format parameter from zip_wrapper - Fix shellcheck redirect warning (use : > instead of >) - Use pipe-safe format: FILE|size|filename (filename at end) - Add proper PHPDoc comment for parse_zip_progress - Update PHP parser to read filename from third field
- Change from pipeline to direct redirect - Avoids PID mismatch (pipeline returns wrong PID) - zip_wrapper logs to /var/tmp/zip.progress independently
…ip -> TEST-2.zip)
…add debug logging to archive name selection
…ps for speed calculation, fix regex to match end of line
…acter-by-character dot streaming
…may break PID tracking temporarily)
… needs AJAX or server-side implementation)
…for line end (dots come on same line)
- zip: add -ds as short option for --dot-size - tar: add -b/-bN/--blocking-factor[=N] short option support - tar extract TOTAL: replace extension-based listing+stat-poller with 'timeout 5 tar -tvf' — works for all formats including compressed; TOTAL is skipped silently if listing times out - remove tar stat-poller branch (no longer needed) - fix stray chars in comments, use generic example values
timeout's exit code is lost in a pipeline; capture output in a variable first, check exit code, then sum with awk only on success
unzip builds differ: Debian uses YYYY-MM-DD, Info-ZIP (Unraid) uses MM-DD-YYYY. LC_ALL=C does not affect the date format. Accept both via alternation in the regex. BASH_REMATCH[2] is now the date group; filename moved to BASH_REMATCH[3].
stat-poller: limit wait for file appearance to 15s. Without -^, unzip strips control characters from filenames on extraction (e.g. foo^Jbar becomes foobar), so the manifest path never appears on disk and the poller would loop forever. Document unzip -^ as required option with explanation.
…st order
Old design (stat-poller) iterated fname_to_size[] in manifest order and waited
up to 75×0.2s for each file to appear. Since unzip extracts in its own order
this caused arbitrary delays and on atum a full hang (pipe filled, unzip blocked,
stat-poller waited forever for a file that could never be extracted).
New design:
- Read unzip stdout char-by-char (same as zip/tar branch); check -f on the
growing candidate path. Fires as soon as open_outfile() creates the file —
64µs before any data is written (confirmed via strace). No timing/sleep.
- Use -f not -e: intermediate dir components are -f false, so partial paths
like /tmp or /dest/subdir don't trigger early.
- Once -f fires, poll stat -c%s every 0.2s until size stops growing.
- Replace parallel file_names[]/file_sizes[] arrays with assoc array
fname_to_size for O(1) size lookup by filename.
- Remove cat >/dev/null & drain (no longer needed).
- Remove run_stat_poller() entirely.
- Handle all three -d variants correctly:
no -d: stdout relative to compress_cwd
-d /abs: stdout absolute
-d rel: stdout relative to compress_cwd
In all cases fpath is resolved to absolute before stat.
- Add _sleep_cmd=$(command -v sleep) to avoid CWD probe (./sleep ENOENT)
when progress CWD is inside the extraction destination.
…collision
- Trigger only when $char is a space: unzip's format string is '%-22s ...'
so spaces always follow the filename before the \n arrives. This prevents
matching a filename prefix ('file' firing while reading 'filename') because
there is no space inside 'filename' until the field ends.
- Require exact fname_to_size key match as second guard for filenames with
internal spaces that could still produce a space-delimited prefix collision.
- Document control-char limitation: unzip -l and stdout both use FnFilter1
(caret notation), so -f check fails for real control-char filenames on disk;
FILE line still appears, no CHUNK emitted.
The space-trigger approach still fails when two files share a common space-delimited prefix (e.g. 'foo' and 'foo bar.txt' both in the archive). The exact manifest match guard does not help in that case. New approach: read -t 0.1 with dual triggers. - Timeout (rc > 128, 100ms silence): unzip wrote the filename line atomically in one write(2) syscall, then paused to write the file. Buffer holds the complete filename. Emit FILE + poll growth. - \n arrival (rc == 0, empty char): file write done. Catches small/fast files that complete before the 100ms timeout fires. Emit FILE + one CHUNK (fsize). - processed_files assoc array prevents double-processing if both paths fire. The atomic write syscall guarantee is the key property: timeout never fires mid-filename, so the buffer always holds the full line when we evaluate it.
…filenames
unzip reader:
- Include directory entries (creating:) in fname_to_size (size 0);
eliminates the old creating:/keyword special-case.
- Drop [[:space:]]{2} guard from regex (redundant: trailing-space strip
handles padding; creating: lines have no trailing spaces anyway).
- Add stall guard: on timeout-path regex miss, if char_buf hasn't grown
since the last failed check → bail with Error: to avoid spinning forever.
- Replace processed_files assoc array with already_handled bool.
- Remove -f check from both paths: trust unzip stdout as authoritative.
- \n path: emit FILE directly from manifest, no CHUNK needed.
- Timeout path: add Error: message on manifest miss (fsize=0 fallback).
- Comment: document fpath absolute/relative decision (with/without -d).
- Comment: note FnFilter1 caret notation behaviour for control-char names.
tar reader:
- Apply printf '%b' to each filename before stat: GNU tar escape-quotes
special chars by default (\n -> newline etc.), so the raw line_buffer
does not match the on-disk filename without unescaping.
_prog_chunk_line was resetting _prog_chunk_t/_prog_chunk_done after every render, making dt < 0.05s on the very next call — new_speed stayed -1 and speed/ETA always showed '--'. Fix: remove the reset so _prog_chunk_t stays at the TOTAL baseline; speed is computed once enough time (>0.05s) has passed. _prog_done_line now forces _prog_done=_prog_total and calls _prog_render before the final newline so the bar always reaches 100% even when tar/zip skip the checkpoint for the last partial block. Add --progress flag to force show_progress=1 even when stderr is not a TTY (useful for testing and redirected/piped runs). Also: refactor option parsing from while-comparisons to case; replace awk forks in _prog_fmt_bytes with pure bash arithmetic; simplify _prog_render awk call and _prog_total_line bar-width calculation.
Integer-second timestamps caused speed to snap to discrete multiples (100MB/1s=100MB/s, 100MB/2s=50MB/s, ...). EPOCHREALTIME provides microsecond resolution for accurate speed calculation in file_manager.
When pid is empty (new action about to start), publish an empty status immediately and reset last_published_status. nchan buffers the last message for 30s, causing clients to briefly see the previous operation's final status when a new one starts.
- zip -y: store symlinks as symlinks instead of skipping them (broken and cross-device symlinks were silently dropped without -y) - LANG=en_US.UTF-8 for zip and unzip: Unraid runs with POSIX locale, causing non-ASCII filenames (e.g. Chinese/Arabic) to be mangled as #Uxxxx escape sequences; UTF-8 locale preserves them correctly
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/scripts/progress (2)
568-574: 💤 Low valueGuard
statagainst filenames starting with-.
stat -c%s "$fname"will treat a filename like-brokenas an option. With GNU tar's default escape quoting it's uncommon but legal. Adding a--separator (and the same on Line 506) avoids surprises:- fsize=$(cd "$compress_cwd" && stat -c%s "$fname" 2>/dev/null) + fsize=$(cd "$compress_cwd" && stat -c%s -- "$fname" 2>/dev/null)- cur_size=$(stat -c%s "$fpath" 2>/dev/null || echo 0) + cur_size=$(stat -c%s -- "$fpath" 2>/dev/null || echo 0)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@emhttp/plugins/dynamix/scripts/progress` around lines 568 - 574, The stat invocation can misinterpret filenames starting with "-" as options; update the calls that compute fsize (and the earlier stat usage around the same block referenced in the comment) to pass a "--" separator before the filename so stat treats the value in fname as a path, not an option; locate the lines using stat -c%s "$fname" (and the related earlier occurrence) and add the "--" argument, leaving the surrounding logic that sets fname, computes fsize, calls _prog_file_line and increments file_count unchanged.
446-446: 💤 Low valueTrailing slash in
-dargument breaks the destination prefix strip.
${candidate#"$unzip_dest"/}always appends a/to the prefix. If the user passesunzip -d /tmp/,unzip_destbecomes/tmp/and the strip pattern becomes/tmp//, which never matches the actual/tmp/file.txtlines emitted by unzip. The result:fnamekeeps the absolute prefix, the manifest lookup misses,fsizefalls back to 0, and theCHUNKpoll path at Line 506 stats a wrong-relativefpathand emits no growth progress.Normalize
unzip_destonce after parsing (strip a trailing/, except when it's the lone root) so both stripping sites stay correct.🔧 Proposed normalization
dbg "unzip dest=$unzip_dest" + # Normalize: strip trailing slash so "${candidate#"$unzip_dest"/}" works for `-d /tmp/` too. + [[ ${`#unzip_dest`} -gt 1 && $unzip_dest == */ ]] && unzip_dest=${unzip_dest%/}Also applies to: 491-492
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@emhttp/plugins/dynamix/scripts/progress` at line 446, The unzip destination variable unzip_dest should be normalized immediately after parsing to remove any trailing slash except when it's the lone root ("/") so the prefix-strip patterns like fname="${candidate#"$unzip_dest"/}" and the other strip at lines ~491-492 match actual paths; update the code that sets unzip_dest to trim a trailing '/' (but leave "/" intact) and then keep all existing uses (e.g., the fname assignment and the other candidate stripping) unchanged so the parameter expansion correctly removes the destination prefix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@emhttp/plugins/dynamix/scripts/progress`:
- Around line 274-275: The unzip and zip/tar compress paths emit "TOTAL 0" even
when size is unknown; update those emission sites to only print TOTAL and call
_prog_total_line when total_bytes is greater than zero (i.e., guard the printf
'TOTAL %s' "$total_bytes" and the subsequent _prog_total_line "$total_bytes"
calls with a test total_bytes -gt 0) so behaviour matches the tar-extract branch
and downstream parse_compress_progress sees "unknown" instead of a zero total.
- Line 451: The printf format string currently uses an unescaped \n in the label
so it becomes an actual newline; update the printf call that prints "Error:
unzip manifest miss (\n path): %s\n" to escape the backslash in the label (so
the literal "\n" appears in the output, e.g. change "(\n path)" to "(\\n path)")
and leave the final "%s\n" unchanged; this keeps the diagnostic on one line
while preserving the filename substitution.
---
Nitpick comments:
In `@emhttp/plugins/dynamix/scripts/progress`:
- Around line 568-574: The stat invocation can misinterpret filenames starting
with "-" as options; update the calls that compute fsize (and the earlier stat
usage around the same block referenced in the comment) to pass a "--" separator
before the filename so stat treats the value in fname as a path, not an option;
locate the lines using stat -c%s "$fname" (and the related earlier occurrence)
and add the "--" argument, leaving the surrounding logic that sets fname,
computes fsize, calls _prog_file_line and increments file_count unchanged.
- Line 446: The unzip destination variable unzip_dest should be normalized
immediately after parsing to remove any trailing slash except when it's the lone
root ("/") so the prefix-strip patterns like fname="${candidate#"$unzip_dest"/}"
and the other strip at lines ~491-492 match actual paths; update the code that
sets unzip_dest to trim a trailing '/' (but leave "/" intact) and then keep all
existing uses (e.g., the fname assignment and the other candidate stripping)
unchanged so the parameter expansion correctly removes the destination prefix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e55e8505-dcd5-4fc4-bc03-1b6d932651ed
📒 Files selected for processing (4)
emhttp/plugins/dynamix/Browse.pageemhttp/plugins/dynamix/BrowseButton.pageemhttp/plugins/dynamix/nchan/file_manageremhttp/plugins/dynamix/scripts/progress
🚧 Files skipped from review as they are similar to previous changes (3)
- emhttp/plugins/dynamix/BrowseButton.page
- emhttp/plugins/dynamix/nchan/file_manager
- emhttp/plugins/dynamix/Browse.page
Unraid uses POSIX/C locale by default. Without LANG=en_US.UTF-8, unzip -l escapes non-ASCII filenames as #Uxxxx sequences while the extraction stdout (called with LANG=en_US.UTF-8 in file_manager) emits real UTF-8, causing manifest lookup mismatches for non-ASCII filenames. Fix: hardcode LANG=en_US.UTF-8 for 'unzip -l' in progress so both sides always use the same locale, independent of how the caller invokes the script. Also: if a manifest miss occurs and the filename contains ^ or #U (escape sequences from unzip running in C locale), emit a hint pointing to the missing LANG=en_US.UTF-8 on the unzip invocation side.
- TOTAL 0 guard: unzip and zip/tar compress paths now only emit TOTAL when total_bytes > 0, consistent with the tar-extract path. Avoids downstream consumers treating TOTAL 0 as a known-zero size on empty/failed manifests. - unzip_dest normalization: strip trailing slash after parsing -d argument (except bare '/') so the candidate prefix-strip always matches the actual path emitted by unzip, even when the caller passes e.g. unzip -d /tmp/. - printf escape: '(\n path)' label in manifest-miss error used a real newline; escaped to '(\n path)' so the message stays on one line. - stat -- guard: both stat calls now use '--' to prevent filenames starting with '-' from being interpreted as options.
Compress dialog: - Add 'Overwrite existing archive' checkbox (dfm_exist): skips the archive-already-exists check when checked. Uses the same dfm_exist field already handled by extract and copy/move dialogs. BrowseButton.page: - Guard dfm_read.source undefined in dfm_makeDialog: if mode:'read' returns data without a source field (e.g. timing race during active operation), the previous split() call threw a TypeError that silently aborted dialog rendering mid-way. Now falls back to empty string.
Add support for compressing a single file (no folder) to a bare compressed format without a tar wrapper: - gz -> gzip -c -- <file> > archive.gz - xz -> xz -c -- <file> > archive.xz - bz2 -> bzip2 -c -- <file> > archive.bz2 - zst -> zstd -c -- <file> > archive.zst - lz4 -> lz4 -c -- <file> > archive.lz4 Frontend: option elements with class dfm_single_format are removed from the format dropdown when the selection contains more than one item or any folder (trailing slash). If the previously selected format was a single-file format and now becomes unavailable, the dropdown resets to zip or tar.zst (for appdata paths). Backend: new switch cases in file_manager case 16 set $single_compressor; the command pipes output through the progress script for consistent status.
…p detects it When a small archive (e.g. tar.zst with one file) completes before progress can detect the compress process via pgrep, the script previously emitted 'Error: no compress process found' and exited 1. Fallback: buffer all stdin via mapfile, detect tool from first line pattern, parse filenames per tool, collect sizes via stat, emit TOTAL+FILE and exit 0. Tool detection: - unzip: first line starts with 'Archive:' - zip: first line matches '^ (adding|updating):' - tar: fallback (no distinctive first line) For unzip, the dest prefix is stripped from fname (FILE output) the same way as in the live reader. In the fallback compress_cwd=PWD=unzip_dest, so fname is stat-able directly from CWD - no separate fpath needed. Also documents that CWD should be source/dest dir when running the pipeline, since progress uses CWD to locate files via stat.
The dfm_single_format filter logic was only added to doAction (three-dot menu on a single item) but was missing from doActions (toolbar compress button with multiple items selected). Add the same guard to doActions case 16: remove .dfm_single_format options unless exactly one file (not a directory, type !== 'd') is selected.
Control.php: store overwrite as int (0/1) instead of rsync flag string. Before: exist => --ignore-existing or empty string After: overwrite => 0 or 1 file_manager: $exist -> $overwrite throughout; derive rsync flags from bool. - copy/move: $ignore_existing = $overwrite ? "" : "--ignore-existing" - compress: !$overwrite check (fix: was inverted !empty($exist)) - extract zip: $overwrite ? "-o" : "-n" - extract tar: $overwrite ? "" : "--skip-old-files ..." Removed pointless $no_overwrite helper var. Templates.php: dfm_exist -> dfm_overwrite in all templates. Also fix: compress template used :: (double-colon) for overwrite wrapper, rendering visibly as :: in UI. Fixed to :. Browse.page/BrowseButton.page: dfm_exist -> dfm_overwrite; post key exist -> overwrite; restore logic dfm_read.exist?false:true -> !!dfm_read.overwrite. BrowseButton.css: input#dfm_exist -> input#dfm_overwrite.
….bz2/.xz/.zst/.lz4) Browse.page: isArchive() regex now includes .gz/.bz2/.xz/.zst/.lz4 so the Extract toolbar button activates when exactly one such file is selected. file_manager: case 17 (extract) handles single-file decompressors via gunzip/bunzip2/unxz/unzstd/unlz4 with -c pipe. Uses same tmp+rename pattern as compress: writes to hidden .tmp file, renames to final name on success, cleans up on error/cancel. Sets chown/chmod (nobody:users or root:root for Docker paths) identical to compress post-processing. Manual overwrite check: returns error if dest file exists and overwrite=0.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/BrowseButton.page (1)
429-438: ⚡ Quick winRe-enable
#dfm_formatand#dfm_archive_nameon error so the compress dialog stays usable.After a backend error (e.g.,
Archive already exists),#dfm_target/#dfm_overwrite/#dfm_sparse/ owner-group-other inputs are restored, but#dfm_formatand#dfm_archive_name(disabled at lines 362-363 when the dialog opens) remain locked. Users who want to retry with a different name/format have to close and reopen the dialog.♻️ Suggested change
dfm.window.find('#dfm_target').prop('disabled',false); dfm.window.find('#dfm_sparse').prop('disabled',false); dfm.window.find('#dfm_overwrite').prop('disabled',false); dfm.window.find('#dfm_owner').prop('disabled',false); dfm.window.find('#dfm_group').prop('disabled',false); dfm.window.find('#dfm_other').prop('disabled',false); + dfm.window.find('#dfm_format').prop('disabled',false); + dfm.window.find('#dfm_archive_name').prop('disabled',false); $('.ui-dfm .ui-dialog-buttonset button:lt(2)').prop('disabled',false);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@emhttp/plugins/dynamix/BrowseButton.page` around lines 429 - 438, The error handler currently re-enables many inputs but forgets to re-enable the archive format and name fields; inside the same error block where dfm.window.find('#dfm_target'), '#dfm_sparse', '#dfm_overwrite', '#dfm_owner', '#dfm_group', and '#dfm_other' are re-enabled (and after calling dfm_fileManager('stop')), also call dfm.window.find('#dfm_format').prop('disabled',false) and dfm.window.find('#dfm_archive_name').prop('disabled',false) so users can change format/name and retry without closing the dialog.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@emhttp/plugins/dynamix/Browse.page`:
- Around line 175-180: The isArchive check is using $('#'+id).attr('data') where
id is the name element (name_X) so fileName is undefined; change the lookup to
use the row id via id.dfm_proxy() (e.g. call $('#'+id.dfm_proxy()).attr('data'))
so isArchive receives the actual filename and the Extract option will appear;
update the fileName assignment in the block that builds opts (near the opts.push
for Compress and the isArchive branch) to use id.dfm_proxy() and leave the rest
(doAction/opts.push) unchanged.
In `@emhttp/plugins/dynamix/nchan/file_manager`:
- Around line 984-997: The code builds $source_path, $source_parent and
$source_basenames from $source[0] without validating the incoming path; add the
same server-side validation used for copy/move/delete (the validname() check
used around the copy/move logic) before computing dirname/basename and before
accepting $source[0] as an $archive (case 16 and case 17). Specifically,
validate $source[0] is under the allowed roots (/mnt or /boot) and reject or
return an error if not, then proceed to compute $source_path, $source_parent,
$source_basenames and escape them as before.
- Around line 1283-1292: The current gating uses filesize($error) to decide
whether to rename $target_tmp to $archive_path, which can falsely treat benign
stderr warnings as failures and unlink a valid archive; update the check in the
compress/extract-single-file branch (the in_array($action, [16, 17]) block
handling $target_tmp/$error) to instead rely on the child process exit code when
available (preferably) or, if exit-code plumbing is not feasible in this change,
inspect $error contents and whitelist known-benign patterns (e.g., "warning:",
"file changed as we read it", etc.) before treating non-empty $error as fatal;
ensure rename($target_tmp, $archive_path) and subsequent exec('chown
...')/exec('chmod ...') run when the process exit code indicates success or
$error only matches whitelisted benign messages, and only unlink($target_tmp) on
real failures.
---
Nitpick comments:
In `@emhttp/plugins/dynamix/BrowseButton.page`:
- Around line 429-438: The error handler currently re-enables many inputs but
forgets to re-enable the archive format and name fields; inside the same error
block where dfm.window.find('#dfm_target'), '#dfm_sparse', '#dfm_overwrite',
'#dfm_owner', '#dfm_group', and '#dfm_other' are re-enabled (and after calling
dfm_fileManager('stop')), also call
dfm.window.find('#dfm_format').prop('disabled',false) and
dfm.window.find('#dfm_archive_name').prop('disabled',false) so users can change
format/name and retry without closing the dialog.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2f7a07d0-0138-4090-abbb-36b2eb0b9091
📒 Files selected for processing (7)
emhttp/plugins/dynamix/Browse.pageemhttp/plugins/dynamix/BrowseButton.pageemhttp/plugins/dynamix/include/Control.phpemhttp/plugins/dynamix/include/Templates.phpemhttp/plugins/dynamix/nchan/file_manageremhttp/plugins/dynamix/scripts/progressemhttp/plugins/dynamix/sheets/BrowseButton.css
🚧 Files skipped from review as they are similar to previous changes (1)
- emhttp/plugins/dynamix/include/Control.php
Comment on lines
+1283
to
+1292
| // For compress/extract-single-file: move tmp to final name + set permissions, or clean up on failure | ||
| if (in_array($action, [16, 17]) && isset($target_tmp)) { | ||
| if ((!file_exists($error) || filesize($error) === 0) && isset($archive_path, $archive_chown, $archive_chmod)) { | ||
| rename($target_tmp, $archive_path); | ||
| exec('chown ' . escapeshellarg($archive_chown) . ' ' . escapeshellarg($archive_path)); | ||
| exec('chmod ' . escapeshellarg($archive_chmod) . ' ' . escapeshellarg($archive_path)); | ||
| } else { | ||
| @unlink($target_tmp); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /root/repo && git ls-files | grep -i file_manager | head -20Repository: unraid/webgui
Length of output: 118
🏁 Script executed:
pwd && find . -name "file_manager" -type f 2>/dev/null | head -10Repository: unraid/webgui
Length of output: 120
🏁 Script executed:
wc -l ./emhttp/plugins/dynamix/nchan/file_managerRepository: unraid/webgui
Length of output: 106
🏁 Script executed:
sed -n '1200,1332p' ./emhttp/plugins/dynamix/nchan/file_manager | cat -nRepository: unraid/webgui
Length of output: 6724
🏁 Script executed:
sed -n '1,200p' ./emhttp/plugins/dynamix/nchan/file_manager | cat -nRepository: unraid/webgui
Length of output: 8563
🏁 Script executed:
sed -n '800,1100p' ./emhttp/plugins/dynamix/nchan/file_manager | cat -nRepository: unraid/webgui
Length of output: 15619
🏁 Script executed:
sed -n '1100,1300p' ./emhttp/plugins/dynamix/nchan/file_manager | cat -nRepository: unraid/webgui
Length of output: 11005
🏁 Script executed:
rg '\$\?' ./emhttp/plugins/dynamix/nchan/file_manager -A 2 -B 2Repository: unraid/webgui
Length of output: 39
🏁 Script executed:
rg 'exit' ./emhttp/plugins/dynamix/nchan/file_manager | head -20Repository: unraid/webgui
Length of output: 938
🏁 Script executed:
rg 'waitpid|wait\(' ./emhttp/plugins/dynamix/nchan/file_managerRepository: unraid/webgui
Length of output: 39
🏁 Script executed:
find . -name "progress" -type f 2>/dev/nullRepository: unraid/webgui
Length of output: 99
🏁 Script executed:
cat ./emhttp/plugins/dynamix/scripts/progressRepository: unraid/webgui
Length of output: 27163
🏁 Script executed:
rg 'parse_compress_progress|parse_extract' ./emhttp/plugins/dynamix/nchan/file_manager -B 2 -A 5 | head -40Repository: unraid/webgui
Length of output: 1916
🏁 Script executed:
rg 'exit.*code|filesize.*error' ./emhttp/plugins/dynamix/nchan/file_manager -B 3 -A 3Repository: unraid/webgui
Length of output: 601
🏁 Script executed:
rg 'wait|status|exit_code' ./emhttp/plugins/dynamix/nchan/file_manager | grep -v logger | grep -v '//' | head -20Repository: unraid/webgui
Length of output: 1577
🏁 Script executed:
rg 'waitpid|proc_status|wait' ./emhttp/plugins/dynamix/nchan/file_managerRepository: unraid/webgui
Length of output: 39
Gating the rename on an empty $error file can silently discard successful archives due to non-fatal warnings.
zip and tar write non-fatal diagnostics to stderr (e.g., zip warning: Not all files were readable, tar: file changed as we read it) while still producing a valid archive. Because 2>$error captures all stderr, any such message makes filesize($error) > 0, triggering the else branch and silently unlinking $target_tmp — discarding a successful archive.
A sturdier signal is the child process's exit code. If exit-code plumbing is not feasible for this PR, an interim mitigation is to whitelist known-benign warning patterns in the gating check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@emhttp/plugins/dynamix/nchan/file_manager` around lines 1283 - 1292, The
current gating uses filesize($error) to decide whether to rename $target_tmp to
$archive_path, which can falsely treat benign stderr warnings as failures and
unlink a valid archive; update the check in the compress/extract-single-file
branch (the in_array($action, [16, 17]) block handling $target_tmp/$error) to
instead rely on the child process exit code when available (preferably) or, if
exit-code plumbing is not feasible in this change, inspect $error contents and
whitelist known-benign patterns (e.g., "warning:", "file changed as we read it",
etc.) before treating non-empty $error as fatal; ensure rename($target_tmp,
$archive_path) and subsequent exec('chown ...')/exec('chmod ...') run when the
process exit code indicates success or $error only matches whitelisted benign
messages, and only unlink($target_tmp) on real failures.
After a backend error the compress dialog now re-enables #dfm_format and #dfm_archive_name alongside the other inputs, so the user can retry with a different name or format without reopening the dialog.
The data attribute containing the filename is on id=row_X, not id=name_X. Using the wrong element caused fileName to always be undefined, so isArchive() always returned false and the Extract menu item never appeared. Use id.dfm_proxy() consistent with the rest of the codebase.
case 16 (compress): validate all source entries via validname(); ensure all sources share the same parent directory to prevent mixing paths across filesystem boundaries in a single operation. case 17 (extract): validate archive path via validname() before use. Both cases now reject paths outside /mnt and /boot with 'Invalid source path', consistent with target path validation already in place.
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.
Closes #2511
Summary
Adds compress and extract functionality to the Unraid File Manager.
What's new
Compress
.gz,.xz,.bz2,.zst,.lz4when exactly one file (no folder) is selected/boot/config/docker.cfgto detect the user's appdata/docker paths - selects TAR.ZST for those paths (ideal for container backups), ZIP for everything else.gzetc.) show "Running..." only — no byte-level progress (tool writes directly to output file)Extract
.zip,.tar,.tar.gz/.tgz,.tar.bz2/.tbz2,.tar.xz/.txz,.tar.zst,.tar.lz4,.gz,.bz2,.xz,.zst,.lz4-n/-o, TAR uses--skip-old-files --warning=no-existing-file; single-file formats abort with "File already exists" error when overwrite is off.gzetc.) show "Running..." only — no byte-level progressPermissions
Created archives and new target directories get the following permissions:
chown nobody:users,chmod 666- accessible via SMBchown root:root,chmod 600- prevents accidental exposure via container webservers777 nobody:usersfor shares,755 root:rootfor docker paths/mnt/<disk>/prefix so it correctly matches on any disk or pool (e.g./mnt/user/appdata,/mnt/disk1/appdata,/mnt/cache/appdataall match)Progress implementation
All progress parsing is handled by a dedicated
progressscript (scripts/progress) that is piped directly from the compress/extract command. It detects the running tool (zip/tar/unzip) by inspecting/proc, parses its stdout, and emits a structured log to the status file:The PHP worker reads this file incrementally every 250 ms and derives %, speed, and ETA from it. CHUNK lines carry sub-second timestamps (
$EPOCHREALTIME) for accurate speed calculation - integer-second timestamps would cause speed to snap to discrete multiples (e.g. 100 MB/1s, 100 MB/2s).ZIP compress:
zip -r -du -dd -ds 100memits a(size)suffix per file followed by one dot per 100 MB. The progress script parses stdout char-by-char: detects the(size)suffix to emit aFILEline, then counts dots to emitCHUNKlines.TOTALis pre-calculated withdu -sbfrom the source file list in the zip command.ZIP extract:
unziponly prints a filename after each file is fully written, so byte-level progress cannot come from stdout. Instead, the progress script reads the manifest fromunzip -lupfront, then polls each output file withstatevery 0.2s to emitCHUNKlines proportional to growing file size. The script reads unzip stdout char-by-char with a 100ms read timeout: on timeout (file is being written) it starts polling; on newline (small file finished before timeout) it emits a FILE line directly.TOTALis the sum of all file sizes from the manifest.TAR compress: uses
--checkpoint=N --checkpoint-action=exec="printf 'CHUNK\n'"(1 checkpoint = 100 MB). The progress script parses tar's verbose filename output forFILElines and theCHUNKsentinel lines for progress.TOTALis pre-calculated withdu -sbfrom the source file list.TAR extract: same checkpoint mechanism for
CHUNKlines.TOTALis obtained by runningtar -tvfwith a 5-second timeout before the main extract starts - instant for uncompressed archives, usually finishes within the timeout for compressed ones. If the timeout is hit, progress runs without a total (shows bytes transferred + speed only, switches to % automatically once TOTAL arrives).Single-file formats (
.gz,.bz2,.xz,.zst,.lz4): no stdout-based progress available - the decompressor writes directly to the output file. The UI shows "Running..." for the duration and "Done" on completion.Security
archive_nameis validated server-side (basename()+ empty/dot check) to prevent writing outside the target directory/mnt,/boot) server-side - same as all other file manager operationsescapeshellarg(); heredoc interpolation was replaced with pre-escaped variablesOther
filemanager.json) - same as copy/move.tmpfile reliablyuniqid-based name with collision-check loop, stored in the active job JSON so cancel can locate it even after a page reloadLANG=en_US.UTF-8for both compress and extract - Unraid runs with POSIX locale which mangles non-ASCII filenames (e.g. Chinese, Arabic) as#Uxxxxescape sequences without this-yto store symlinks as symlinks rather than skipping them (broken symlinks and cross-device symlinks were silently dropped without this)--xattrs --xattrs-include='*.*' --acls --sparse: preserves extended attributes (all namespaces:user.*,security.*,trusted.*), POSIX ACLs, and sparse-file holes (important for VM images, container overlay filesystems, and databases)--numeric-owner: restores exact uid/gid numbers instead of doing a name-based lookup (critical when restoring to a system where numeric IDs differ from the archive)Implementation status
^Jnewline in filenames handled; overwrite flag-n/-o)-y→ stored as symlinks (broken + cross-device symlinks preserved); TAR stores symlinks by default$, newlines - tested with real-world dataLANG=en_US.UTF-8on zip/unzip prevents#Uxxxxmangling--xattrs --acls; tested withuser.author,user.comment)-rincludes directory entries → empty dirs preserved; verified withdiff -rq(no differences)uniqid('', true)with a collision-check loop as an additional safeguardchown nobody:users,chmod 666- accessible via SMBchown root:root,chmod 600- prevents accidental exposure via container webservers777 nobody:usersfor shares,755 root:rootfor docker paths/mnt/<disk>/prefix so it matches correctly on any disk or pool (same logic as the format auto-detection in Browse.page).tmpfile/mnt/disk4/...paths could silently overwrite data on a different disk if the disk layout changed); relative paths (the default tar/zip behavior) are always safe and sufficient.gz,.xz,.bz2,.zst,.lz4) — only shown in the format dropdown when exactly one file (no folder) is selected; no progress % yet (compressor writes directly to output file, no stdout-based progress)pvor stat-polling (to be implemented)