From 1e41333b58d3e8518512933ec87c5128df42ad8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1l=20Akp=C4=B1nar?= <4ni1ak@gmail.com> Date: Sun, 23 Aug 2026 02:39:13 +0300 Subject: [PATCH] fix(gui): resolve depot metadata named only by the manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `load_files` accepted a depot only when it carried one of the three hardcoded `METADATA_FILES` names. Depots whose variants are handed rather than coloured ship none of them: the Lift keys its hotspot metadata `core_metadata_left.json` / `core_metadata_right.json` and names the right one in the manifest's `image_metadata` resource. The name lookup missed, `resolve` returned `None` for every root, and every Lift and Lift for Business rendered the generic silhouette with a complete bundle on disk. Resolve the metadata filename through the manifest first — same model-id candidates as the image lookup, since a manifest is keyed on whichever pid Logi authored it against — then fall back to the well-known names for bundles without a manifest. Manifest-sourced names now pass through `safe_component_path` like every other asset file. The download side has to follow, or the resolver looks for a file no sync ever fetched: the desktop sync adds `image_metadata` to its manifest-mapped resource pass (and consults every model-id candidate there too), and the CLI bundle treats `core_metadata_*.json` / `metadata_*.json` as optional assets so an offline bundle carries the variant metadata. Fixes #782 --- crates/openlogi-cli/src/cmd/assets/sync.rs | 13 ++ .../openlogi-desktop/src/services/assets.rs | 113 ++++++++++++++++-- .../src/services/assets/images.rs | 16 +++ .../src/services/assets/sync.rs | 34 ++++-- 4 files changed, 155 insertions(+), 21 deletions(-) diff --git a/crates/openlogi-cli/src/cmd/assets/sync.rs b/crates/openlogi-cli/src/cmd/assets/sync.rs index a7db2cd98..4050e54ff 100644 --- a/crates/openlogi-cli/src/cmd/assets/sync.rs +++ b/crates/openlogi-cli/src/cmd/assets/sync.rs @@ -24,12 +24,25 @@ use openlogi_assets::{AssetRegistry, FRONT_RENDER_FILES, FetchOutcome, METADATA_ /// carousel and the buttons-config view. Newer depots name them /// `front_ext_N`, older ones `front_extN`; the `front_ext` prefix covers /// both. +/// - `core_metadata_*.json` / `metadata_*.json` — per-variant hotspot +/// metadata. Depots whose variants are *handed* rather than coloured ship +/// no bare `core_metadata.json` at all (Lift has only +/// `core_metadata_left.json` / `core_metadata_right.json`), and the depot +/// manifest's `image_metadata` is what names the right one. Bundling them +/// is what keeps such a device off the generic silhouette offline. /// /// (`back_*` renders stay remote until an easyswitch view needs them.) fn is_optional_asset(name: &str) -> bool { if name == "side_core.png" || name == "side.png" { return true; } + if (name.starts_with("core_metadata_") || name.starts_with("metadata_")) + && std::path::Path::new(name) + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("json")) + { + return true; + } let path = std::path::Path::new(name); let ext_is_png = path .extension() diff --git a/crates/openlogi-desktop/src/services/assets.rs b/crates/openlogi-desktop/src/services/assets.rs index 7fa6c4d1a..f2357e703 100644 --- a/crates/openlogi-desktop/src/services/assets.rs +++ b/crates/openlogi-desktop/src/services/assets.rs @@ -26,13 +26,16 @@ use std::sync::Arc; use openlogi_assets::http::safe_component_path; use openlogi_assets::{ - BUTTONS_RENDER_FILES, DeviceEntry, FRONT_RENDER_FILES, Index, METADATA_FILES, Metadata, + BUTTONS_RENDER_FILES, DepotManifest, DeviceEntry, FRONT_RENDER_FILES, Index, METADATA_FILES, + Metadata, }; use openlogi_core::device::{DeviceKind, DeviceModelInfo}; use tracing::{debug, warn}; use walkdir::WalkDir; -use self::images::{buttons_image_for, load_manifest, read_png_dimensions, variant_image_for}; +use self::images::{ + buttons_image_for, load_manifest, metadata_for, read_png_dimensions, variant_image_for, +}; use self::paths::{bundle_assets_root, load_index, user_cache_root}; /// Total bytes of the per-user asset cache — the tier [`sync`] writes and @@ -225,13 +228,6 @@ impl AssetResolver { ); continue; }; - // Hotspot metadata in whichever schema this depot cached: - // `core_metadata.json` (newer) or `metadata.json` (older). - let Some(&meta_name) = METADATA_FILES.iter().find(|n| dir.join(n).exists()) else { - continue; - }; - let meta_path = dir.join(meta_name); - // Pick the colour variant matching this device's HID++ // extended_model_id byte. Logi calibrates the assignment // markers against the *buttons* image (typically @@ -246,6 +242,13 @@ impl AssetResolver { // colour render resolves regardless of which pid Logi keyed on. // Parse the manifest once and consult it for every candidate. let manifest = load_manifest(&dir); + + let Some((meta_name, meta_path)) = + resolve_metadata(&dir, entry, manifest.as_ref(), model.extended_model_id) + else { + continue; + }; + let buttons_name = manifest.as_ref().and_then(|m| { entry .model_id_candidates() @@ -289,7 +292,7 @@ impl AssetResolver { let metadata = match Metadata::load_from(&meta_path) { Ok(m) => m, Err(e) => { - warn!(depot, root = %root.display(), file = meta_name, error = ?e, "device metadata unparseable — rendering image without hotspots"); + warn!(depot, root = %root.display(), file = meta_name.as_str(), error = ?e, "device metadata unparseable — rendering image without hotspots"); Metadata::default() } }; @@ -399,6 +402,36 @@ impl Default for AssetResolver { } } +/// Resolve a depot's hotspot-metadata file inside `dir`, as `(filename, path)`. +/// +/// The manifest's `image_metadata` for this colour variant comes first, then +/// the well-known schema names ([`METADATA_FILES`]). Depots whose variants are +/// *handed* rather than coloured ship none of the well-known names — the Lift +/// keys its metadata `core_metadata_left.json` / `core_metadata_right.json` — +/// so a name-only lookup skips the depot outright and the GUI falls back to the +/// generic silhouette. Manifest-sourced names are attacker-influenced, so they +/// pass the same component check as every other asset file. +fn resolve_metadata( + dir: &Path, + entry: &DeviceEntry, + manifest: Option<&DepotManifest>, + ext: u8, +) -> Option<(String, PathBuf)> { + let mut candidates: Vec = manifest + .and_then(|m| { + entry + .model_id_candidates() + .find_map(|base| metadata_for(m, base, ext)) + }) + .into_iter() + .collect(); + candidates.extend(METADATA_FILES.map(str::to_string)); + candidates.into_iter().find_map(|name| { + let path = safe_component_path(dir, &name, "asset file").ok()?; + path.exists().then_some((name, path)) + }) +} + /// Match a connected device's HID++ model info against a loaded index, /// returning the depot name + entry without touching the filesystem. /// @@ -624,6 +657,66 @@ mod tests { assert_eq!(asset.metadata.assignments().count(), 1); } + /// A depot whose variants are handed rather than coloured ships none of + /// [`METADATA_FILES`] — the Lift keys its metadata `core_metadata_left` + /// / `core_metadata_right` and names the right one in the manifest's + /// `image_metadata`. Resolving by well-known name alone skipped the + /// depot outright and rendered the generic silhouette. + #[test] + fn resolves_depot_whose_metadata_is_only_named_by_the_manifest() { + let root = tempfile::tempdir().expect("create temp dir"); + let depot = "mx_vertical_mini"; + let dir = root.path().join(depot); + std::fs::create_dir_all(&dir).expect("create depot dir"); + std::fs::write( + dir.join("manifest.json"), + r#"{"devices":[{"modelId":"b031_ext4","resources":[ + {"key":"image_metadata","src":"core_metadata_right.json"}, + {"key":"device_image","src":"front_ext_2.png"} + ]}]}"#, + ) + .expect("write manifest"); + std::fs::write( + dir.join("core_metadata_right.json"), + r#"{"images":[ + {"key":"device_buttons_image","origin":{"width":860,"height":1256}, + "assignments":[{"slotName":"SLOT_NAME_MIDDLE_BUTTON", + "marker":{"x":50,"y":50},"label":{"x":0,"y":0}}]} + ]}"#, + ) + .expect("write variant metadata"); + std::fs::write(dir.join("front_ext_2.png"), png_header(860, 1256)) + .expect("write variant render"); + + let resolver = AssetResolver { + read_roots: vec![root.path().to_path_buf()], + write_root: root.path().to_path_buf(), + has_bundle: false, + index: None, + }; + let entry = DeviceEntry { + model_id: "b031".to_string(), + model_ids: Vec::new(), + display_name: "Lift".to_string(), + kind: "MOUSE".to_string(), + asset_path: format!("v1/devices/{depot}/"), + files: Vec::new(), + }; + let model = DeviceModelInfo { + extended_model_id: 4, + ..bare_model() + }; + + let asset = resolver + .load_files(depot, &entry, &model) + .expect("manifest-named metadata should resolve the depot"); + assert_eq!( + asset.image_path.file_name().expect("image has a file name"), + "front_ext_2.png" + ); + assert_eq!(asset.metadata.assignments().count(), 1); + } + #[test] fn resolves_standalone_registry_model_without_synthetic_hidpp_info() { let root = tempfile::tempdir().expect("create temp dir"); diff --git a/crates/openlogi-desktop/src/services/assets/images.rs b/crates/openlogi-desktop/src/services/assets/images.rs index c87b88e80..2960dadd8 100644 --- a/crates/openlogi-desktop/src/services/assets/images.rs +++ b/crates/openlogi-desktop/src/services/assets/images.rs @@ -67,6 +67,22 @@ pub(super) fn buttons_image_for( .map(str::to_string) } +/// Like [`variant_image_for`] but returns the `image_metadata` resource — +/// the hotspot-metadata JSON calibrated against this colour variant's +/// renders. Depots whose variants are *handed* rather than coloured ship no +/// well-known metadata name at all (Lift keys its metadata as +/// `core_metadata_left.json` / `core_metadata_right.json`), so the manifest +/// is the only place their filename appears. +pub(super) fn metadata_for( + manifest: &DepotManifest, + base_model_id: &str, + ext: u8, +) -> Option { + manifest + .resource_for_variant(base_model_id, ext, "image_metadata") + .map(str::to_string) +} + /// Load and parse a depot's `manifest.json`, or `None` when it's missing / /// malformed. Read once per [`load_files`](super::AssetResolver::load_files) /// so the variant lookups above don't re-parse it for each candidate base. diff --git a/crates/openlogi-desktop/src/services/assets/sync.rs b/crates/openlogi-desktop/src/services/assets/sync.rs index 5d5470ada..b65a18702 100644 --- a/crates/openlogi-desktop/src/services/assets/sync.rs +++ b/crates/openlogi-desktop/src/services/assets/sync.rs @@ -169,26 +169,33 @@ fn sync_depot( warn!(depot, error = %e, "buttons render fetch failed"); } - // Optional second pass: download the manifest-mapped render PNGs — the + // Optional second pass: download the manifest-mapped resources — the // colour variant matching the device's `extended_model_id` for the front - // (gallery) and side / buttons (mouse-model) views, plus the camera hero + // (gallery) and side / buttons (mouse-model) views, the camera hero // (`device_camera_image` — camera depots ship no bare `front*.png`, so the - // baseline fetch above brings no render for them at all). Failure is - // non-fatal — `AssetResolver.load_files` falls back to whatever landed. + // baseline fetch above brings no render for them at all), and this + // variant's hotspot metadata (`image_metadata`, the only place a depot + // with handed variants names its metadata file). Failure is non-fatal — + // `AssetResolver.load_files` falls back to whatever landed. let manifest_path = dir.join("manifest.json"); for resource_key in [ "device_image", "device_buttons_image", "device_camera_image", + "image_metadata", ] { - let Some(variant) = - pick_variant_filename(&manifest_path, &entry.model_id, ext, resource_key) - else { + let Some(variant) = pick_variant_filename(&manifest_path, entry, ext, resource_key) else { continue; }; if matches!( variant.as_str(), - "front_core.png" | "front.png" | "side_core.png" | "side.png" + "front_core.png" + | "front.png" + | "side_core.png" + | "side.png" + | "core_metadata.json" + | "metadata_full.json" + | "metadata.json" ) { continue; } @@ -230,9 +237,13 @@ fn fetch_to_cache( /// needed for depots whose base render isn't a baseline `front*.png` (the /// caller's skip list keeps already-fetched baseline names from re-fetching). /// `None` when the manifest is missing, malformed, or lacks the variant. +/// +/// Every model id the depot answers to is tried as the variant base, the +/// same way the resolver does: a manifest is keyed on whichever pid Logi +/// authored it against, which isn't always the index primary. fn pick_variant_filename( manifest_path: &Path, - base_model_id: &str, + entry: &DeviceEntry, ext: u8, resource_key: &str, ) -> Option { @@ -242,8 +253,9 @@ fn pick_variant_filename( let manifest = DepotManifest::load_from(manifest_path) .map_err(|e| warn!(error = %e, path = %manifest_path.display(), "manifest unreadable")) .ok()?; - manifest - .resource_for_variant(base_model_id, ext, resource_key) + entry + .model_id_candidates() + .find_map(|base| manifest.resource_for_variant(base, ext, resource_key)) .map(str::to_string) }