On 8/11/18 4:25 AM, Lukas Fleischer wrote:
> On Fri, 10 Aug 2018 at 23:26:28, Eli Schwartz wrote:
>> The notify script expects to see the userid followed by additional
>> arguments like the pkgbase id, however, these were getting sent swapped
>> around (presumably due to the similarity with the db connection which
>> expects them in the other order when processing SQL statements).
>>
>> As a result, some random userid with the same id as the pkgbase, got sent a
>> notification regarding some package with the same id as the real user's id.
>>
>> Signed-off-by: Eli Schwartz <eschwa...@archlinux.org>
>> ---
>>  aurweb/git/serve.py | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>> [...]
> 
> Wow, good catch!
> 
> The patch is totally correct but the suspected cause is not. I think
> thus is a regression introduced by f3b4c5c (Refactor the notification
> script, 2018-05-17) where some of the parameters where accidentally
> pushed around.
> 
> It is a shame that our test suite did not catch this but looking at the
> tests...
> 
>     "$NOTIFY" adopt 1 1 &&
>     [...]
>     "$NOTIFY" comaintainer-add 1 1 &&
>     [...]
>     "$NOTIFY" comaintainer-remove 1 1 &&
> 
> ... it is not much of a surprise either. Maybe we should try to make
> each parameter unique? I also realized that we don't seem to test disown
> notifications at all!
> 
> I will add a reference to f3b4c5c (Refactor the notification script,
> 2018-05-17) to the commit message, merge to pu and patch our live setup
> in a minute.

Looks good to me. Minor nit: s/where/were/ in the commit message.

-- 
Eli Schwartz
Bug Wrangler and Trusted User

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to