alex/ephyra
Watch
1
0
Fork
You've already forked ephyra
 0
Code Issues 2 Pull requests Projects Releases Packages 1 Wiki Activity Actions

/Items: six id filters are accepted and never applied, genreIds matches nothing, and a parentless listing returns the whole library #91

New issue
Closed
opened 2026-08-13 22:40:46 +02:00 by Claude · 1 comment
Claude commented 2026-08-13 22:40:46 +02:00
Collaborator
Copy link

Found while replaying crates/jellyfin-harness/scenarios/clients.json — the scenario derived from Swiftfin's and Findroid's own source. Six id-valued filters of /Items are accepted and never applied, one is applied and matches nothing, and a listing that names no parent returns the wrong set. Swiftfin sends three of these on ordinary screens, so this is not a corner.

None of it is hypothetical: every line below was measured on a live pair on 2026-08-13, reference (v12.0-rc5, 6d501ba4) on :8096 and ours on :8097, fixture library, Recursive=true unless stated.

What differs

request upstream ours
/Items?Recursive=true&ids=<Alien> 1 22
/Items?Recursive=true&excludeItemIds=<Alien> 21 22
/Items?Recursive=true&personIds=<Ridley Scott> 1 22
/Items?Recursive=true&studioIds=<Brandywine> 1 22
/Items?Recursive=true&genreIds=<Science Fiction> 2 0
/Items (no parentId, not recursive) 6 22

albumIds and artistIds are the same shape as studioIds and are untested only because the fixture's music library is thin.

22 is every row in the library. A filter that matches everything is worse than one that errors: a client asking "which items are these three ids" gets the whole library back and displays it.

Why

The six that are ignored

Params in crates/jellyfin-http/src/routes/items.rs:37 has no field for ids, personIds, studioIds, albumIds, artistIds or excludeItemIds. The struct is #[serde(default)] with no deny_unknown_fields, so the parameters are accepted and dropped in silence.

Two of them need no new plumbing at all — jellyfin_core::query::ItemQuery already carries item_ids (crates/jellyfin-core/src/query.rs:24) and exclude_item_ids (:45), and crates/jellyfin-db/src/query.rs already translates both. They are simply never populated from this route. item_ids is populated on two other paths (items.rs:405 and :466), which is why the field exists and passes tests.

Upstream:

  • Jellyfin.Api/Controllers/ItemsController.cs:206,230,235,239,240,256 — each bound with CommaDelimitedCollectionModelBinder, so comma-delimited (the doc comment on albumIds/studioIds says pipe-delimited; the binder is comma. Trust the binder).
  • Assigned to the query at ItemsController.cs:426-445.
  • Applied in Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs: ItemIds :942, ExcludeItemIds :947, PersonIds :363, AlbumIds :584, StudioIds :620.

Note AlbumIds is not a by-name lookup — :586 is e.ParentId.HasValue && filter.AlbumIds.Contains(e.ParentId.Value). PersonIds :366 goes through PeopleBaseItemMap. StudioIds goes through WhereReferencedItem, same as genres — see below.

genreIds, which is applied and wrong

items.rs:736 maps the parameter straight into ItemQuery::item_value_ids, which crates/jellyfin-core/src/query.rs documents as "by ItemValues.Id". But genreIds names the genre's own BaseItems id, not an ItemValues row. Two different id spaces, so nothing ever matches — hence 0 against upstream's 2.

Upstream resolves it in WhereReferencedItem, src/Jellyfin.Database/Jellyfin.Database.Implementations/JellyfinQueryHelperExtensions.cs:130. Read the remark above it before porting — it is load-bearing:

Matching is on CleanName alone. Genre/artist/album etc items do not set an ItemValue of their own type, so the referenced item's Type is never consulted and ids whose names clean to the same value are interchangeable across types.

So the lookup is: id → that BaseItems row's CleanName → ItemValues of the given type(s) with that CleanValue → ItemValuesMap. A studio id and a genre id that clean to the same string select the same rows upstream, and reproducing that is the point.

The parentless listing

ItemsController.cs:304-313: when no parentId resolves to a folder, upstream substitutes _libraryManager.GetUserRootFolder() and lists its children — the libraries plus Playlists — rather than everything. Ours returns the whole library. This is Findroid's first library request, so it is the first screen it draws.

Reproducing

Both servers are normally up; if not, cargo xtask reference up and ./target/debug/jellyfin-server config.toml.

tok() { curl -s -X POST "http://127.0.0.1:$1/Users/AuthenticateByName" \
  -H 'Content-Type: application/json' \
  -H 'Authorization: MediaBrowser Client="p", Device="p", DeviceId="p", Version="1"' \
  -d '{"Username":"jellyfin","Pw":"jellyfin"}' \
  | python3 -c 'import sys,json;print(json.load(sys.stdin)["AccessToken"])'; }
UT=$(tok 8096); OT=$(tok 8097)
ALIEN=1d1b052a6ce2c7f930ea6ab81f263023
for p in 8096:$UT 8097:$OT; do
  curl -s "http://127.0.0.1:${p%%:*}/Items?Recursive=true&ids=$ALIEN" \
    -H "Authorization: MediaBrowser Token=\"${p#*:}\"" \
    | python3 -c 'import sys,json;print(json.load(sys.stdin)["TotalRecordCount"])'
done

Item ids are path-derived and identical on both servers, so an id from one can be sent to the other. Alien is 1d1b052a6ce2c7f930ea6ab81f263023, Science Fiction 0430d514c35e1f158fa93f1eeb8361ea, Brandywine Productions d0e2aad659a9d003ce81400a2980f955, Ridley Scott a19d5c522c32f2b3ad5f203c978a3cbf.

Done when

  • All six parameters are bound comma-delimited and applied, genreIds matches by cleaned name through ItemValues, and a parentless non-recursive /Items lists the user root's children.
  • A route test per parameter that fails without the change.
  • clients.json replays with these differences gone: target/release/jellyfin-harness replay --upstream http://127.0.0.1:8096 --ours http://127.0.0.1:8097 --scenario crates/jellyfin-harness/scenarios/clients.json --out /tmp/run.
  • JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace green (2092 at the time of writing).

Before you start

  • CLAUDE.md is the contract. cargo nextest, not cargo test. Comments describe the current state, never the history, and carry no milestone terminology.
  • Read upstream at upstream/ in this repo — it is the pinned checkout. Not ../jellyfin, which is on a divergent branch.
  • Never git add -A, and never symlink reference/var, reference/snapshots, reference/media or reference/published into a worktree — they are gitignored, and committing such a link deletes the real fixture. That has happened.
  • Do not write to the reference on :8096. To exercise a write path against it, post to an id that does not exist: 404 means the body was accepted, 400 means it was refused.
Found while replaying `crates/jellyfin-harness/scenarios/clients.json` — the scenario derived from Swiftfin's and Findroid's own source. Six id-valued filters of `/Items` are accepted and never applied, one is applied and matches nothing, and a listing that names no parent returns the wrong set. Swiftfin sends three of these on ordinary screens, so this is not a corner. None of it is hypothetical: every line below was measured on a live pair on 2026-08-13, reference (`v12.0-rc5`, `6d501ba4`) on `:8096` and ours on `:8097`, fixture library, `Recursive=true` unless stated. ## What differs | request | upstream | ours | |---|---|---| | `/Items?Recursive=true&ids=<Alien>` | **1** | **22** | | `/Items?Recursive=true&excludeItemIds=<Alien>` | **21** | **22** | | `/Items?Recursive=true&personIds=<Ridley Scott>` | **1** | **22** | | `/Items?Recursive=true&studioIds=<Brandywine>` | **1** | **22** | | `/Items?Recursive=true&genreIds=<Science Fiction>` | **2** | **0** | | `/Items` (no `parentId`, not recursive) | **6** | **22** | `albumIds` and `artistIds` are the same shape as `studioIds` and are untested only because the fixture's music library is thin. 22 is every row in the library. A filter that matches everything is worse than one that errors: a client asking "which items are these three ids" gets the whole library back and displays it. ## Why ### The six that are ignored `Params` in `crates/jellyfin-http/src/routes/items.rs:37` has **no field** for `ids`, `personIds`, `studioIds`, `albumIds`, `artistIds` or `excludeItemIds`. The struct is `#[serde(default)]` with no `deny_unknown_fields`, so the parameters are accepted and dropped in silence. Two of them need no new plumbing at all — `jellyfin_core::query::ItemQuery` already carries `item_ids` (`crates/jellyfin-core/src/query.rs:24`) and `exclude_item_ids` (`:45`), and `crates/jellyfin-db/src/query.rs` already translates both. They are simply never populated from this route. `item_ids` *is* populated on two other paths (`items.rs:405` and `:466`), which is why the field exists and passes tests. Upstream: - `Jellyfin.Api/Controllers/ItemsController.cs:206,230,235,239,240,256` — each bound with `CommaDelimitedCollectionModelBinder`, so **comma-delimited** (the doc comment on `albumIds`/`studioIds` says pipe-delimited; the binder is comma. Trust the binder). - Assigned to the query at `ItemsController.cs:426-445`. - Applied in `Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs`: `ItemIds` `:942`, `ExcludeItemIds` `:947`, `PersonIds` `:363`, `AlbumIds` `:584`, `StudioIds` `:620`. Note `AlbumIds` is **not** a by-name lookup — `:586` is `e.ParentId.HasValue && filter.AlbumIds.Contains(e.ParentId.Value)`. `PersonIds` `:366` goes through `PeopleBaseItemMap`. `StudioIds` goes through `WhereReferencedItem`, same as genres — see below. ### `genreIds`, which is applied and wrong `items.rs:736` maps the parameter straight into `ItemQuery::item_value_ids`, which `crates/jellyfin-core/src/query.rs` documents as "by `ItemValues.Id`". But `genreIds` names **the genre's own `BaseItems` id**, not an `ItemValues` row. Two different id spaces, so nothing ever matches — hence 0 against upstream's 2. Upstream resolves it in `WhereReferencedItem`, `src/Jellyfin.Database/Jellyfin.Database.Implementations/JellyfinQueryHelperExtensions.cs:130`. Read the remark above it before porting — it is load-bearing: > Matching is on CleanName alone. Genre/artist/album etc items do not set an ItemValue of their own type, so the referenced item's Type is never consulted and ids whose names clean to the same value are interchangeable across types. So the lookup is: id → that `BaseItems` row's `CleanName` → `ItemValues` of the given type(s) with that `CleanValue` → `ItemValuesMap`. A studio id and a genre id that clean to the same string select the same rows upstream, and reproducing that is the point. ### The parentless listing `ItemsController.cs:304-313`: when no `parentId` resolves to a folder, upstream substitutes `_libraryManager.GetUserRootFolder()` and lists **its children** — the libraries plus Playlists — rather than everything. Ours returns the whole library. This is Findroid's *first* library request, so it is the first screen it draws. ## Reproducing Both servers are normally up; if not, `cargo xtask reference up` and `./target/debug/jellyfin-server config.toml`. ```sh tok() { curl -s -X POST "http://127.0.0.1:$1/Users/AuthenticateByName" \ -H 'Content-Type: application/json' \ -H 'Authorization: MediaBrowser Client="p", Device="p", DeviceId="p", Version="1"' \ -d '{"Username":"jellyfin","Pw":"jellyfin"}' \ | python3 -c 'import sys,json;print(json.load(sys.stdin)["AccessToken"])'; } UT=$(tok 8096); OT=$(tok 8097) ALIEN=1d1b052a6ce2c7f930ea6ab81f263023 for p in 8096:$UT 8097:$OT; do curl -s "http://127.0.0.1:${p%%:*}/Items?Recursive=true&ids=$ALIEN" \ -H "Authorization: MediaBrowser Token=\"${p#*:}\"" \ | python3 -c 'import sys,json;print(json.load(sys.stdin)["TotalRecordCount"])' done ``` Item ids are path-derived and identical on both servers, so an id from one can be sent to the other. `Alien` is `1d1b052a6ce2c7f930ea6ab81f263023`, Science Fiction `0430d514c35e1f158fa93f1eeb8361ea`, Brandywine Productions `d0e2aad659a9d003ce81400a2980f955`, Ridley Scott `a19d5c522c32f2b3ad5f203c978a3cbf`. ## Done when - All six parameters are bound comma-delimited and applied, `genreIds` matches by cleaned name through `ItemValues`, and a parentless non-recursive `/Items` lists the user root's children. - A route test per parameter that fails without the change. - `clients.json` replays with these differences gone: `target/release/jellyfin-harness replay --upstream http://127.0.0.1:8096 --ours http://127.0.0.1:8097 --scenario crates/jellyfin-harness/scenarios/clients.json --out /tmp/run`. - `JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace` green (2092 at the time of writing). ## Before you start - `CLAUDE.md` is the contract. `cargo nextest`, not `cargo test`. Comments describe the current state, never the history, and carry no milestone terminology. - Read upstream at `upstream/` in this repo — it is the pinned checkout. **Not** `../jellyfin`, which is on a divergent branch. - Never `git add -A`, and never symlink `reference/var`, `reference/snapshots`, `reference/media` or `reference/published` into a worktree — they are gitignored, and committing such a link deletes the real fixture. That has happened. - Do not write to the reference on `:8096`. To exercise a write path against it, post to an id that does not exist: `404` means the body was accepted, `400` means it was refused.
Claude commented 2026-08-14 00:38:51 +02:00
Author
Collaborator
Copy link

Done, on main.

commit
ca86d19 items: the six ids a listing names, and the name a genre is reached through
54f4b75 items: the libraries a listing that named nothing answers with
937381e harness: the id filters a replay holds, and the listing above the libraries
5eabf87 items: the order a listing answers the rows it named in
e506618 items: the filters a listing drops, which still keep it off the libraries

Every request in the issue's table now answers what upstream answers, measured on the live pair and re-measured independently by a reviewer:

request upstream before after
ids=<Alien> 1 22 1
excludeItemIds=<Alien> 21 22 21
personIds=<Ridley Scott> 1 22 1
studioIds=<Brandywine> 1 22 1
genreIds=<Science Fiction> 2 0 2
artistIds=<Sine Ensemble> 3 22 3
albumIds=<First Signal> 2 22 2
/Items (no parent) 6 22 6

albumIds and artistIds are measured rather than assumed. The issue expected the fixture's music library to be too thin for them; it is not — artistIds returning 3 is the Artist ∪ AlbumArtist union doing real work, and albumIds=<a film> answers 0 on both.

genreIds goes through the WhereReferencedItem shape rather than an id space of its own: ItemValues.CleanValue matched against the referenced row's CleanName, with the referenced row's Type never consulted. The remark above JellyfinQueryHelperExtensions.cs:130 is the rule and is restated where the code implements it. A route test pins the cross-type case with a Studio row named "Science Fiction" whose id, given as genreIds, returns the genre's films. That leg is held by the fixture rather than by the pair: the reference library has no name collision across types, and making one is a write.

What the review changed

A listing naming several ids came back sorted, where upstream returns the rows in the order the ids were named (Folder.GetItems:996-1008, when ItemIds.Length > 1 and OrderBy is empty). The scenario written to guard this change pinned SortBy=SortName, which is exactly the condition that turns upstream's request-order sort off — so it could not have caught it. It now names its ids out of sort-name order with no SortBy, and replaying the new scenario against the pre-fix binary reports 301 differences over 4 requests. The sortBy=Nonsense corner falls out for free on both servers: an enum binder that drops a name it cannot read leaves OrderBy empty, and the request order applies.

The new parentless branch answered the wrong kind of thing. narrows() stands in for HasFilters, and it can only see the parameters this route binds — so any filter the route does not bind sent the request to the libraries branch, and a client asking for science-fiction films got CollectionFolder rows to render as library tiles. It now also reads the raw query for the 53 names HasFilters counts and the route does not bind. maxOfficialRating is measured rather than assumed to be one of them: upstream counts the minimum parental rating and not the maximum, so it correctly still reaches the libraries.

Those requests now answer a superset of items rather than library tiles — the pre-existing unbound-filter gap, filed as #120. Genres= and nameLessThan= are deliberately not in the scenario, because they still differ for that reason and a committed scenario that cannot replay clean is worse than no coverage.

The widening this introduces, faithfully

narrow_to_every_library now exempts a query naming item_ids, which is LibraryManager.cs:2019-2026's own precondition for setting TopParentIds. Measured: an account with EnableAllFolders=false can reach a row outside its libraries by id — on both servers. Upstream has the same hole and the controller calls the DTO layer with skipVisibilityCheck: true. Reproduced rather than fixed, and it is the second half of #111.

The exemption is shared with movies.rs and by_name.rs, which pass through the same choke point; ca86d19's message says so.

Numbers

JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace on main: 2129 passed, 0 skipped. Clippy clean. items.json went 12 → 28 requests at 0 differences. clients.json — the scenario that found this — went from 264 differences over 12 requests to 98 over 7, and every one of the seven is a separately filed defect or an environmental difference the harness README already names.

No ignore-list entry was added. ignore.rs and both lists are untouched; the README's move of two findings is out of clients.json's open list into items.json, which measures them at 0.

Browser pass on the worktree binary: the person and studio pages now list one film each, agreeing with upstream. The genre page is still wrong, and not because of this: the pinned bundle sends Genres=, never GenreIds=, so it lands on the unbound-filter gap — #120, which is where it belongs and where nothing but driving a browser would have found it.

Five defects this turned up are filed: #120, #121, #122, #123, #124.

Done, on `main`. | commit | | |---|---| | `ca86d19` | items: the six ids a listing names, and the name a genre is reached through | | `54f4b75` | items: the libraries a listing that named nothing answers with | | `937381e` | harness: the id filters a replay holds, and the listing above the libraries | | `5eabf87` | items: the order a listing answers the rows it named in | | `e506618` | items: the filters a listing drops, which still keep it off the libraries | Every request in the issue's table now answers what upstream answers, measured on the live pair and re-measured independently by a reviewer: | request | upstream | before | after | |---|---|---|---| | `ids=<Alien>` | 1 | 22 | 1 | | `excludeItemIds=<Alien>` | 21 | 22 | 21 | | `personIds=<Ridley Scott>` | 1 | 22 | 1 | | `studioIds=<Brandywine>` | 1 | 22 | 1 | | `genreIds=<Science Fiction>` | 2 | 0 | 2 | | `artistIds=<Sine Ensemble>` | 3 | 22 | 3 | | `albumIds=<First Signal>` | 2 | 22 | 2 | | `/Items` (no parent) | 6 | 22 | 6 | **`albumIds` and `artistIds` are measured rather than assumed.** The issue expected the fixture's music library to be too thin for them; it is not — `artistIds` returning 3 is the Artist ∪ AlbumArtist union doing real work, and `albumIds=<a film>` answers 0 on both. `genreIds` goes through the `WhereReferencedItem` shape rather than an id space of its own: `ItemValues.CleanValue` matched against the referenced row's `CleanName`, with the referenced row's `Type` never consulted. The remark above `JellyfinQueryHelperExtensions.cs:130` is the rule and is restated where the code implements it. A route test pins the cross-type case with a **Studio** row named "Science Fiction" whose id, given as `genreIds`, returns the genre's films. That leg is held by the fixture rather than by the pair: the reference library has no name collision across types, and making one is a write. ## What the review changed **A listing naming several ids came back sorted, where upstream returns the rows in the order the ids were named** (`Folder.GetItems:996-1008`, when `ItemIds.Length > 1` and `OrderBy` is empty). The scenario written to guard this change pinned `SortBy=SortName`, which is exactly the condition that turns upstream's request-order sort off — so it could not have caught it. It now names its ids out of sort-name order with no `SortBy`, and replaying the new scenario against the pre-fix binary reports 301 differences over 4 requests. The `sortBy=Nonsense` corner falls out for free on both servers: an enum binder that drops a name it cannot read leaves `OrderBy` empty, and the request order applies. **The new parentless branch answered the wrong *kind* of thing.** `narrows()` stands in for `HasFilters`, and it can only see the parameters this route binds — so any filter the route does not bind sent the request to the libraries branch, and a client asking for science-fiction films got `CollectionFolder` rows to render as library tiles. It now also reads the raw query for the 53 names `HasFilters` counts and the route does not bind. `maxOfficialRating` is measured rather than assumed to be one of them: upstream counts the minimum parental rating and not the maximum, so it correctly still reaches the libraries. Those requests now answer a superset of items rather than library tiles — the pre-existing unbound-filter gap, filed as #120. `Genres=` and `nameLessThan=` are deliberately **not** in the scenario, because they still differ for that reason and a committed scenario that cannot replay clean is worse than no coverage. ## The widening this introduces, faithfully `narrow_to_every_library` now exempts a query naming `item_ids`, which is `LibraryManager.cs:2019-2026`'s own precondition for setting `TopParentIds`. Measured: an account with `EnableAllFolders=false` can reach a row outside its libraries **by id** — on both servers. Upstream has the same hole and the controller calls the DTO layer with `skipVisibilityCheck: true`. Reproduced rather than fixed, and it is the second half of #111. The exemption is shared with `movies.rs` and `by_name.rs`, which pass through the same choke point; `ca86d19`'s message says so. ## Numbers `JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace` on `main`: **2129 passed, 0 skipped**. Clippy clean. `items.json` went 12 → 28 requests at 0 differences. `clients.json` — the scenario that found this — went from **264 differences over 12 requests to 98 over 7**, and every one of the seven is a separately filed defect or an environmental difference the harness README already names. **No ignore-list entry was added.** `ignore.rs` and both lists are untouched; the README's move of two findings is out of `clients.json`'s open list into `items.json`, which measures them at 0. Browser pass on the worktree binary: the person and studio pages now list one film each, agreeing with upstream. The **genre page is still wrong**, and not because of this: the pinned bundle sends `Genres=`, never `GenreIds=`, so it lands on the unbound-filter gap — #120, which is where it belongs and where nothing but driving a browser would have found it. Five defects this turned up are filed: #120, #121, #122, #123, #124.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
No results found.
Labels
Clear labels   
correctness
Diverges from upstream

  
encoding
Transcoding, ffmpeg, and the arg strings

  
http
Route behaviour

  
jobs
Scheduled tasks

  
operations
Installing a server and keeping one running

  
p1
Do this first

  
p2
After the p1s

  
p3
Real, and not urgent

  
performance
Speed or resource use

  
providers
Metadata from somewhere other than the file

  
scanner
The library scan

  
security
Who is allowed to see what

  
testing
Test coverage and harness

No labels
correctness
encoding
http
jobs
operations
p1
p2
p3
performance
providers
scanner
security
testing
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
alex/ephyra#91
No description provided.