An IncludeItemTypes value that does not match is dropped rather than narrowing, and matching is case-sensitive #181

Closed
opened 2026-08-22 17:01:28 +02:00 by Claude · 1 comment
Collaborator

Found on a person's page in the throwaway instance in TESTCONTAINER/, which
draws an Audiobooks section holding two music tracks. The library has no
audiobooks.

What it looks like

The pinned web client asks the person page's audiobook section for

GET /Users/{userId}/Items?IncludeItemTypes=Audiobook&Recursive=true
    &PersonIds=…&SortBy=ProductionYear,SortName&Limit=10

— Audiobook, with a small b, which is how jellyfin-web spells it. The enum
member is AudioBook. We do not match it, and rather than narrowing to nothing
we drop the filter and answer with everything the rest of the query allows.

What answers

Same request, one person, only the type name changed:

IncludeItemTypes answered
Audiobook 2 — an Audio and a MusicAlbum
AudioBook 0
movie 2 — an Audio and a MusicAlbum
NotAThing 2 — an Audio and a MusicAlbum
omitted 2 — an Audio and a MusicAlbum

So the matching is case-sensitive, and an unmatched value is silently the same
as asking for no narrowing at all.

Upstream

ASP.NET binds a BaseItemKind through EnumConverter, which parses
case-insensitively — Audiobook, audiobook and AudioBook are one value —
and a value that is no member of the enum fails model binding, which on a
[ApiController] is a 400, not a wider answer.

Where to start

Wherever IncludeItemTypes is parsed into item types. Two changes: fold case
when matching, and decide what an unmatched value is — upstream's answer is a
400, and quietly widening the query is the one answer it cannot be. The same
question applies to ExcludeItemTypes, where dropping an unmatched value
excludes nothing.

How to know it is done

IncludeItemTypes=Audiobook and IncludeItemTypes=audiobook answer exactly
what AudioBook answers, an unmatched value is refused rather than widening,
and the person page's Audiobooks section is empty for a library with no
audiobooks. bundle.json is the scenario written by the client rather than by
us, so this is the kind of request it should be carrying.

House rules

Tests run under cargo nextest; upstream's source is the checkout in
upstream/ at the pin. Generated artifacts are regenerated, never edited.

Found on a person's page in the throwaway instance in `TESTCONTAINER/`, which draws an **Audiobooks** section holding two music tracks. The library has no audiobooks. ## What it looks like The pinned web client asks the person page's audiobook section for ``` GET /Users/{userId}/Items?IncludeItemTypes=Audiobook&Recursive=true &PersonIds=…&SortBy=ProductionYear,SortName&Limit=10 ``` — `Audiobook`, with a small `b`, which is how jellyfin-web spells it. The enum member is `AudioBook`. We do not match it, and rather than narrowing to nothing we drop the filter and answer with everything the rest of the query allows. ## What answers Same request, one person, only the type name changed: | `IncludeItemTypes` | answered | |---|---| | `Audiobook` | 2 — an `Audio` and a `MusicAlbum` | | `AudioBook` | 0 | | `movie` | 2 — an `Audio` and a `MusicAlbum` | | `NotAThing` | 2 — an `Audio` and a `MusicAlbum` | | *omitted* | 2 — an `Audio` and a `MusicAlbum` | So the matching is case-sensitive, and an unmatched value is silently the same as asking for no narrowing at all. ## Upstream ASP.NET binds a `BaseItemKind` through `EnumConverter`, which parses case-insensitively — `Audiobook`, `audiobook` and `AudioBook` are one value — and a value that is no member of the enum fails model binding, which on a `[ApiController]` is a `400`, not a wider answer. ## Where to start Wherever `IncludeItemTypes` is parsed into item types. Two changes: fold case when matching, and decide what an unmatched value is — upstream's answer is a `400`, and quietly widening the query is the one answer it cannot be. The same question applies to `ExcludeItemTypes`, where dropping an unmatched value excludes nothing. ## How to know it is done `IncludeItemTypes=Audiobook` and `IncludeItemTypes=audiobook` answer exactly what `AudioBook` answers, an unmatched value is refused rather than widening, and the person page's Audiobooks section is empty for a library with no audiobooks. `bundle.json` is the scenario written by the client rather than by us, so this is the kind of request it should be carrying. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin**. Generated artifacts are regenerated, never edited.
Author
Collaborator

Fixed in 1995867, with a review's comment notes in db0bad8.

Half of what this issue claimed was wrong, and measuring it is what showed
that. Against the reference instance at the pin:

IncludeItemTypes=Movie      -> 4      IncludeItemTypes=Audiobook -> 0
IncludeItemTypes=movie      -> 4      IncludeItemTypes=AudioBook -> 0
IncludeItemTypes=MOVIE      -> 4      IncludeItemTypes=NotAThing -> 19  (the whole library)

So the case-insensitivity is real and is fixed. The refusal is not:
upstream's CommaDelimitedCollectionModelBinder catches the conversion failure
per element and keeps the ones that converted, so a value naming no variant is
answered as if it were not there — which is what this server already did. The
400 this issue asked for would have been the deviation.

ExcludeItemTypes binds through the same binder and is now case-insensitive
with it.

One thing a review turned up, not filed because no client sends it: upstream's
binder also accepts the underlying integer, so IncludeItemTypes=13 names a
variant there and names none here.

Fixed in `1995867`, with a review's comment notes in `db0bad8`. Half of what this issue claimed was wrong, and measuring it is what showed that. Against the reference instance at the pin: ``` IncludeItemTypes=Movie -> 4 IncludeItemTypes=Audiobook -> 0 IncludeItemTypes=movie -> 4 IncludeItemTypes=AudioBook -> 0 IncludeItemTypes=MOVIE -> 4 IncludeItemTypes=NotAThing -> 19 (the whole library) ``` So the case-insensitivity is real and is fixed. **The refusal is not**: upstream's `CommaDelimitedCollectionModelBinder` catches the conversion failure per element and keeps the ones that converted, so a value naming no variant is answered as if it were not there — which is what this server already did. The `400` this issue asked for would have been the deviation. `ExcludeItemTypes` binds through the same binder and is now case-insensitive with it. One thing a review turned up, not filed because no client sends it: upstream's binder also accepts the underlying integer, so `IncludeItemTypes=13` names a variant there and names none here.
Sign in to join this conversation.
No description provided.