/UserItems/Resume applies parameters upstream's resume action never binds #124
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
/UserItems/Resumesharesbuildwith/Itemsincrates/jellyfin-http/src/routes/items.rs, so it reads and appliesgenreIds,years,nameStartsWith,filters,isFavorite,isPlayed,seriesIdandparentIndexNumber. Upstream'sGetResumeItemsbinds none of them: they are not in the action's signature, so a client sending one gets it ignored and we narrow on it.This is argued from the two signatures, not measured. The reviewer tried and could not: the fixture has no resume rows, so both servers answer an empty page whatever is sent. Confirming it needs a playback position written to the reference first —
crates/jellyfin-harness/scenarios/playstate.jsonis the scenario that writes one, and the harness README's ordering rules apply, since a read scenario running first warms the cache that makes a replay unrepeatable.Two of the six new id filters were deliberately kept off this path during #91 for exactly this reason, so the precedent is set; what is left is the ones that predate it.
Where to start
crates/jellyfin-http/src/routes/items.rs— the sharedbuild, and which of its parameters each caller is allowed to reach. Upstream:ItemsController.GetResumeItems.The shape to prefer: make the route say what it binds rather than sharing everything and subtracting, since subtracting is what left this behind.
How to know it is done
With a resume position written, a request naming one of these parameters answers the same on both servers, and the scenario that writes the position carries the check.
House rules
Tests run under
cargo nextest; upstream's source is the checkout inupstream/at the pin (
v12.0-rc5,6d501ba4); generated files are regenerated ratherthan edited; comments say what the code does, not how it got there. The
reference pair is
cargo xtask reference up, a run iscargo run -p jellyfin-harness -- replay --upstream URL --ours URL --scenario FILE --out DIR,and
crates/jellyfin-harness/README.mdsays what a diff means. A server startedoff current
mainbinds UDP 7359 for discovery — set[discovery] enabled = falsein a scratch config if another one is already running.
Done, on
main./UserItems/Resumesharedbuildwith/Items, so it applied filters upstream'sGetResumeItemsnever binds. Each route now declares what upstream's action takes — sharing everything and subtracting is what left this behind.GetResumeItemstakes fifteen parameters. Thirteen are read;imageTypeLimitandenableImageTypesare ignored here as they are on/Items. The audit was one short on the first pass:excludeActiveSessionsis a genuine narrowing parameter and was neither bound nor registered, so a client sending it got back in-progress items upstream would have removed. It is bound now, and expands each active session's item to every version — without that, an alternate cut of what is playing stays in the listing, which is exactly the row a client would offer to resume.And the route was missing most of a Continue Watching row.
TranslateQuery.cs:495-527appliesfolderIsResumableFilterunconditionally wheneverIsResumableis set, over[Series, Season]. Ours gated it on the request having asked for series, and a comment claimed that was what upstream branches on — it does not. Measured on an enriched pair: 2 rows against upstream's 4, now 4 and identical —Test Pattern S01E03, Night Signal, Season 1, Test Pattern. The descendant test runs throughAncestorIdsrather thanSeriesId, which is what lets one expression answer for a season as well as the series above it.The batch this closed in
Seven issues went out together because they looked like one defect class. They turned out to be three roots:
/Items'Paramshad been grown one client at a time instead of held against upstream's controller signature, and/UserItems/Resumeshared it.sortBy=Namemaps toCleanNameupstream, notSortName, and a by-name search orders by relevance with no sort-name tiebreak, so ties fall out in id order.PremiereDatecoalesces withProductionYearwhere ours read the bare column.A reviewer re-derived the parameter audit mechanically — extracting every
[FromQuery]name fromGetItemsand doing set arithmetic against what we bind — and got nothing bound that upstream does not take and nothing in the register that is not an upstream parameter. This class is closed rather than narrowed, which matters because it had been found three times.Verified on a restored pair:
browsing34 → 0,items33 requests at 0,clients56 → 3,portable115 → 6.JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace: 2228 passed, 0 skipped. Clippy clean. No ignore-list entry was added or touched.Two things the review caught that the batch had not
Five of the ten new scenario entries could not fail. The
defaultsnapshot has no genres, studios or tags, so fiveitems.jsonentries compared two empty pages — reverting the whole fix would still have replayed clean. They moved intoediting.json, after the save that gives one film a genre, a tag, a certificate and a studio and then takes them away, so each selects one row out of nineteen. That is the fifth time this session the same failure has turned up in a different layer, after M5's vacuous scenarios, M8'sbundle.json, #111's all-refusalsvisibility.jsonand #137's silently-skipping tests.Reading the reference can write to it.
Studios=,Artists=andAlbums=resolve through upstream'sGetStudio/GetArtist, which callCreateItemByName— so a read files aBaseItemsrow. That is how a strayBrandywine Productionsgot into the shared fixture mid-session and madeportable.jsondisagree by 149.Genres,TagsandOfficialRatingsnarrow throughItemValuesand are safe. Both halves are now written intocrates/jellyfin-harness/README.md.