mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 23:05:46 +00:00
fix(cover): sanitize server_index_key so Windows :port URLs work (#889)
* fix(cover): sanitize server_index_key on disk so Windows accepts ":port" URLs `serverIndexKeyFromUrl` (frontend) strips the URL scheme and leaves the rest of the host as the index key — for a Navidrome instance running on the default `:4533` port that is `host:4533/...`. On Linux/macOS the `:` is fine as a path segment; on Windows `CreateDirectory` rejects the whole path with `ERROR_INVALID_NAME` (os error 123). Result: every `cover_cache_ensure` and `cover_cache_peek_batch` rejected its promise and the album / now-playing / mainstage / lightbox surfaces stayed blank. Empirically verified — switching the active server to a colon-free reverse-proxy URL made the covers load again without any other change. Centralize the fix in a new `cover_server_dir(root, key)` helper next to the existing `cover_entity_relative_dir`: it runs `sanitize_path_segment` on the server key the same way kind/entity ids are already cleaned, so `host:4533` becomes `host_4533` and embedded URL paths collapse into one flat bucket instead of nested directories. Every call site that wants the server bucket — `cover_dir`, `count_cached_cover_ids`, `dir_usage_for_server`, the clear-server command, and `clear_cover_fetch_failures` in the backfill worker — now goes through it. The on-disk layout changes (no more colons, no more nested URL paths), so bump `LAYOUT_STAMP` to `canonical-segment-v4`. The existing stamp-mismatch sweep at startup wipes the legacy buckets — users with a previously-working (colon-free) layout rebuild the cache lazily as they browse. Library, offline, and hot-cache data are not touched. Adds two unit tests covering the sanitization on `cover_server_dir` and the `cover_dir` passthrough. Follow-up to #878 (which introduced `cover_cache_layout.rs` with `sanitize_path_segment` applied only to kind/entity_id). * chore(windows): silence dead_code warnings in debug taskbar_win build `lib.rs` gates `taskbar_win::init` on `cfg(not(debug_assertions))` (PR #866 — debug runs alongside an installed release instance and must not fight it for the taskbar subclass). The `update_taskbar_icon` command still ships in debug and early-returns until `init` populates the COM/HWND atomics, so the init-only helpers (icon HICONs, button IDs, subclass plumbing, `make_buttons`, `subclass_proc`, `init` itself) all look unused — 14 dead_code warnings on every Windows debug `cargo build`. File-level `#![cfg_attr(debug_assertions, allow(dead_code))]` suppresses those warnings only in the debug profile. Release builds keep the strict dead-code check, so a real removal would still surface there. * docs(release): CHANGELOG for windows cover-cache server-key fix (PR #889)
This commit is contained in:
committed by
GitHub
parent
ae2e123a14
commit
839c438a6d
@@ -485,6 +485,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
### Covers — load on Windows when the server URL has a `:port`
|
||||||
|
|
||||||
|
**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#889](https://github.com/Psychotoxical/psysonic/pull/889)**
|
||||||
|
|
||||||
|
* Album, now-playing, mainstage, and lightbox covers no longer stay blank on Windows when the active server URL has a `:port` (typical Navidrome LAN setup on `:4533`). The colon used to land in a Windows filesystem segment, so the OS rejected the whole cache path with `ERROR_INVALID_NAME` and every cover load failed silently.
|
||||||
|
* Existing cache buckets on disk are wiped once on the next launch (layout-stamp bump) and rebuild lazily as users browse. Library, offline, and hot caches are untouched.
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
## [1.46.0] - 2026-05-18
|
## [1.46.0] - 2026-05-18
|
||||||
|
|
||||||
> **🙏 Special thanks to [@zz5zz](https://github.com/zz5zz)** for his tireless quirk-spotting and bug reports on the [Psysonic Discord](https://discord.gg/AMnDRErm4u) — several of the polish fixes in this release landed directly off the back of his messages.
|
> **🙏 Special thanks to [@zz5zz](https://github.com/zz5zz)** for his tireless quirk-spotting and bug reports on the [Psysonic Discord](https://discord.gg/AMnDRErm4u) — several of the polish fixes in this release landed directly off the back of his messages.
|
||||||
|
|||||||
@@ -4,14 +4,20 @@
|
|||||||
//! Navidrome `album.id` is often a bare hash/snowflake; `coverArt` may use `al-*`.
|
//! Navidrome `album.id` is often a bare hash/snowflake; `coverArt` may use `al-*`.
|
||||||
//! Rarely `mf-*` / `dc-*` on disk when UI enables per-disc art. Path shape:
|
//! Rarely `mf-*` / `dc-*` on disk when UI enables per-disc art. Path shape:
|
||||||
//!
|
//!
|
||||||
//! `{root}/{server_index_key}/{kind}/{entity_id}/128.webp`
|
//! `{root}/{server_segment}/{kind}/{entity_id}/128.webp`
|
||||||
|
//!
|
||||||
|
//! `server_segment` is derived from the frontend's `serverIndexKeyFromUrl` (host + path,
|
||||||
|
//! no scheme). On Windows that key would otherwise drop a `:` straight into the filesystem
|
||||||
|
//! whenever the user runs Navidrome on a `:port` URL — `CreateDirectory` then rejects the
|
||||||
|
//! whole path with `ERROR_INVALID_NAME`. [`cover_server_dir`] sanitizes the key before it
|
||||||
|
//! hits disk; every caller that wants a server-scoped cover directory goes through it.
|
||||||
//!
|
//!
|
||||||
//! Bump [`LAYOUT_STAMP`] when the on-disk format changes (app wipes legacy dirs on startup).
|
//! Bump [`LAYOUT_STAMP`] when the on-disk format changes (app wipes legacy dirs on startup).
|
||||||
|
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
|
|
||||||
/// Written to `{cover_root}/.storage-layout` — mismatch triggers cache reset.
|
/// Written to `{cover_root}/.storage-layout` — mismatch triggers cache reset.
|
||||||
pub const LAYOUT_STAMP: &str = "canonical-segment-v3";
|
pub const LAYOUT_STAMP: &str = "canonical-segment-v4";
|
||||||
|
|
||||||
/// True for ids that are only valid as `getCoverArt` targets, not library entity keys.
|
/// True for ids that are only valid as `getCoverArt` targets, not library entity keys.
|
||||||
pub fn is_fetch_only_cover_id(id: &str) -> bool {
|
pub fn is_fetch_only_cover_id(id: &str) -> bool {
|
||||||
@@ -36,13 +42,20 @@ pub fn sanitize_path_segment(segment: &str) -> String {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Relative path under `{root}/{server_index_key}/` — change format here only.
|
/// Relative path under `{root}/{server_segment}/` — change format here only.
|
||||||
pub fn cover_entity_relative_dir(cache_kind: &str, cache_entity_id: &str) -> PathBuf {
|
pub fn cover_entity_relative_dir(cache_kind: &str, cache_entity_id: &str) -> PathBuf {
|
||||||
let kind = sanitize_path_segment(cache_kind);
|
let kind = sanitize_path_segment(cache_kind);
|
||||||
let entity = sanitize_path_segment(cache_entity_id);
|
let entity = sanitize_path_segment(cache_entity_id);
|
||||||
PathBuf::from(kind).join(entity)
|
PathBuf::from(kind).join(entity)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Per-server cache root (`{root}/{server_segment}/`). Sanitizes the index key so
|
||||||
|
/// `host:port` and embedded URL paths survive on Windows. Every caller that wants the
|
||||||
|
/// server bucket — list/count/clear/backfill — must go through this helper.
|
||||||
|
pub fn cover_server_dir(root: &Path, server_index_key: &str) -> PathBuf {
|
||||||
|
root.join(sanitize_path_segment(server_index_key))
|
||||||
|
}
|
||||||
|
|
||||||
/// Absolute directory for one cover entity (`…/album/al-…/` or `…/artist/ar-…/`).
|
/// Absolute directory for one cover entity (`…/album/al-…/` or `…/artist/ar-…/`).
|
||||||
pub fn cover_dir(
|
pub fn cover_dir(
|
||||||
root: &Path,
|
root: &Path,
|
||||||
@@ -50,7 +63,8 @@ pub fn cover_dir(
|
|||||||
cache_kind: &str,
|
cache_kind: &str,
|
||||||
cache_entity_id: &str,
|
cache_entity_id: &str,
|
||||||
) -> PathBuf {
|
) -> PathBuf {
|
||||||
root.join(server_index_key).join(cover_entity_relative_dir(cache_kind, cache_entity_id))
|
cover_server_dir(root, server_index_key)
|
||||||
|
.join(cover_entity_relative_dir(cache_kind, cache_entity_id))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Resolved cover identity — keep in sync with TS `src/cover/resolveEntry.ts`.
|
/// Resolved cover identity — keep in sync with TS `src/cover/resolveEntry.ts`.
|
||||||
@@ -193,6 +207,23 @@ mod tests {
|
|||||||
assert_eq!(dir, root.join("srv").join("album").join("al-1"));
|
assert_eq!(dir, root.join("srv").join("album").join("al-1"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn server_segment_sanitizes_port_colon_and_url_path() {
|
||||||
|
let root = Path::new("/tmp/cover");
|
||||||
|
// Typical LAN URL key from `serverIndexKeyFromUrl`: `host:port/path`.
|
||||||
|
// The `:` is invalid on Windows; the `/` would otherwise create a
|
||||||
|
// nested directory rather than one bucket per server.
|
||||||
|
let dir = cover_server_dir(root, "192.168.1.10:4533/music");
|
||||||
|
assert_eq!(dir, root.join("192.168.1.10_4533_music"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn cover_dir_passes_server_key_through_sanitizer() {
|
||||||
|
let root = Path::new("/tmp/cover");
|
||||||
|
let dir = cover_dir(root, "host:4533", "album", "al-1");
|
||||||
|
assert_eq!(dir, root.join("host_4533").join("album").join("al-1"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn album_and_artist_segments_differ() {
|
fn album_and_artist_segments_differ() {
|
||||||
let al = cover_entity_relative_dir("album", "al-1");
|
let al = cover_entity_relative_dir("album", "al-1");
|
||||||
|
|||||||
@@ -161,7 +161,7 @@ pub fn cover_fetch_recently_failed(cover_dir: &Path) -> bool {
|
|||||||
|
|
||||||
/// Remove `.fetch-failed` markers so the next library pass retries HTTP.
|
/// Remove `.fetch-failed` markers so the next library pass retries HTTP.
|
||||||
pub fn clear_cover_fetch_failures(cover_root: &Path, server_index_key: &str) -> u32 {
|
pub fn clear_cover_fetch_failures(cover_root: &Path, server_index_key: &str) -> u32 {
|
||||||
let server_dir = cover_root.join(server_index_key);
|
let server_dir = cover_cache_layout::cover_server_dir(cover_root, server_index_key);
|
||||||
let mut cleared = 0u32;
|
let mut cleared = 0u32;
|
||||||
for kind in cover_cache_layout::SEGMENT_KINDS {
|
for kind in cover_cache_layout::SEGMENT_KINDS {
|
||||||
let kind_dir = server_dir.join(kind);
|
let kind_dir = server_dir.join(kind);
|
||||||
|
|||||||
@@ -10,7 +10,8 @@ use encode::write_webp_tier;
|
|||||||
use fetch::build_cover_art_url;
|
use fetch::build_cover_art_url;
|
||||||
use image::{DynamicImage, ImageReader};
|
use image::{DynamicImage, ImageReader};
|
||||||
use psysonic_core::cover_cache_layout::{
|
use psysonic_core::cover_cache_layout::{
|
||||||
count_entities_with_canonical_tier, cover_root_disk_usage, server_cover_disk_usage,
|
count_entities_with_canonical_tier, cover_root_disk_usage, cover_server_dir,
|
||||||
|
server_cover_disk_usage,
|
||||||
};
|
};
|
||||||
use psysonic_library::cover_backfill::{
|
use psysonic_library::cover_backfill::{
|
||||||
clear_cover_fetch_failures, collect_cover_backfill_batch, collect_cover_progress,
|
clear_cover_fetch_failures, collect_cover_backfill_batch, collect_cover_progress,
|
||||||
@@ -400,7 +401,7 @@ fn spawn_derive_remaining_tiers(
|
|||||||
|
|
||||||
/// Entity dirs with canonical `800.webp` under `album/` and `artist/` (segment layout).
|
/// Entity dirs with canonical `800.webp` under `album/` and `artist/` (segment layout).
|
||||||
pub(crate) fn count_cached_cover_ids(root: &Path, server_index_key: &str) -> i64 {
|
pub(crate) fn count_cached_cover_ids(root: &Path, server_index_key: &str) -> i64 {
|
||||||
let keyed = count_entities_with_canonical_tier(&root.join(server_index_key));
|
let keyed = count_entities_with_canonical_tier(&cover_server_dir(root, server_index_key));
|
||||||
if keyed > 0 {
|
if keyed > 0 {
|
||||||
return keyed;
|
return keyed;
|
||||||
}
|
}
|
||||||
@@ -419,7 +420,7 @@ pub(crate) fn count_cached_cover_ids(root: &Path, server_index_key: &str) -> i64
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn dir_usage_for_server(root: &Path, server_index_key: &str) -> (u64, u64) {
|
pub(crate) fn dir_usage_for_server(root: &Path, server_index_key: &str) -> (u64, u64) {
|
||||||
server_cover_disk_usage(&root.join(server_index_key))
|
server_cover_disk_usage(&cover_server_dir(root, server_index_key))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn dir_usage_at_root(root: &Path) -> (u64, u64) {
|
pub(crate) fn dir_usage_at_root(root: &Path) -> (u64, u64) {
|
||||||
@@ -693,7 +694,7 @@ pub async fn cover_cache_clear_server(
|
|||||||
) -> Result<(), String> {
|
) -> Result<(), String> {
|
||||||
let st = state(&app)?;
|
let st = state(&app)?;
|
||||||
let guard = st.lock().await;
|
let guard = st.lock().await;
|
||||||
let path = guard.root.join(&server_index_key);
|
let path = cover_server_dir(&guard.root, &server_index_key);
|
||||||
if path.is_dir() {
|
if path.is_dir() {
|
||||||
std::fs::remove_dir_all(&path).map_err(|e| e.to_string())?;
|
std::fs::remove_dir_all(&path).map_err(|e| e.to_string())?;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,6 +3,14 @@
|
|||||||
//! Adds Prev / Play-Pause / Next buttons to the taskbar thumbnail preview.
|
//! Adds Prev / Play-Pause / Next buttons to the taskbar thumbnail preview.
|
||||||
//! Button clicks are intercepted via SetWindowSubclass and routed to the same
|
//! Button clicks are intercepted via SetWindowSubclass and routed to the same
|
||||||
//! `media:prev`, `media:play-pause`, `media:next` events as souvlaki / tray.
|
//! `media:prev`, `media:play-pause`, `media:next` events as souvlaki / tray.
|
||||||
|
//!
|
||||||
|
//! `init` is only called in release builds (lib.rs gates it on
|
||||||
|
//! `not(debug_assertions)` so a `tauri dev` run does not fight an installed
|
||||||
|
//! release instance for the taskbar subclass — see PR #866). In debug builds
|
||||||
|
//! the init-only helpers therefore look unused; the file-level cfg_attr
|
||||||
|
//! suppresses dead-code warnings just for that profile and keeps release
|
||||||
|
//! strict.
|
||||||
|
#![cfg_attr(debug_assertions, allow(dead_code))]
|
||||||
|
|
||||||
use std::sync::atomic::{AtomicIsize, Ordering};
|
use std::sync::atomic::{AtomicIsize, Ordering};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user