fix(online): percent-encode query values and path ids
build_get_items_endpoint pasted ParentId, IncludeItemTypes, SortBy and SortOrder straight into the query string while the Genres parameter twenty lines below and the SearchTerm parameter both percent-encode theirs. Encode them the same way, per list element so the commas Jellyfin splits on survive. The per-call ids interpolated into request paths (item, person and playlist ids) get the same treatment; a Jellyfin GUID is unchanged by encoding, so this is consistency, not a behaviour change. self.user_id is left alone throughout, as it is at the endpoint builders already. TRACES: UR-007 | DR-212 | UT-206
This commit is contained in:
@@ -235,7 +235,11 @@ impl OnlineRepository {
|
|||||||
item_id: &str,
|
item_id: &str,
|
||||||
t: f64,
|
t: f64,
|
||||||
) -> Result<Vec<JRayActor>, RepoError> {
|
) -> 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 {
|
match self.get_json::<JRayContext>(&endpoint).await {
|
||||||
Ok(context) => Ok(context.actors),
|
Ok(context) => Ok(context.actors),
|
||||||
// No plugin / no truth data for this item — not an error to the user.
|
// 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,
|
parent_id: &str,
|
||||||
options: Option<&GetItemsOptions>,
|
options: Option<&GetItemsOptions>,
|
||||||
) -> String {
|
) -> 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(opts) = options {
|
||||||
if let Some(limit) = opts.limit {
|
if let Some(limit) = opts.limit {
|
||||||
@@ -812,13 +826,25 @@ fn build_get_items_endpoint(
|
|||||||
endpoint.push_str(&format!("&StartIndex={}", start_index));
|
endpoint.push_str(&format!("&StartIndex={}", start_index));
|
||||||
}
|
}
|
||||||
if let Some(types) = &opts.include_item_types {
|
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 {
|
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 {
|
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 {
|
if let Some(recursive) = opts.recursive {
|
||||||
endpoint.push_str(&format!("&Recursive={}", 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
|
/// TRACES: UR-021, UR-035 | IR-016, IR-022, JA-005, JA-009
|
||||||
async fn get_item(&self, item_id: &str) -> Result<MediaItem, RepoError> {
|
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 item: JellyfinItem = self.get_json(&endpoint).await?;
|
||||||
let media_item = item.into_media_item(self.user_id.clone());
|
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> {
|
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)]
|
#[derive(Debug, Serialize)]
|
||||||
#[serde(rename_all = "PascalCase")]
|
#[serde(rename_all = "PascalCase")]
|
||||||
@@ -1939,7 +1965,7 @@ impl MediaRepository for OnlineRepository {
|
|||||||
live_stream_id: Option<String>,
|
live_stream_id: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
let endpoint = format!("/Items/{}/PlaybackInfo", item_id);
|
let endpoint = format!("/Items/{}/PlaybackInfo", urlencoding::encode(item_id));
|
||||||
let request = OpenLiveStreamRequest {
|
let request = OpenLiveStreamRequest {
|
||||||
user_id: self.user_id.clone(),
|
user_id: self.user_id.clone(),
|
||||||
auto_open_live_stream: true,
|
auto_open_live_stream: true,
|
||||||
@@ -2214,7 +2240,11 @@ impl MediaRepository for OnlineRepository {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async fn mark_favorite(&self, item_id: &str) -> Result<(), RepoError> {
|
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
|
self.post_json(&endpoint, &serde_json::json!({})).await
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2244,7 +2274,11 @@ impl MediaRepository for OnlineRepository {
|
|||||||
///
|
///
|
||||||
/// TRACES: UR-017 | JA-018, DR-021
|
/// TRACES: UR-017 | JA-018, DR-021
|
||||||
async fn unmark_favorite(&self, item_id: &str) -> Result<(), RepoError> {
|
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 url = format!("{}{}", self.server_url, endpoint);
|
||||||
|
|
||||||
let result = async {
|
let result = async {
|
||||||
@@ -2286,7 +2320,11 @@ impl MediaRepository for OnlineRepository {
|
|||||||
///
|
///
|
||||||
/// TRACES: UR-064 | DR-106, JA-033
|
/// TRACES: UR-064 | DR-106, JA-033
|
||||||
async fn clear_watch_history(&self, item_id: &str) -> Result<(), RepoError> {
|
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 url = format!("{}{}", self.server_url, endpoint);
|
||||||
|
|
||||||
let result = async {
|
let result = async {
|
||||||
@@ -2327,7 +2365,11 @@ impl MediaRepository for OnlineRepository {
|
|||||||
///
|
///
|
||||||
/// TRACES: UR-025 | DR-131 | JA-035
|
/// TRACES: UR-025 | DR-131 | JA-035
|
||||||
async fn mark_played(&self, item_id: &str) -> Result<(), RepoError> {
|
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 url = format!("{}{}", self.server_url, endpoint);
|
||||||
|
|
||||||
let result = async {
|
let result = async {
|
||||||
@@ -2372,7 +2414,11 @@ impl MediaRepository for OnlineRepository {
|
|||||||
///
|
///
|
||||||
/// TRACES: UR-035, UR-036 | IR-022, JA-030
|
/// TRACES: UR-035, UR-036 | IR-022, JA-030
|
||||||
async fn get_person(&self, person_id: &str) -> Result<MediaItem, RepoError> {
|
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?;
|
let item: JellyfinItem = self.get_json(&endpoint).await?;
|
||||||
Ok(item.into_media_item(self.user_id.clone()))
|
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> {
|
async fn delete_playlist(&self, playlist_id: &str) -> Result<(), RepoError> {
|
||||||
info!("[OnlineRepo] Deleting playlist {}", playlist_id);
|
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 url = format!("{}{}", self.server_url, endpoint);
|
||||||
|
|
||||||
let request = self
|
let request = self
|
||||||
@@ -2496,7 +2542,7 @@ impl MediaRepository for OnlineRepository {
|
|||||||
"[OnlineRepo] Renaming playlist {} to '{}'",
|
"[OnlineRepo] Renaming playlist {} to '{}'",
|
||||||
playlist_id, name
|
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 }))
|
self.post_json(&endpoint, &serde_json::json!({ "Name": name }))
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
@@ -2534,8 +2580,17 @@ impl MediaRepository for OnlineRepository {
|
|||||||
item_ids.len(),
|
item_ids.len(),
|
||||||
playlist_id
|
playlist_id
|
||||||
);
|
);
|
||||||
let ids_param = item_ids.join(",");
|
// Encode each id, not the joined string: the comma separates the list.
|
||||||
let endpoint = format!("/Playlists/{}/Items?Ids={}", playlist_id, ids_param);
|
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
|
self.post_json(&endpoint, &serde_json::json!({})).await
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2549,8 +2604,16 @@ impl MediaRepository for OnlineRepository {
|
|||||||
entry_ids.len(),
|
entry_ids.len(),
|
||||||
playlist_id
|
playlist_id
|
||||||
);
|
);
|
||||||
let ids_param = entry_ids.join(",");
|
let ids_param = entry_ids
|
||||||
let endpoint = format!("/Playlists/{}/Items?EntryIds={}", playlist_id, ids_param);
|
.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 url = format!("{}{}", self.server_url, endpoint);
|
||||||
|
|
||||||
let request = self
|
let request = self
|
||||||
@@ -3529,6 +3592,73 @@ mod tests {
|
|||||||
assert!(!off.contains("Filters=IsFavorite"));
|
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.
|
/// A newly-added album must arrive as one entry, not one per track.
|
||||||
///
|
///
|
||||||
/// Jellyfin's `/Items/Latest` defaults to `GroupItems=false`, which returns
|
/// Jellyfin's `/Items/Latest` defaults to `GroupItems=false`, which returns
|
||||||
|
|||||||
Reference in New Issue
Block a user