Skip to content

fix(navidrome): preserve case-sensitivity of coverArtId and prioritize local disk cache #113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
omixin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(navidrome): preserve case-sensitivity of coverArtId and prioritize local disk cache #113

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions app/src/main/java/com/lostf1sh/pixelplayeross/data/image/NavidromeCoilFetcher.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,19 @@ class NavidromeCoilFetcher(
override suspend fun fetch(): FetchResult? {
Timber.v("$TAG: Fetching $uri")

val coverArtId = uri.host ?: uri.path?.removePrefix("/")
val coverArtId = uri.schemeSpecificPart
.substringBefore('?')
.removePrefix("//")
.ifBlank { uri.host ?: uri.path?.removePrefix("/") }

if (coverArtId.isNullOrBlank()) {
Timber.w("$TAG: Invalid URI format: $uri")
return null
}

if (!repository.isLoggedIn) {
Timber.v("$TAG: Not logged in, skipping fetch")
return null
}

val sizeParam = uri.getQueryParameter("size")?.toIntOrNull() ?: 500

// 1. Check local disk cache FIRST (so cached covers display even offline or on cold start)
val cachedFile = File(cacheDir, "navidrome_cover_${coverArtId}_$sizeParam.jpg")
if (cachedFile.exists() && cachedFile.length() > 0) {
Timber.v("$TAG: Using cached cover for $coverArtId")
Expand All @@ -80,6 +80,12 @@ class NavidromeCoilFetcher(
)
}
Comment on lines 71 to 81

@greptile-apps greptile-apps Bot Aug 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Cache bypasses the login boundary

When a cover remains cached after Navidrome logout, fetch() returns the persistent, account-agnostic file before checking repository.isLoggedIn, causing artwork from the previous authenticated session to remain visible and allowing an account with the same cover identifier to receive stale artwork. How this was verified: The cache-return branch precedes the login check, while logout does not clear this directory and the cache key contains no account or server identity.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/src/main/java/com/lostf1sh/pixelplayeross/data/image/NavidromeCoilFetcher.kt
Line: 71-81

Comment:
**Cache bypasses the login boundary**

When a cover remains cached after Navidrome logout, `fetch()` returns the persistent, account-agnostic file before checking `repository.isLoggedIn`, causing artwork from the previous authenticated session to remain visible and allowing an account with the same cover identifier to receive stale artwork. **How this was verified:** The cache-return branch precedes the login check, while logout does not clear this directory and the cache key contains no account or server identity.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex


// 2. Only check login if we actually need to download from network
if (!repository.isLoggedIn) {
Timber.v("$TAG: Not logged in and no local cache, skipping fetch")
return null
}

val coverArtUrl = repository.getCoverArtUrl(coverArtId, sizeParam)
if (coverArtUrl.isNullOrBlank()) {
if (shouldLogFailure("no_url_$coverArtId")) {
Expand Down Expand Up @@ -152,7 +158,10 @@ class NavidromeCoilFetcher(

override fun create(data: Uri, options: Options, imageLoader: ImageLoader): Fetcher? {
return if (data.scheme == "navidrome_cover") {
val cache = cacheDir ?: options.context.cacheDir.also { cacheDir = it }
val cache = cacheDir ?: File(options.context.filesDir, "album_art").also {
if (!it.exists()) it.mkdirs()
cacheDir = it
}
Comment on lines +161 to +164

@greptile-apps greptile-apps Bot Aug 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Persistent covers lack eviction

Each distinct Navidrome cover now creates a navidrome_cover_* file under persistent filesDir/album_art, but existing cache maintenance only processes song_art_* files, so artwork storage grows with the browsed library and is not reclaimed by the application's cache cleanup.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/src/main/java/com/lostf1sh/pixelplayeross/data/image/NavidromeCoilFetcher.kt
Line: 161-164

Comment:
**Persistent covers lack eviction**

Each distinct Navidrome cover now creates a `navidrome_cover_*` file under persistent `filesDir/album_art`, but existing cache maintenance only processes `song_art_*` files, so artwork storage grows with the browsed library and is not reclaimed by the application's cache cleanup.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

NavidromeCoilFetcher(data, repository, okHttpClient, cache)
} else {
null
Expand Down