dtourolle/jellytau
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 40 Wiki Activity

fix(repository): bind query parameters and encode URL values

Browse Source 
Three consistency fixes, each one applying a pattern the same file already
used a few lines away: the offline get_items type filter now binds placeholders
like search at offline.rs:1786 does, build_get_items_endpoint percent-encodes
its values like the Genres block below it does, and player_set_volume clamps
NaN and out-of-range input at the command boundary rather than relying on each
backend to do it.

TRACES: | DR-212 | UT-206
This commit is contained in:
dtourolle
2026-08-20 20:02:57 +02:00
parent fb6bd5cae1 080cdbf383
commit b313b61717
3 changed files with 337 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src-tauri/src/repository/offline.rs
+127 -13
View File
@@ -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::<Vec<_>>()
.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::<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,
/// but never over a row still waiting to be pushed.
///
src-tauri/src/repository/online.rs
+149 -19
View File
@@ -235,7 +235,11 @@ impl OnlineRepository {
item_id: &str,
t: f64,
) -> Result<Vec<JRayActor>, RepoError> {
let endpoint = format!("/Plugins/JRay/Items/{}/jray?t={}", item_id, t);
let endpoint = format!(
"/Plugins/JRay/Items/{}/jray?t={}",
urlencoding::encode(item_id),
t
);
match self.get_json::<JRayContext>(&endpoint).await {
Ok(context) => Ok(context.actors),
// No plugin / no truth data for this item — not an error to the user.
@@ -802,7 +806,17 @@ fn build_get_items_endpoint(
parent_id: &str,
options: Option<&GetItemsOptions>,
) -> String {
let mut endpoint = format!("/Users/{}/Items?ParentId={}", user_id, parent_id);
// Every value below is percent-encoded before it goes into the query
// string, the same way `Genres` and `SearchTerm` already are: these are
// values, not URL syntax, so a space or an `&` in one must not split it
// into another parameter.
//
// TRACES: UR-007 | DR-212 | UT-206
let mut endpoint = format!(
"/Users/{}/Items?ParentId={}",
user_id,
urlencoding::encode(parent_id)
);
if let Some(opts) = options {
if let Some(limit) = opts.limit {
@@ -812,13 +826,25 @@ fn build_get_items_endpoint(
endpoint.push_str(&format!("&StartIndex={}", start_index));
}
if let Some(types) = &opts.include_item_types {
endpoint.push_str(&format!("&IncludeItemTypes={}", types.join(",")));
// Encode each type, not the joined string: the comma is the
// list separator Jellyfin splits on.
let encoded: Vec<String> = types
.iter()
.map(|t| urlencoding::encode(t).into_owned())
.collect();
endpoint.push_str(&format!("&IncludeItemTypes={}", encoded.join(",")));
}
if let Some(sort_by) = &opts.sort_by {
endpoint.push_str(&format!("&SortBy={}", sort_by));
// SortBy is likewise a comma-delimited list (`hybrid.rs` sends
// "ParentIndexNumber,IndexNumber,SortName"), so encode per field.
let encoded: Vec<String> = sort_by
.split(',')
.map(|field| urlencoding::encode(field).into_owned())
.collect();
endpoint.push_str(&format!("&SortBy={}", encoded.join(",")));
}
if let Some(sort_order) = &opts.sort_order {
endpoint.push_str(&format!("&SortOrder={}", sort_order));
endpoint.push_str(&format!("&SortOrder={}", urlencoding::encode(sort_order)));
}
if let Some(recursive) = opts.recursive {
endpoint.push_str(&format!("&Recursive={}", recursive));
@@ -1147,7 +1173,7 @@ impl MediaRepository for OnlineRepository {
///
/// TRACES: UR-021, UR-035 | IR-016, IR-022, JA-005, JA-009
async fn get_item(&self, item_id: &str) -> Result<MediaItem, RepoError> {
let endpoint = format!("/Users/{}/Items/{}?Fields=BackdropImageTags,ParentBackdropImageTags,People,MediaStreams,MediaSources,PremiereDate,UserData", self.user_id, item_id);
let endpoint = format!("/Users/{}/Items/{}?Fields=BackdropImageTags,ParentBackdropImageTags,People,MediaStreams,MediaSources,PremiereDate,UserData", self.user_id, urlencoding::encode(item_id));
let item: JellyfinItem = self.get_json(&endpoint).await?;
let media_item = item.into_media_item(self.user_id.clone());
@@ -1510,7 +1536,7 @@ impl MediaRepository for OnlineRepository {
}
async fn get_playback_info(&self, item_id: &str) -> Result<PlaybackInfo, RepoError> {
let endpoint = format!("/Items/{}/PlaybackInfo", item_id);
let endpoint = format!("/Items/{}/PlaybackInfo", urlencoding::encode(item_id));
#[derive(Debug, Serialize)]
#[serde(rename_all = "PascalCase")]
@@ -1939,7 +1965,7 @@ impl MediaRepository for OnlineRepository {
live_stream_id: Option<String>,
}
let endpoint = format!("/Items/{}/PlaybackInfo", item_id);
let endpoint = format!("/Items/{}/PlaybackInfo", urlencoding::encode(item_id));
let request = OpenLiveStreamRequest {
user_id: self.user_id.clone(),
auto_open_live_stream: true,
@@ -2214,7 +2240,11 @@ impl MediaRepository for OnlineRepository {
}
async fn mark_favorite(&self, item_id: &str) -> Result<(), RepoError> {
let endpoint = format!("/Users/{}/FavoriteItems/{}", self.user_id, item_id);
let endpoint = format!(
"/Users/{}/FavoriteItems/{}",
self.user_id,
urlencoding::encode(item_id)
);
self.post_json(&endpoint, &serde_json::json!({})).await
}
@@ -2244,7 +2274,11 @@ impl MediaRepository for OnlineRepository {
///
/// TRACES: UR-017 | JA-018, DR-021
async fn unmark_favorite(&self, item_id: &str) -> Result<(), RepoError> {
let endpoint = format!("/Users/{}/FavoriteItems/{}", self.user_id, item_id);
let endpoint = format!(
"/Users/{}/FavoriteItems/{}",
self.user_id,
urlencoding::encode(item_id)
);
let url = format!("{}{}", self.server_url, endpoint);
let result = async {
@@ -2286,7 +2320,11 @@ impl MediaRepository for OnlineRepository {
///
/// TRACES: UR-064 | DR-106, JA-033
async fn clear_watch_history(&self, item_id: &str) -> Result<(), RepoError> {
let endpoint = format!("/Users/{}/PlayedItems/{}", self.user_id, item_id);
let endpoint = format!(
"/Users/{}/PlayedItems/{}",
self.user_id,
urlencoding::encode(item_id)
);
let url = format!("{}{}", self.server_url, endpoint);
let result = async {
@@ -2327,7 +2365,11 @@ impl MediaRepository for OnlineRepository {
///
/// TRACES: UR-025 | DR-131 | JA-035
async fn mark_played(&self, item_id: &str) -> Result<(), RepoError> {
let endpoint = format!("/Users/{}/PlayedItems/{}", self.user_id, item_id);
let endpoint = format!(
"/Users/{}/PlayedItems/{}",
self.user_id,
urlencoding::encode(item_id)
);
let url = format!("{}{}", self.server_url, endpoint);
let result = async {
@@ -2372,7 +2414,11 @@ impl MediaRepository for OnlineRepository {
///
/// TRACES: UR-035, UR-036 | IR-022, JA-030
async fn get_person(&self, person_id: &str) -> Result<MediaItem, RepoError> {
let endpoint = format!("/Users/{}/Items/{}", self.user_id, person_id);
let endpoint = format!(
"/Users/{}/Items/{}",
self.user_id,
urlencoding::encode(person_id)
);
let item: JellyfinItem = self.get_json(&endpoint).await?;
Ok(item.into_media_item(self.user_id.clone()))
}
@@ -2461,7 +2507,7 @@ impl MediaRepository for OnlineRepository {
async fn delete_playlist(&self, playlist_id: &str) -> Result<(), RepoError> {
info!("[OnlineRepo] Deleting playlist {}", playlist_id);
let endpoint = format!("/Items/{}", playlist_id);
let endpoint = format!("/Items/{}", urlencoding::encode(playlist_id));
let url = format!("{}{}", self.server_url, endpoint);
let request = self
@@ -2496,7 +2542,7 @@ impl MediaRepository for OnlineRepository {
"[OnlineRepo] Renaming playlist {} to '{}'",
playlist_id, name
);
let endpoint = format!("/Items/{}", playlist_id);
let endpoint = format!("/Items/{}", urlencoding::encode(playlist_id));
self.post_json(&endpoint, &serde_json::json!({ "Name": name }))
.await
}
@@ -2534,8 +2580,17 @@ impl MediaRepository for OnlineRepository {
item_ids.len(),
playlist_id
);
let ids_param = item_ids.join(",");
let endpoint = format!("/Playlists/{}/Items?Ids={}", playlist_id, ids_param);
// Encode each id, not the joined string: the comma separates the list.
let ids_param = item_ids
.iter()
.map(|id| urlencoding::encode(id).into_owned())
.collect::<Vec<_>>()
.join(",");
let endpoint = format!(
"/Playlists/{}/Items?Ids={}",
urlencoding::encode(playlist_id),
ids_param
);
self.post_json(&endpoint, &serde_json::json!({})).await
}
@@ -2549,8 +2604,16 @@ impl MediaRepository for OnlineRepository {
entry_ids.len(),
playlist_id
);
let ids_param = entry_ids.join(",");
let endpoint = format!("/Playlists/{}/Items?EntryIds={}", playlist_id, ids_param);
let ids_param = entry_ids
.iter()
.map(|id| urlencoding::encode(id).into_owned())
.collect::<Vec<_>>()
.join(",");
let endpoint = format!(
"/Playlists/{}/Items?EntryIds={}",
urlencoding::encode(playlist_id),
ids_param
);
let url = format!("{}{}", self.server_url, endpoint);
let request = self
@@ -3529,6 +3592,73 @@ mod tests {
assert!(!off.contains("Filters=IsFavorite"));
}
/// UT-206 — the values this endpoint builder puts in the query string are
/// percent-encoded, like `Genres` and `SearchTerm` already are.
///
/// Unencoded, a value carrying `&` or `=` splits into an extra query
/// parameter (a parent id containing a space produced a malformed URL
/// outright), so the request the server sees is not the one that was built.
///
/// TRACES: UR-007 | DR-212 | UT-206
#[test]
fn test_get_items_endpoint_encodes_query_values() {
let endpoint = build_get_items_endpoint(
"u1",
"lib 1&Filters=IsFavorite",
Some(&GetItemsOptions {
include_item_types: Some(vec!["Movie&x=1".to_string()]),
sort_by: Some("Sort Name".to_string()),
sort_order: Some("Ascending&y=2".to_string()),
..Default::default()
}),
);
assert!(
endpoint.contains("ParentId=lib%201%26Filters%3DIsFavorite"),
"{endpoint}"
);
assert!(
endpoint.contains("&IncludeItemTypes=Movie%26x%3D1"),
"{endpoint}"
);
assert!(endpoint.contains("&SortBy=Sort%20Name"), "{endpoint}");
assert!(
endpoint.contains("&SortOrder=Ascending%26y%3D2"),
"{endpoint}"
);
// Nothing smuggled in as a parameter of its own.
assert!(!endpoint.contains("&Filters=IsFavorite"), "{endpoint}");
assert!(!endpoint.contains("&x=1"), "{endpoint}");
assert!(!endpoint.contains("&y=2"), "{endpoint}");
}
/// The separators inside a list parameter must survive encoding: Jellyfin
/// splits `SortBy` and `IncludeItemTypes` on commas, and `hybrid.rs` sends
/// "ParentIndexNumber,IndexNumber,SortName" to order episodes.
///
/// TRACES: UR-007 | DR-212 | UT-206
#[test]
fn test_get_items_endpoint_keeps_list_separators() {
let endpoint = build_get_items_endpoint(
"u1",
"lib-1",
Some(&GetItemsOptions {
sort_by: Some("ParentIndexNumber,IndexNumber,SortName".to_string()),
include_item_types: Some(vec!["Movie".to_string(), "Series".to_string()]),
..Default::default()
}),
);
assert!(
endpoint.contains("&SortBy=ParentIndexNumber,IndexNumber,SortName"),
"{endpoint}"
);
assert!(
endpoint.contains("&IncludeItemTypes=Movie,Series"),
"{endpoint}"
);
// A plain GUID parent id is unchanged by encoding.
assert!(endpoint.contains("ParentId=lib-1"), "{endpoint}");
}
/// A newly-added album must arrive as one entry, not one per track.
///
/// Jellyfin's `/Items/Latest` defaults to `GroupItems=false`, which returns