diff --git a/webui/backend/app/api/__pycache__/routes_files.cpython-313.pyc b/webui/backend/app/api/__pycache__/routes_files.cpython-313.pyc index c80d222..feb4169 100644 Binary files a/webui/backend/app/api/__pycache__/routes_files.cpython-313.pyc and b/webui/backend/app/api/__pycache__/routes_files.cpython-313.pyc differ diff --git a/webui/backend/app/api/routes_files.py b/webui/backend/app/api/routes_files.py index ad9c2dc..db2655d 100644 --- a/webui/backend/app/api/routes_files.py +++ b/webui/backend/app/api/routes_files.py @@ -1,7 +1,8 @@ from __future__ import annotations -from fastapi import APIRouter, Depends, File, Form, Request, UploadFile +from fastapi import APIRouter, Depends, File, Form, Query, Request, UploadFile from fastapi.responses import StreamingResponse +from starlette.background import BackgroundTask from backend.app.api.schemas import DeleteRequest, DeleteResponse, FileInfoResponse, MkdirRequest, MkdirResponse, RenameRequest, RenameResponse, SaveRequest, SaveResponse, UploadResponse, ViewResponse from backend.app.dependencies import get_file_ops_service @@ -63,15 +64,18 @@ async def info( @router.get("/download") async def download( - path: str, + path: list[str] = Query(...), service: FileOpsService = Depends(get_file_ops_service), ) -> StreamingResponse: - prepared = service.prepare_download(path=path) - return StreamingResponse( + prepared = service.prepare_download(paths=path) + response = StreamingResponse( prepared["content"], headers=prepared["headers"], media_type=prepared["content_type"], ) + if prepared.get("cleanup"): + response.background = BackgroundTask(prepared["cleanup"]) + return response @router.get("/video") diff --git a/webui/backend/app/services/__pycache__/file_ops_service.cpython-313.pyc b/webui/backend/app/services/__pycache__/file_ops_service.cpython-313.pyc index 1e16a31..9f23335 100644 Binary files a/webui/backend/app/services/__pycache__/file_ops_service.cpython-313.pyc and b/webui/backend/app/services/__pycache__/file_ops_service.cpython-313.pyc differ diff --git a/webui/backend/app/services/file_ops_service.py b/webui/backend/app/services/file_ops_service.py index 8b560a8..ef44c03 100644 --- a/webui/backend/app/services/file_ops_service.py +++ b/webui/backend/app/services/file_ops_service.py @@ -1,5 +1,9 @@ from __future__ import annotations +import os +from io import BytesIO +import zipfile +from datetime import datetime, timezone from pathlib import Path from backend.app.api.errors import AppError @@ -353,31 +357,18 @@ class FileOpsService: height=metadata["height"], ) - def prepare_download(self, path: str) -> dict: - resolved_target = self._path_guard.resolve_existing_path(path) - - if resolved_target.absolute.is_dir(): + def prepare_download(self, paths: list[str]) -> dict: + if not paths: raise AppError( - code="type_conflict", - message="Source must be a file", - status_code=409, - details={"path": resolved_target.relative}, - ) - if not resolved_target.absolute.is_file(): - raise AppError( - code="type_conflict", - message="Unsupported path type for download", - status_code=409, - details={"path": resolved_target.relative}, + code="invalid_request", + message="At least one path is required", + status_code=400, ) - return { - "content": self._filesystem.stream_file(resolved_target.absolute), - "headers": { - "Content-Disposition": f'attachment; filename="{resolved_target.absolute.name}"', - }, - "content_type": self._content_type_for(resolved_target.absolute) or "application/octet-stream", - } + resolved_targets = [self._path_guard.resolve_existing_path(path) for path in paths] + if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_file(): + return self._prepare_single_file_download(resolved_targets[0]) + return self._prepare_zip_download(resolved_targets) def save(self, path: str, content: str, expected_modified: str) -> SaveResponse: resolved_target = self._path_guard.resolve_existing_path(path) @@ -660,9 +651,104 @@ class FileOpsService: @staticmethod def _now_iso() -> str: - from datetime import datetime, timezone - return datetime.now(timezone.utc).replace(microsecond=0).isoformat().replace("+00:00", "Z") + + def _prepare_single_file_download(self, resolved_target) -> dict: + _, _, lexical_source, _ = self._path_guard.resolve_lexical_path(resolved_target.relative) + if lexical_source.is_symlink(): + raise AppError( + code="type_conflict", + message="Source must not be a symlink", + status_code=409, + details={"path": resolved_target.relative}, + ) + return { + "content": self._filesystem.stream_file(resolved_target.absolute), + "headers": { + "Content-Disposition": f'attachment; filename="{resolved_target.absolute.name}"', + }, + "content_type": self._content_type_for(resolved_target.absolute) or "application/octet-stream", + } + + def _prepare_zip_download(self, resolved_targets: list) -> dict: + archive_names: set[str] = set() + for resolved_target in resolved_targets: + self._validate_download_target(resolved_target) + archive_name = resolved_target.absolute.name + if archive_name in archive_names: + raise AppError( + code="invalid_request", + message="Selected items must have distinct top-level names", + status_code=400, + ) + archive_names.add(archive_name) + + if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_dir(): + download_name = f"{resolved_targets[0].absolute.name}.zip" + else: + download_name = f"kodidownload-{datetime.now(timezone.utc).strftime('%Y%m%d-%H%M%S')}.zip" + + buffer = BytesIO() + with zipfile.ZipFile(buffer, "w", compression=zipfile.ZIP_DEFLATED) as archive: + for resolved_target in resolved_targets: + self._write_download_target_to_zip(archive, resolved_target) + payload = buffer.getvalue() + + async def _stream_zip(): + yield payload + + return { + "content": _stream_zip(), + "headers": { + "Content-Disposition": f'attachment; filename="{download_name}"', + }, + "content_type": "application/zip", + } + + def _validate_download_target(self, resolved_target) -> None: + _, _, lexical_source, _ = self._path_guard.resolve_lexical_path(resolved_target.relative) + if lexical_source.is_symlink(): + raise AppError( + code="type_conflict", + message="Source must not be a symlink", + status_code=409, + details={"path": resolved_target.relative}, + ) + if resolved_target.absolute.is_file(): + return + if resolved_target.absolute.is_dir(): + for root, dirnames, filenames in os.walk(resolved_target.absolute, followlinks=False): + root_path = Path(root) + for name in [*dirnames, *filenames]: + entry = root_path / name + if entry.is_symlink(): + raise AppError( + code="type_conflict", + message="Source directory must not contain symlinks", + status_code=409, + details={"path": resolved_target.relative}, + ) + return + raise AppError( + code="type_conflict", + message="Unsupported path type for download", + status_code=409, + details={"path": resolved_target.relative}, + ) + + def _write_download_target_to_zip(self, archive: zipfile.ZipFile, resolved_target) -> None: + root_name = resolved_target.absolute.name + if resolved_target.absolute.is_file(): + archive.write(resolved_target.absolute, arcname=root_name) + return + + archive.writestr(f"{root_name}/", b"") + for child in sorted(resolved_target.absolute.rglob("*")): + arcname = f"{root_name}/{child.relative_to(resolved_target.absolute).as_posix()}" + if child.is_dir(): + archive.writestr(f"{arcname}/", b"") + else: + archive.write(child, arcname=arcname) @staticmethod def _parse_range_header(range_header: str, file_size: int) -> tuple[int, int]: def invalid_range() -> AppError: diff --git a/webui/backend/tests/golden/__pycache__/test_api_download_golden.cpython-313.pyc b/webui/backend/tests/golden/__pycache__/test_api_download_golden.cpython-313.pyc index 815dd44..82594ab 100644 Binary files a/webui/backend/tests/golden/__pycache__/test_api_download_golden.cpython-313.pyc and b/webui/backend/tests/golden/__pycache__/test_api_download_golden.cpython-313.pyc differ diff --git a/webui/backend/tests/golden/__pycache__/test_ui_smoke_golden.cpython-313.pyc b/webui/backend/tests/golden/__pycache__/test_ui_smoke_golden.cpython-313.pyc index 1317c6b..91b2992 100644 Binary files a/webui/backend/tests/golden/__pycache__/test_ui_smoke_golden.cpython-313.pyc and b/webui/backend/tests/golden/__pycache__/test_ui_smoke_golden.cpython-313.pyc differ diff --git a/webui/backend/tests/golden/test_api_download_golden.py b/webui/backend/tests/golden/test_api_download_golden.py index 3f487e1..6e749ea 100644 --- a/webui/backend/tests/golden/test_api_download_golden.py +++ b/webui/backend/tests/golden/test_api_download_golden.py @@ -4,6 +4,8 @@ import asyncio import sys import tempfile import unittest +import zipfile +from io import BytesIO from pathlib import Path import httpx @@ -55,6 +57,75 @@ class DownloadApiGoldenTest(unittest.TestCase): def test_download_directory_type_conflict(self) -> None: (self.root / "docs").mkdir() + (self.root / "docs" / "a.txt").write_text("a", encoding="utf-8") + + response = self._get("/api/files/download?path=storage1/docs") + + self.assertEqual(response.status_code, 200) + self.assertIn('attachment; filename="docs.zip"', response.headers.get("content-disposition", "")) + with zipfile.ZipFile(BytesIO(response.content)) as archive: + self.assertIn("docs/", archive.namelist()) + self.assertIn("docs/a.txt", archive.namelist()) + self.assertEqual(archive.read("docs/a.txt"), b"a") + + def test_download_multi_file_selection_as_zip(self) -> None: + (self.root / "a.txt").write_text("A", encoding="utf-8") + (self.root / "b.txt").write_text("B", encoding="utf-8") + + response = self._get("/api/files/download?path=storage1/a.txt&path=storage1/b.txt") + + self.assertEqual(response.status_code, 200) + self.assertRegex( + response.headers.get("content-disposition", ""), + r'attachment; filename="kodidownload-\d{8}-\d{6}\.zip"', + ) + with zipfile.ZipFile(BytesIO(response.content)) as archive: + self.assertIn("a.txt", archive.namelist()) + self.assertIn("b.txt", archive.namelist()) + self.assertEqual(archive.read("a.txt"), b"A") + self.assertEqual(archive.read("b.txt"), b"B") + + def test_download_multi_directory_selection_as_zip(self) -> None: + (self.root / "dir1" / "sub").mkdir(parents=True) + (self.root / "dir2").mkdir() + (self.root / "dir1" / "sub" / "a.txt").write_text("A", encoding="utf-8") + (self.root / "dir2" / "b.txt").write_text("B", encoding="utf-8") + + response = self._get("/api/files/download?path=storage1/dir1&path=storage1/dir2") + + self.assertEqual(response.status_code, 200) + self.assertRegex( + response.headers.get("content-disposition", ""), + r'attachment; filename="kodidownload-\d{8}-\d{6}\.zip"', + ) + with zipfile.ZipFile(BytesIO(response.content)) as archive: + self.assertIn("dir1/", archive.namelist()) + self.assertIn("dir1/sub/", archive.namelist()) + self.assertIn("dir1/sub/a.txt", archive.namelist()) + self.assertIn("dir2/b.txt", archive.namelist()) + + def test_download_mixed_file_and_directory_selection_as_zip(self) -> None: + (self.root / "readme.txt").write_text("R", encoding="utf-8") + (self.root / "photos" / "nested").mkdir(parents=True) + (self.root / "photos" / "nested" / "img.txt").write_text("P", encoding="utf-8") + + response = self._get("/api/files/download?path=storage1/readme.txt&path=storage1/photos") + + self.assertEqual(response.status_code, 200) + self.assertRegex( + response.headers.get("content-disposition", ""), + r'attachment; filename="kodidownload-\d{8}-\d{6}\.zip"', + ) + with zipfile.ZipFile(BytesIO(response.content)) as archive: + self.assertIn("readme.txt", archive.namelist()) + self.assertIn("photos/", archive.namelist()) + self.assertIn("photos/nested/img.txt", archive.namelist()) + + def test_download_directory_with_symlink_rejected(self) -> None: + target = self.root / "real.txt" + target.write_text("x", encoding="utf-8") + (self.root / "docs").mkdir() + (self.root / "docs" / "link.txt").symlink_to(target) response = self._get("/api/files/download?path=storage1/docs") diff --git a/webui/backend/tests/golden/test_ui_smoke_golden.py b/webui/backend/tests/golden/test_ui_smoke_golden.py index f39a314..954cbd4 100644 --- a/webui/backend/tests/golden/test_ui_smoke_golden.py +++ b/webui/backend/tests/golden/test_ui_smoke_golden.py @@ -215,7 +215,7 @@ class UiSmokeGoldenTest(unittest.TestCase): self.assertIn('function openContextMenu(pane, entry, event)', app_js) self.assertIn('function closeContextMenu()', app_js) self.assertIn('function isOpenableSelection(item)', app_js) - self.assertIn('async function downloadFileRequest(path)', app_js) + self.assertIn('async function downloadFileRequest(paths)', app_js) self.assertIn('function applyContextMenuSelection()', app_js) self.assertIn('function startContextMenuOpen()', app_js) self.assertIn('function startContextMenuEdit()', app_js) @@ -239,9 +239,9 @@ class UiSmokeGoldenTest(unittest.TestCase): self.assertIn('const editableSingle = items.length === 1 && isEditableSelection(items[0]);', app_js) self.assertIn('elements.editButton.classList.toggle("hidden", isMulti || items.length !== 1 || items[0].kind !== "file");', app_js) self.assertIn('elements.editButton.disabled = !editableSingle;', app_js) - self.assertIn('const downloadableSingle = items.length === 1 && items[0].kind === "file";', app_js) - self.assertIn('elements.downloadButton.classList.toggle("hidden", !downloadableSingle);', app_js) - self.assertIn('elements.downloadButton.disabled = !downloadableSingle;', app_js) + self.assertIn('const downloadableSelection = items.length > 0;', app_js) + self.assertIn('elements.downloadButton.classList.remove("hidden");', app_js) + self.assertIn('elements.downloadButton.disabled = !downloadableSelection;', app_js) self.assertIn('elements.renameButton.classList.toggle("hidden", isMulti);', app_js) self.assertIn('elements.copyButton.classList.remove("hidden");', app_js) self.assertIn('elements.copyButton.disabled = items.length === 0;', app_js) @@ -250,8 +250,8 @@ class UiSmokeGoldenTest(unittest.TestCase): self.assertIn('elements.propertiesButton.disabled = items.length === 0;', app_js) self.assertIn('openCurrentDirectory();', app_js) self.assertIn('openEditor();', app_js) - self.assertIn('downloadFileRequest(selected.path);', app_js) - self.assertIn('anchor.download = selected.name;', app_js) + self.assertIn('downloadFileRequest(selectedItems.map((item) => item.path));', app_js) + self.assertIn('anchor.download = fileName || selected.name;', app_js) self.assertIn('openRenamePopup();', app_js) self.assertIn('startCopySelected();', app_js) self.assertIn('openF6Flow();', app_js) diff --git a/webui/html/app.js b/webui/html/app.js index 8f7fac8..7495988 100644 --- a/webui/html/app.js +++ b/webui/html/app.js @@ -384,15 +384,15 @@ function openContextMenu(pane, entry, event) { const isMulti = items.length > 1; const openableSingle = items.length === 1 && isOpenableSelection(items[0]); const editableSingle = items.length === 1 && isEditableSelection(items[0]); - const downloadableSingle = items.length === 1 && items[0].kind === "file"; + const downloadableSelection = items.length > 0; elements.scope.textContent = isMulti ? "Multi-selection" : "Single item"; elements.target.textContent = isMulti ? `${items.length} selected items` : entry.name; elements.openButton.classList.toggle("hidden", isMulti); elements.openButton.disabled = !openableSingle; elements.editButton.classList.toggle("hidden", isMulti || items.length !== 1 || items[0].kind !== "file"); elements.editButton.disabled = !editableSingle; - elements.downloadButton.classList.toggle("hidden", !downloadableSingle); - elements.downloadButton.disabled = !downloadableSingle; + elements.downloadButton.classList.remove("hidden"); + elements.downloadButton.disabled = !downloadableSelection; elements.renameButton.classList.toggle("hidden", isMulti); elements.copyButton.classList.remove("hidden"); elements.copyButton.disabled = items.length === 0; @@ -496,21 +496,21 @@ function startContextMenuEdit() { async function startDownloadSelected() { const selectedItems = activePaneState().selectedItems; - if (selectedItems.length !== 1 || selectedItems[0].kind !== "file") { + if (selectedItems.length === 0) { return; } - const selected = selectedItems[0]; try { - const blob = await downloadFileRequest(selected.path); + const selected = selectedItems[0]; + const { blob, fileName } = await downloadFileRequest(selectedItems.map((item) => item.path)); const url = URL.createObjectURL(blob); const anchor = document.createElement("a"); anchor.href = url; - anchor.download = selected.name; + anchor.download = fileName || selected.name; document.body.append(anchor); anchor.click(); anchor.remove(); URL.revokeObjectURL(url); - setStatus(`Download started: ${selected.name}`); + setStatus(`Download started: ${anchor.download}`); } catch (err) { setActionError("Download", err); } @@ -782,13 +782,22 @@ function createApiError(response, data) { return err; } -async function downloadFileRequest(path) { - const response = await fetch(`/api/files/download?${new URLSearchParams({ path }).toString()}`); +async function downloadFileRequest(paths) { + const params = new URLSearchParams(); + for (const path of paths) { + params.append("path", path); + } + const response = await fetch(`/api/files/download?${params.toString()}`); if (!response.ok) { const data = await response.json().catch(() => ({})); throw createApiError(response, data); } - return response.blob(); + const disposition = response.headers.get("content-disposition") || ""; + const match = disposition.match(/filename=\"([^\"]+)\"/); + return { + blob: await response.blob(), + fileName: match ? match[1] : null, + }; } async function uploadFileRequest(targetPath, file, overwrite = false) {