fix: name media-server downloads from the server's Content-Disposition - #133
Conversation
Downloads were saved under a name fixed before the request: `originalFileName ?: "<title>.mp3"`.
Audiobookshelf list items are minified and carry no file name, so every ABS download got the
".mp3" guess, and ABS answers `api/items/{id}/download` for a book in its own folder with a zip.
Archive expansion goes by the saved name, so the zip was never unpacked and the import produced an
unplayable file ("It was not possible to play the item from file system").
The saved name now comes from the response's Content-Disposition (RFC 5987 `filename*` first, then
`filename`, last path segment only, sanitized) when it carries an extension, else the caller's name
— what iOS does through URLResponse.suggestedFilename. ABS books now arrive as "<title>.zip" and are
unpacked; Jellyfin's "<file>.m4b" is unchanged. A zip signature under a non-archive name (a proxy
that drops the header) is renamed to ".zip" so expansion still runs. A file-only re-download keeps
the existing item's name, which the accept step matches it by.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
✅ Claude PR Review —
|
| Finding | Status |
|---|---|
app/src/main/java/com/tortugapower/audiobookplayer/logic/ImportManager.kt:272 (warn) ⚠︎ moved |
✅ verified fixed in 9dd20f6 |
Converged: nothing new this round, and every earlier finding is settled.
Model claude-opus-5-5 · run log · 0 new · 0 carried over · 1 verified closed · 0 resolved · advisory (a human should still review). Findings are de-duplicated across pushes; an earlier finding closes only when the verification pass judges it against the current code — fixed, no longer applicable, accepted by a maintainer, or a duplicate of a finding reported on this push.
Only the requested name was claimed and checked before the request, but the file is now written under the name the server chose. When that name differs: - the library checks rerun on it before the body is copied: an item that already has its audio is skipped, one whose audio is missing is restored (file-only) instead of duplicated. Archives are still checked per extracted entry; - it gets a staging file of its own (an atomic createNewFile over uniqueDestination), so two downloads can't share one and a file already waiting in the import sheet is never truncated; the zip rename reserves its target the same way. Content-Disposition names with control characters (a decoded %00 or %0A) are rejected, falling back to the requested name. ImportManagerDownloadTest covers the skip, restore, staged-file, zip and header-less zip paths against a MockWebServer.
A file-only restore found through the server's name must keep the item's exact name, so it couldn't take a reserved "-1" name and was written unreserved. It is now created atomically under that name; if the file already exists, a copy of the same file is already staged or in flight, and the download is skipped as a duplicate instead of truncating it.
Round 2 skipped a file-only restore whenever BPBackup already held a file with the item's name, but BPBackup routinely keeps leftovers (the storage screen lists them as orphans), so one leftover could block that restore for good while the requested-name path simply replaced it. The restore now skips only when the name is taken by something current — a file staged in the import sheet, or a name another download in flight has claimed — checked and claimed on Main, released in finally. A mere leftover is replaced, as on the requested-name path.
This branch was successfully deployed
1 participant
What was wrong
Media-server downloads (the Download button on an item's details and the multi-select Download) were saved under a name fixed before the request:
originalFileName ?: "<title>.mp3".audioFiles), sooriginalFileNameis always null and every ABS download got the.mp3guess.api/items/{id}/downloadfor a book in its own folder — the usual layout — with a zip (Content-Disposition: attachment; filename="<title>.zip").ImportArchiveUtils.isArchive) goes by the saved name, so the zip was never unpacked: the import produced a zip named.mp3, and playing it failed with "It was not possible to play the item from file system" (UnrecognizedInputFormatException, file starts withPK).Jellyfin escaped it because its list items carry the real file name (from
Path).Change
The saved name now comes from the response's
Content-Dispositionwhen it names a file with an extension, else the caller's name — what iOS does throughURLResponse.suggestedFilename:DownloadFileName(:core, pure): RFC 5987filename*first, thenfilename(quoted or bare); only the last path segment is kept, thenFilenameUtils.sanitizeFilename..zip, so expansion still runs if a proxy drops the header.expandArchives. A server-chosen name gets its own staging file (atomiccreateNewFileoveruniqueDestination), so two downloads can't share one and a file waiting in the import sheet is never truncated; the zip rename reserves its target the same way. Names with control characters are rejected.finally); a file merely left over inBPBackupis replaced, as on the requested-name path.Plain "Download from URL" imports go through the same path, so a URL like
…/download?id=…now also gets the server's name.Verification
Emulator (bp-api36, devDebug, app data cleared), throwaway servers:
Book Delta)Book Delta.mp3= a zip, unplayableBook Delta.zip→ unpacked →Book Delta.m4b, BOOK, duration 420 s, linked to its ABS item, playsJellyfin Twelve)Jellyfin Twelve.m4bTests.
./gradlew assembleDevDebug testDevDebugUnitTest :core:testDebugUnitTest lintDevDebuggreen (759 tests: 224 app, 68 wear, 467 core).DownloadFileNameTestpins the real ABS and Jellyfin headers,filename*decoding (non-ASCII), bare and escaped forms, path stripping, fallbacks (no header, no extension), sanitizing, and the zip safety net. Ignoring the header or disabling the zip check fails exactly the name-choice and zip tests.ImportManagerDownloadTest(Robolectric + MockWebServer) covers a server-named duplicate being skipped, an item without audio being restored, a staged file never being overwritten (for a fresh name or a restore), a leftover inBPBackupnever blocking a restore, and an ABS zip being unpacked with or without Content-Disposition; removing the round-1 checks fails exactly those tests.Found along the way (not in this PR)
api/items/{id}/downloadURL (AndroidExternalServiceUtils.downloadUrlFor, iOSbuildAudiobookshelfDownloadUrl), which returns the zip. Reproduced on Android ("It was not possible to play the item from external resource"). The per-file endpointapi/items/{id}/file/{ino}serves the raw audio with range support (206); multi-file books need design.