fix(offline): bind item-type filter as query parameters
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
This commit is contained in:
@@ -1245,20 +1245,22 @@ impl MediaRepository for OfflineRepository {
|
|||||||
_ => "i.sort_name ASC, i.name ASC",
|
_ => "i.sort_name ASC, i.name ASC",
|
||||||
};
|
};
|
||||||
|
|
||||||
// Build type filter for optional filtering
|
// Bind the type filter rather than interpolating it: `include_item_types`
|
||||||
let type_filter = if let Some(include_item_types) = &opts.include_item_types {
|
// is settable straight from the frontend (GenericMediaListPage passes it),
|
||||||
if !include_item_types.is_empty() {
|
// so a quote in a type must be data, not syntax. Same shape as `search`
|
||||||
let types = include_item_types
|
// and `get_favorites`.
|
||||||
.iter()
|
//
|
||||||
.map(|t| format!("'{}'", t))
|
// TRACES: UR-065 | DR-212 | UT-206
|
||||||
.collect::<Vec<_>>()
|
let type_values: &[String] = opts
|
||||||
.join(",");
|
.include_item_types
|
||||||
format!(" AND i.item_type IN ({})", types)
|
.as_deref()
|
||||||
} else {
|
.filter(|types| !types.is_empty())
|
||||||
String::new()
|
.unwrap_or(&[]);
|
||||||
}
|
let type_filter = if type_values.is_empty() {
|
||||||
} else {
|
|
||||||
String::new()
|
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
|
// 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()), // i.series_id = ?
|
||||||
QueryParam::String(parent_id.to_string()), // libraries.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() {
|
if !favorites_filter.is_empty() {
|
||||||
params.push(QueryParam::String(self.user_id.clone())); // ud.user_id = ?
|
params.push(QueryParam::String(self.user_id.clone())); // ud.user_id = ?
|
||||||
}
|
}
|
||||||
@@ -4481,6 +4488,113 @@ mod tests {
|
|||||||
assert_eq!(ids, vec!["movie-fav"]);
|
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::<Vec<_>>()
|
||||||
|
);
|
||||||
|
|
||||||
|
// 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,
|
/// UT-102 — caching a server result mirrors its favourite state locally,
|
||||||
/// but never over a row still waiting to be pushed.
|
/// but never over a row still waiting to be pushed.
|
||||||
///
|
///
|
||||||
|
|||||||
Reference in New Issue
Block a user