From d52470e0cd01ba0350017896b09e601489fcd845 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 20 Aug 2026 19:56:40 +0200 Subject: [PATCH] fix(offline): bind item-type filter as query parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_items built its `AND i.item_type IN (…)` fragment by interpolating each requested type into the SQL string, while `search`, `get_favorites` and `prune_stale_catalog` in the same file bind the identical filter as `?` placeholders. Follow the existing pattern so the listing query is consistent with its neighbours. The type values bind between the six parent-matching ids and the favourites user id, matching where `{type_filter}` lands in the statement. TRACES: UR-065 | DR-212 | UT-206 --- src-tauri/src/repository/offline.rs | 140 +++++++++++++++++++++++++--- 1 file changed, 127 insertions(+), 13 deletions(-) diff --git a/src-tauri/src/repository/offline.rs b/src-tauri/src/repository/offline.rs index 9c24725c..c8c4a2fa 100644 --- a/src-tauri/src/repository/offline.rs +++ b/src-tauri/src/repository/offline.rs @@ -1245,20 +1245,22 @@ impl MediaRepository for OfflineRepository { _ => "i.sort_name ASC, i.name ASC", }; - // Build type filter for optional filtering - let type_filter = if let Some(include_item_types) = &opts.include_item_types { - if !include_item_types.is_empty() { - let types = include_item_types - .iter() - .map(|t| format!("'{}'", t)) - .collect::>() - .join(","); - format!(" AND i.item_type IN ({})", types) - } else { - String::new() - } - } else { + // Bind the type filter rather than interpolating it: `include_item_types` + // is settable straight from the frontend (GenericMediaListPage passes it), + // so a quote in a type must be data, not syntax. Same shape as `search` + // and `get_favorites`. + // + // TRACES: UR-065 | DR-212 | UT-206 + let type_values: &[String] = opts + .include_item_types + .as_deref() + .filter(|types| !types.is_empty()) + .unwrap_or(&[]); + let type_filter = if type_values.is_empty() { String::new() + } else { + let placeholders = vec!["?"; type_values.len()].join(","); + format!(" AND i.item_type IN ({})", placeholders) }; // Favourites narrowing for a normal library listing. Bound rather than @@ -1350,6 +1352,11 @@ impl MediaRepository for OfflineRepository { QueryParam::String(parent_id.to_string()), // i.series_id = ? QueryParam::String(parent_id.to_string()), // libraries.id = ? ]; + // Positional order matters: the type placeholders sit in `{type_filter}`, + // which the statement interpolates immediately after the parent-matching + // group and before `{favorites_filter}`, so they bind here — after the + // six ids above, before the favourites user id. + params.extend(type_values.iter().cloned().map(QueryParam::String)); if !favorites_filter.is_empty() { params.push(QueryParam::String(self.user_id.clone())); // ud.user_id = ? } @@ -4481,6 +4488,113 @@ mod tests { assert_eq!(ids, vec!["movie-fav"]); } + /// UT-206 — `include_item_types` reaches the listing query as bound + /// parameters, so a type name can only ever be compared as data. + /// + /// Interpolated, the type below closed the `IN (` list and commented out the + /// rest of the line, leaving `... AND i.item_type IN ('Movie') OR 1=1`, which + /// is true for every row — the listing then returned the whole cache + /// regardless of parent or type. Bound, it is just a type name that matches + /// nothing. + /// + /// TRACES: UR-065 | DR-212 | UT-206 + #[tokio::test] + async fn test_get_items_type_filter_is_bound_not_interpolated() { + let _guard = lock_catalog_browse(); + set_include_catalog_browse(true); + + let db_service = create_test_db(); + seed_favorites(&db_service).await; + let repo = OfflineRepository::new( + db_service, + "test-server".to_string(), + "test-user".to_string(), + ); + + let injected = repo + .get_items( + "lib-1", + Some(GetItemsOptions { + include_item_types: Some(vec!["Movie') OR 1=1 --".to_string()]), + ..Default::default() + }), + ) + .await + .expect("a hostile type name must be data, not a broken query"); + assert!( + injected.items.is_empty(), + "no cached item has that type, so nothing may come back; got {:?}", + injected + .items + .iter() + .map(|i| i.id.as_str()) + .collect::>() + ); + + // A quote on its own is likewise just a character in a type name. + let quoted = repo + .get_items( + "lib-1", + Some(GetItemsOptions { + include_item_types: Some(vec!["Mo'vie".to_string()]), + ..Default::default() + }), + ) + .await + .expect("an embedded quote must not break the query"); + assert!(quoted.items.is_empty()); + } + + /// UT-206 — binding the type filter must not disturb the positions of the + /// parameters around it: the parent ids bind before it and the favourites + /// user id after it. A misordered vec would silently compare `user_id` + /// against `item_type`, so this asserts the filters still compose. + /// + /// TRACES: UR-065, UR-067 | DR-212 | UT-206 + #[tokio::test] + async fn test_get_items_binds_multiple_types_in_parameter_order() { + let _guard = lock_catalog_browse(); + set_include_catalog_browse(true); + + let db_service = create_test_db(); + seed_favorites(&db_service).await; + let repo = OfflineRepository::new( + db_service, + "test-server".to_string(), + "test-user".to_string(), + ); + + let both = repo + .get_items( + "lib-1", + Some(GetItemsOptions { + include_item_types: Some(vec!["Movie".to_string(), "MusicAlbum".to_string()]), + ..Default::default() + }), + ) + .await + .unwrap(); + let mut ids: Vec<&str> = both.items.iter().map(|i| i.id.as_str()).collect(); + ids.sort(); + assert_eq!(ids, vec!["album-fav", "movie-fav", "movie-plain"]); + + // Two type placeholders *and* the favourites parameter after them. + let favourites = repo + .get_items( + "lib-1", + Some(GetItemsOptions { + include_item_types: Some(vec!["Movie".to_string(), "MusicAlbum".to_string()]), + favorites_only: Some(true), + ..Default::default() + }), + ) + .await + .unwrap(); + let mut ids: Vec<&str> = favourites.items.iter().map(|i| i.id.as_str()).collect(); + ids.sort(); + assert_eq!(ids, vec!["album-fav", "movie-fav"]); + } + /// UT-102 — caching a server result mirrors its favourite state locally, /// but never over a row still waiting to be pushed. ///