Subject: [bug#62298] [PATCH 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.

From: Liliana Marie Prikler <[email-masked]>
Date: Tue, 21 Mar 2023 06:30:10 +0100

Hi Bruno,

Am Dienstag, dem 21.03.2023 um 02:10 +0000 schrieb Bruno Victal:
> After giving some thought to this, IMO I think it's simply
> uninteresting for these fields to accept string values.
> Prior to the 5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1 refactor, the
> names were hardcoded and the refactor allowed them to be changed.
I think it's a little late to come to this realization.  Note how my
prior attempt at fixing 61570 was delayed for more than a month so that
a proper sanitizer can be implemented and would have had a better user
interface than what you are currently proposing.

> Accepting strings is simply uninteresting (or bad) since:
> 
> * A string doesn't uniquely identify an account and results in buggy
> behavior [1].
> 
> * Since the string values are only used to set the 'name' of the
> user-account/group records, which is specific to the service as
> they're created within the mpd-account procedure, it's simply setting
> a vanity value. It's as interesting as allowing the filename in
> (mixed-text-file "mpd.conf" ...) to be set by the user.
> 
> * It's clearly unsanitizable since it would require accessing other
> fields.  Monkeying within (mpd-accounts) with special symbols just
> obfuscates the code with no clear benefits to be had, in addition to
> defeating the point of having a sanitizer in the first place.
> 
> 
> I fail to see the utility in ever accepting strings here for what
> amounts to a vanity change in 'ps aux' output. 
Need I remind you that the original concern was about backwards API
compatibility?  Yes, accepting strings and doing things with them is
broken for the reasons you mentioned and there should be a deprecation
warning about this.  But not heeding the user values is silly and you
should still set those vanity values for the sake of vanity itself.

Cheers