fix(import): close the append path check and the relocate overwrite
This commit is contained in:
+12
-4
@@ -42,10 +42,11 @@ function toast(msg, isError = false) {
|
|||||||
setTimeout(() => (t.hidden = true), 4000);
|
setTimeout(() => (t.hidden = true), 4000);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// quotes included: the result also lands inside attributes (value="${esc(...)}")
|
||||||
|
const ESC_CHARS = { "&": "&", "<": "<", ">": ">", '"': """, "'": "'" };
|
||||||
|
|
||||||
function esc(s) {
|
function esc(s) {
|
||||||
const d = document.createElement("div");
|
return String(s ?? "").replace(/[&<>"']/g, (c) => ESC_CHARS[c]);
|
||||||
d.textContent = s ?? "";
|
|
||||||
return d.innerHTML;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// last used library per media type
|
// last used library per media type
|
||||||
@@ -1040,7 +1041,8 @@ $("#import-merge").addEventListener("click", () => {
|
|||||||
const name =
|
const name =
|
||||||
commonPrefix(chosen.map((it) => it.name)) || parentName || chosen[0].name;
|
commonPrefix(chosen.map((it) => it.name)) || parentName || chosen[0].name;
|
||||||
const merged = {
|
const merged = {
|
||||||
path: chosen[0].path.slice(0, chosen[0].path.length - chosen[0].name.length - 1) || chosen[0].path,
|
path: chosen[0].dir || chosen[0].path,
|
||||||
|
dir: chosen[0].dir,
|
||||||
name,
|
name,
|
||||||
rel_dir: chosen[0].rel_dir,
|
rel_dir: chosen[0].rel_dir,
|
||||||
media_type: chosen[0].media_type,
|
media_type: chosen[0].media_type,
|
||||||
@@ -1398,6 +1400,12 @@ async function collapseSharedRequests(chosen) {
|
|||||||
|
|
||||||
// deepest folder that holds all of the group's entries
|
// deepest folder that holds all of the group's entries
|
||||||
function sharedParent(group) {
|
function sharedParent(group) {
|
||||||
|
// base64 paths carry no readable separators, so only the scanner's own
|
||||||
|
// parent dir says anything about them
|
||||||
|
if (group.some((it) => it.path.startsWith("b64:"))) {
|
||||||
|
const dirs = new Set(group.map((it) => it.dir));
|
||||||
|
return dirs.size === 1 ? group[0].dir : group[0].path;
|
||||||
|
}
|
||||||
const parts = group.map((it) => it.path.split("/"));
|
const parts = group.map((it) => it.path.split("/"));
|
||||||
const first = parts[0];
|
const first = parts[0];
|
||||||
let i = 0;
|
let i = 0;
|
||||||
|
|||||||
@@ -983,3 +983,113 @@ def test_a_single_folder_with_many_files_is_still_one_audiobook(client):
|
|||||||
(d / f"{i:02d}.mp3").write_bytes(b"")
|
(d / f"{i:02d}.mp3").write_bytes(b"")
|
||||||
items = client.get("/api/import/scan").json()["items"]
|
items = client.get("/api/import/scan").json()["items"]
|
||||||
assert len(items) == 1 and items[0]["is_dir"] and len(items[0]["files"]) == 3
|
assert len(items) == 1 and items[0]["is_dir"] and len(items[0]["files"]) == 3
|
||||||
|
|
||||||
|
|
||||||
|
def test_append_rejects_files_outside_download_dir(client):
|
||||||
|
"""The append branch used to skip the download-dir check entirely, so it
|
||||||
|
would happily pull in - and clean up - folders anywhere on disk."""
|
||||||
|
root = client.tmp_path / "library" / "k"
|
||||||
|
lib_id = client.post("/api/libraries", json={
|
||||||
|
"name": "K", "media_type": "audiobook", "root_path": str(root),
|
||||||
|
"folder_template": "{Title}", "file_template": "{Title}",
|
||||||
|
}).json()["id"]
|
||||||
|
req_id = client.post("/api/requests",
|
||||||
|
json={"library_id": lib_id, "title": "Toteninsel"}).json()["id"]
|
||||||
|
|
||||||
|
part_a = client.downloads / "A"
|
||||||
|
part_a.mkdir()
|
||||||
|
for i in (1, 2):
|
||||||
|
(part_a / f"{i:02d}.mp3").write_bytes(b"")
|
||||||
|
client.post("/api/import", json={"items": [{
|
||||||
|
"path": str(part_a), "is_dir": True,
|
||||||
|
"files": [str(f) for f in sorted(part_a.iterdir())], "request_id": req_id,
|
||||||
|
}]})
|
||||||
|
|
||||||
|
outside = client.tmp_path / "elsewhere"
|
||||||
|
outside.mkdir()
|
||||||
|
stranger = outside / "private.mp3"
|
||||||
|
stranger.write_bytes(b"not yours")
|
||||||
|
|
||||||
|
res = client.post("/api/import", json={"items": [{
|
||||||
|
"path": str(outside), "is_dir": True,
|
||||||
|
"files": [str(stranger)], "request_id": req_id, "append": True,
|
||||||
|
}]}).json()["results"][0]
|
||||||
|
assert not res["ok"]
|
||||||
|
assert res["error"] == "path outside download dir"
|
||||||
|
assert stranger.exists() and outside.is_dir()
|
||||||
|
|
||||||
|
|
||||||
|
def test_import_survives_a_broken_base64_path(client):
|
||||||
|
"""A malformed b64: payload is one bad item, not a 500 for the whole batch."""
|
||||||
|
lib_id = client.post("/api/libraries", json={
|
||||||
|
"name": "K", "media_type": "ebook", "root_path": str(client.tmp_path / "k"),
|
||||||
|
}).json()["id"]
|
||||||
|
req_id = client.post("/api/requests",
|
||||||
|
json={"library_id": lib_id, "title": "X"}).json()["id"]
|
||||||
|
resp = client.post("/api/import", json={"items": [{
|
||||||
|
"path": "b64:!!!not base64!!!", "is_dir": False,
|
||||||
|
"files": [], "request_id": req_id,
|
||||||
|
}]})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert not resp.json()["results"][0]["ok"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_relocate_does_not_overwrite_within_the_same_folder(client):
|
||||||
|
"""Renaming in place: a target name may already be held by another file of
|
||||||
|
the same set. Sorting decides who moves first, so the slot can still be
|
||||||
|
occupied when the move happens - it must not be overwritten."""
|
||||||
|
root = client.tmp_path / "library" / "k"
|
||||||
|
lib_id = client.post("/api/libraries", json={
|
||||||
|
"name": "K", "media_type": "audiobook", "root_path": str(root),
|
||||||
|
"folder_template": "{Title}", "file_template": "{Title}",
|
||||||
|
}).json()["id"]
|
||||||
|
req_id = client.post("/api/requests",
|
||||||
|
json={"library_id": lib_id, "title": "Insel"}).json()["id"]
|
||||||
|
src = client.downloads / "insel"
|
||||||
|
src.mkdir()
|
||||||
|
for i in (1, 2):
|
||||||
|
(src / f"{i:02d}.mp3").write_bytes(f"track {i}".encode())
|
||||||
|
client.post("/api/import", json={"items": [{
|
||||||
|
"path": str(src), "is_dir": True,
|
||||||
|
"files": [str(f) for f in sorted(src.iterdir())], "request_id": req_id,
|
||||||
|
}]})
|
||||||
|
dest = root / "Insel"
|
||||||
|
|
||||||
|
# "Aaa" sorts first, so it claims "Part 01" while part 1 still sits there
|
||||||
|
(dest / "Insel - Part 02.mp3").rename(dest / "Aaa.mp3")
|
||||||
|
res = client.post(f"/api/requests/{req_id}/relocate").json()
|
||||||
|
assert res["ok"], res
|
||||||
|
|
||||||
|
assert sorted(f.name for f in dest.iterdir()) == [
|
||||||
|
"Insel - Part 01.mp3", "Insel - Part 02.mp3",
|
||||||
|
]
|
||||||
|
blobs = [f.read_bytes() for f in dest.iterdir()]
|
||||||
|
assert all(any(f"track {i}".encode() in b for b in blobs) for i in (1, 2))
|
||||||
|
|
||||||
|
|
||||||
|
def test_single_file_folder_import_removes_the_empty_source(client):
|
||||||
|
lib_id = client.post("/api/libraries", json={
|
||||||
|
"name": "K", "media_type": "audiobook", "root_path": str(client.tmp_path / "k"),
|
||||||
|
}).json()["id"]
|
||||||
|
req_id = client.post("/api/requests",
|
||||||
|
json={"library_id": lib_id, "title": "Solo"}).json()["id"]
|
||||||
|
folder = client.downloads / "solo release"
|
||||||
|
folder.mkdir()
|
||||||
|
(folder / "book.m4b").write_bytes(b"")
|
||||||
|
res = client.post("/api/import", json={"items": [{
|
||||||
|
"path": str(folder), "is_dir": True,
|
||||||
|
"files": [str(folder / "book.m4b")], "request_id": req_id,
|
||||||
|
}]}).json()["results"][0]
|
||||||
|
assert res["ok"], res
|
||||||
|
assert not folder.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_does_not_loop_on_a_symlink_cycle(client):
|
||||||
|
from wordarr.importer import scanner
|
||||||
|
|
||||||
|
d = client.downloads / "series"
|
||||||
|
d.mkdir()
|
||||||
|
(d / "book.m4b").write_bytes(b"")
|
||||||
|
(d / "back").symlink_to(client.downloads, target_is_directory=True)
|
||||||
|
items = scanner.scan(client.downloads)
|
||||||
|
assert [i["name"] for i in items] == ["book"]
|
||||||
|
|||||||
+14
-7
@@ -59,14 +59,26 @@ def do_import(data: ImportIn, session: Session = Depends(get_session)):
|
|||||||
download_root = Path(config.DOWNLOAD_DIR).resolve()
|
download_root = Path(config.DOWNLOAD_DIR).resolve()
|
||||||
for item in data.items:
|
for item in data.items:
|
||||||
# the client hands back whatever the scan produced, base64 included
|
# the client hands back whatever the scan produced, base64 included
|
||||||
item.path = decode_path(item.path)
|
shown = item.path
|
||||||
item.files = [decode_path(f) for f in item.files]
|
try:
|
||||||
|
item.path = decode_path(item.path)
|
||||||
|
item.files = [decode_path(f) for f in item.files]
|
||||||
|
except Exception as exc:
|
||||||
|
results.append({"path": shown, "ok": False,
|
||||||
|
"error": f"unlesbarer Pfad: {exc}"})
|
||||||
|
continue
|
||||||
shown = readable(item.path)
|
shown = readable(item.path)
|
||||||
req = session.get(BookRequest, item.request_id)
|
req = session.get(BookRequest, item.request_id)
|
||||||
if not req:
|
if not req:
|
||||||
results.append({"path": shown, "ok": False,
|
results.append({"path": shown, "ok": False,
|
||||||
"error": f"Anfrage #{item.request_id} existiert nicht mehr"})
|
"error": f"Anfrage #{item.request_id} existiert nicht mehr"})
|
||||||
continue
|
continue
|
||||||
|
# every branch below moves these files, so check them before branching.
|
||||||
|
# merged items carry files from several folders, so check them all
|
||||||
|
paths = [Path(p).resolve() for p in [item.path, *item.files]]
|
||||||
|
if any(download_root not in p.parents and p != download_root for p in paths):
|
||||||
|
results.append({"path": shown, "ok": False, "error": "path outside download dir"})
|
||||||
|
continue
|
||||||
path_gone = not req.imported_path or not Path(req.imported_path).exists()
|
path_gone = not req.imported_path or not Path(req.imported_path).exists()
|
||||||
if req.status == "imported" and path_gone:
|
if req.status == "imported" and path_gone:
|
||||||
# the library copy is gone; treat this as a fresh import
|
# the library copy is gone; treat this as a fresh import
|
||||||
@@ -89,11 +101,6 @@ def do_import(data: ImportIn, session: Session = Depends(get_session)):
|
|||||||
" — mehrere Einträge auf dieselbe Anfrage? Dann vorher zusammenfassen."
|
" — mehrere Einträge auf dieselbe Anfrage? Dann vorher zusammenfassen."
|
||||||
)})
|
)})
|
||||||
continue
|
continue
|
||||||
# merged items carry files from several folders, so check them all
|
|
||||||
paths = [Path(p).resolve() for p in [item.path, *item.files]]
|
|
||||||
if any(download_root not in p.parents and p != download_root for p in paths):
|
|
||||||
results.append({"path": shown, "ok": False, "error": "path outside download dir"})
|
|
||||||
continue
|
|
||||||
try:
|
try:
|
||||||
dest = mover.import_item(item.path, item.files, item.is_dir, req, req.library)
|
dest = mover.import_item(item.path, item.files, item.is_dir, req, req.library)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
|
|||||||
@@ -30,7 +30,6 @@ def import_item(item_path: str, files: list[str], is_dir: bool, request, library
|
|||||||
Returns the destination path (folder for multi-file audiobooks, else the file)."""
|
Returns the destination path (folder for multi-file audiobooks, else the file)."""
|
||||||
root = Path(library.root_path)
|
root = Path(library.root_path)
|
||||||
folder = root / render_template(library.folder_template, request)
|
folder = root / render_template(library.folder_template, request)
|
||||||
folder.mkdir(parents=True, exist_ok=True)
|
|
||||||
base = render_template(library.file_template, request)
|
base = render_template(library.file_template, request)
|
||||||
|
|
||||||
paths = [Path(f) for f in files]
|
paths = [Path(f) for f in files]
|
||||||
@@ -49,6 +48,7 @@ def import_item(item_path: str, files: list[str], is_dir: bool, request, library
|
|||||||
+ (f" (und {len(taken) - 1} weitere)" if len(taken) > 1 else "")
|
+ (f" (und {len(taken) - 1} weitere)" if len(taken) > 1 else "")
|
||||||
+ " — trägt ein anderer Titel nach diesem Namensschema denselben Namen?"
|
+ " — trägt ein anderer Titel nach diesem Namensschema denselben Namen?"
|
||||||
)
|
)
|
||||||
|
folder.mkdir(parents=True, exist_ok=True)
|
||||||
for i, (src, dest) in enumerate(zip(paths, targets), 1):
|
for i, (src, dest) in enumerate(zip(paths, targets), 1):
|
||||||
shutil.move(str(src), dest)
|
shutil.move(str(src), dest)
|
||||||
if request.media_type == "audiobook":
|
if request.media_type == "audiobook":
|
||||||
@@ -60,9 +60,13 @@ def import_item(item_path: str, files: list[str], is_dir: bool, request, library
|
|||||||
dest = folder / sanitize(f"{base}{src.suffix.lower()}")
|
dest = folder / sanitize(f"{base}{src.suffix.lower()}")
|
||||||
if dest.exists():
|
if dest.exists():
|
||||||
raise FileExistsError(f"Destination already exists: {dest}")
|
raise FileExistsError(f"Destination already exists: {dest}")
|
||||||
|
folder.mkdir(parents=True, exist_ok=True)
|
||||||
shutil.move(str(src), dest)
|
shutil.move(str(src), dest)
|
||||||
if request.media_type == "audiobook":
|
if request.media_type == "audiobook":
|
||||||
tagger.tag_audio(dest, request)
|
tagger.tag_audio(dest, request)
|
||||||
|
if is_dir:
|
||||||
|
# a folder holding a single audiobook file is emptied by the move too
|
||||||
|
_cleanup_sources(Path(item_path), paths)
|
||||||
return str(dest)
|
return str(dest)
|
||||||
|
|
||||||
|
|
||||||
@@ -134,16 +138,35 @@ def relocate(request, library) -> str:
|
|||||||
folder.mkdir(parents=True, exist_ok=True)
|
folder.mkdir(parents=True, exist_ok=True)
|
||||||
|
|
||||||
width = max(2, len(str(len(audio))))
|
width = max(2, len(str(len(audio))))
|
||||||
for i, f in enumerate(audio, 1):
|
targets = [
|
||||||
name = sanitize(
|
folder / sanitize(
|
||||||
f"{base} - Part {i:0{width}d}{f.suffix.lower()}" if len(audio) > 1
|
f"{base} - Part {i:0{width}d}{f.suffix.lower()}" if len(audio) > 1
|
||||||
else f"{base}{f.suffix.lower()}"
|
else f"{base}{f.suffix.lower()}"
|
||||||
)
|
)
|
||||||
dest = folder / name
|
for i, f in enumerate(audio, 1)
|
||||||
if dest.resolve() != f.resolve():
|
]
|
||||||
shutil.move(str(f), dest)
|
# renaming in place may aim at a name that is already taken: fine when the
|
||||||
|
# occupant is one of the files being moved, never when it is a stranger
|
||||||
|
sources = {f.resolve() for f in audio}
|
||||||
|
taken = [d for d in targets if d.exists() and d.resolve() not in sources]
|
||||||
|
if taken or len(set(targets)) != len(targets):
|
||||||
|
raise FileExistsError(
|
||||||
|
f"Destination already exists: {taken[0] if taken else targets[0]}"
|
||||||
|
" — trägt ein anderer Titel nach diesem Namensschema denselben Namen?"
|
||||||
|
)
|
||||||
|
current = list(audio)
|
||||||
|
for i, dest in enumerate(targets):
|
||||||
|
if current[i].resolve() != dest.resolve():
|
||||||
|
if dest.exists():
|
||||||
|
# a file further down the list still sits in this slot
|
||||||
|
j = next(k for k, c in enumerate(current) if c.resolve() == dest.resolve())
|
||||||
|
stashed = dest.with_name(dest.name + ".wordarr-tmp")
|
||||||
|
shutil.move(str(current[j]), stashed)
|
||||||
|
current[j] = stashed
|
||||||
|
shutil.move(str(current[i]), dest)
|
||||||
|
current[i] = dest
|
||||||
if request.media_type == "audiobook":
|
if request.media_type == "audiobook":
|
||||||
tagger.tag_audio(dest, request, track=i, total=len(audio))
|
tagger.tag_audio(dest, request, track=i + 1, total=len(audio))
|
||||||
for extra in extras:
|
for extra in extras:
|
||||||
target = folder / extra.name
|
target = folder / extra.name
|
||||||
if target.resolve() != extra.resolve() and not target.exists():
|
if target.resolve() != extra.resolve() and not target.exists():
|
||||||
|
|||||||
@@ -113,13 +113,24 @@ def scan(root: Path, split_dirs: bool = False) -> list[dict]:
|
|||||||
except ValueError:
|
except ValueError:
|
||||||
return ""
|
return ""
|
||||||
|
|
||||||
|
seen: set[Path] = set()
|
||||||
|
|
||||||
def walk(d: Path):
|
def walk(d: Path):
|
||||||
|
# symlinked folders may point back up; without this a loop never ends
|
||||||
|
try:
|
||||||
|
real = d.resolve()
|
||||||
|
except OSError:
|
||||||
|
return
|
||||||
|
if real in seen:
|
||||||
|
return
|
||||||
|
seen.add(real)
|
||||||
audio = audio_files(d)
|
audio = audio_files(d)
|
||||||
if not split_dirs:
|
if not split_dirs:
|
||||||
discs = disc_audio(d) if d != root else None
|
discs = disc_audio(d) if d != root else None
|
||||||
if discs:
|
if discs:
|
||||||
items.append({
|
items.append({
|
||||||
"path": encode_path(str(d)),
|
"path": encode_path(str(d)),
|
||||||
|
"dir": encode_path(str(d.parent)),
|
||||||
"name": readable(d.name),
|
"name": readable(d.name),
|
||||||
"rel_dir": readable(rel_dir(d.parent)),
|
"rel_dir": readable(rel_dir(d.parent)),
|
||||||
"media_type": "audiobook",
|
"media_type": "audiobook",
|
||||||
@@ -133,6 +144,7 @@ def scan(root: Path, split_dirs: bool = False) -> list[dict]:
|
|||||||
if len(audio) > 1 and d != root:
|
if len(audio) > 1 and d != root:
|
||||||
items.append({
|
items.append({
|
||||||
"path": encode_path(str(d)),
|
"path": encode_path(str(d)),
|
||||||
|
"dir": encode_path(str(d.parent)),
|
||||||
"name": readable(d.name),
|
"name": readable(d.name),
|
||||||
"rel_dir": readable(rel_dir(d.parent)),
|
"rel_dir": readable(rel_dir(d.parent)),
|
||||||
"media_type": "audiobook",
|
"media_type": "audiobook",
|
||||||
@@ -151,6 +163,7 @@ def scan(root: Path, split_dirs: bool = False) -> list[dict]:
|
|||||||
if mt:
|
if mt:
|
||||||
items.append({
|
items.append({
|
||||||
"path": encode_path(str(entry)),
|
"path": encode_path(str(entry)),
|
||||||
|
"dir": encode_path(str(d)),
|
||||||
"name": readable(entry.stem),
|
"name": readable(entry.stem),
|
||||||
"rel_dir": readable(rel_dir(d)),
|
"rel_dir": readable(rel_dir(d)),
|
||||||
"media_type": mt,
|
"media_type": mt,
|
||||||
|
|||||||
@@ -37,7 +37,8 @@ async def _query(client: httpx.AsyncClient, tld: str, query: str, num_results: i
|
|||||||
resp = await client.get(f"https://api.audible.{tld}/1.0/catalog/products", params=params)
|
resp = await client.get(f"https://api.audible.{tld}/1.0/catalog/products", params=params)
|
||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
return resp.json().get("products", [])
|
return resp.json().get("products", [])
|
||||||
except httpx.HTTPError:
|
except (httpx.HTTPError, ValueError):
|
||||||
|
# a 200 that is not JSON (error page, captcha) must not kill the search
|
||||||
return []
|
return []
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user