canonical / pebble

Take control of your internal daemons!
https://canonical-pebble.readthedocs-hosted.com/
GNU General Public License v3.0
143 stars 54 forks source link

fix(daemon): handle invalid notice types in filter #329

Closed olivercalder closed 9 months ago

olivercalder commented 9 months ago

Invalid notice types are ignored when parsing the API request to construct the notice filter. However, if only invalid notice types are included in the request, discarding all types results in a filter indistinguishable from one which never had any types in the first place, and thus would return notices of any type. This commit fixes this behaviour, so no notices are returned if only invalid notice types are requested.

benhoyt commented 9 months ago

As you probably saw, we did make a conscious decision to ignore invalid notice types. The comment is:

            // Ignore invalid notice types (so requests from newer clients
            // with unknown types succeed).

But I just want to confirm what you have in mind.

When a client specifies say /v1/notices?types=custom&types=verynewtype, we want an older server to return just the notices with the type the server knows about, "custom". But if a client specifies /v1/notices?types=verynewtype&types=anothernewtype, neither of which the older server knows about, we want it to return no notices (as there of course won't be any) rather than all notices. Correct?

olivercalder commented 9 months ago

When a client specifies say /v1/notices?types=custom&types=verynewtype, we want an older server to return just the notices with the type the server knows about, "custom". But if a client specifies /v1/notices?types=verynewtype&types=anothernewtype, neither of which the older server knows about, we want it to return no notices (as there of course won't be any) rather than all notices. Correct?

Yes, that's what I intended. We want to prevent the default "all notices" behavior from kicking in if we prune all the unknown notice types and are left with no types remaining.