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
signature.asc
Description: OpenPGP digital signature
