`docker/add-user.sh`: Don't crash on updating when there is a single user

[erjoalgo]

When a single dovecot user exists and their password is updated, the grep -v command intended to remove the user's old password will not match any lines and fail, causing the entire script to fail.

[albertito]

Hi! Thank you for sending this!

While using sed -i to remove the line seems reasonable in principle, I am not sure if I understand the bug this is fixing.

I am worried that what makes the change work for you could be the removal of the : at the end of the expression, which can be a problem because the wrong email could be removed (e.g. if you have a user abc and an abcde).

So I think I need to understand the problem better.

Can you send the contents of the users file the current script is failing on? How did you generate it?

Thank you!

[erjoalgo]

Here is an example repro script:

#!/bin/bash -x

set -e

# prelude setting up /data/dovecot/users
sudo mkdir -p /data/dovecot/
sudo chown -R $(whoami) /data
cat <<EOF > /data/dovecot/users
me@mail.example.com:{CRYPT}$2y$05$gTTsE0TKRLke8d8H/qIKhfyQf0HCdZ17mh975LdHfTMZbPhmVQ/56::::
EOF
EMAIL=me@mail.example.com

# code from add-users.sh
mkdir -p /data/dovecot
touch /data/dovecot/users
if grep -q "^${EMAIL}:" /data/dovecot/users; then
    cp /data/dovecot/users /data/dovecot/users.old
        # the next command will fail
    grep -v "^${EMAIL}:" /data/dovecot/users.old \
        > /data/dovecot/users
fi

echo "success" # this line never runs

Basically the grep -v command will fail because after removing the email in question, there are no lines left.

The removal of the : was not intended, and I've added it back and updated the PR.

[albertito]

Thank you for the details!

Here is an example repro script:

cat <<EOF > /data/dovecot/users
me@mail.example.com:{CRYPT}$2y$05$gTTsE0TKRLke8d8H/qIKhfyQf0HCdZ17mh975LdHfTMZbPhmVQ/56::::
EOF

[...]

if grep -q "^${EMAIL}:" /data/dovecot/users; then
  cp /data/dovecot/users /data/dovecot/users.old
        # the next command will fail
  grep -v "^${EMAIL}:" /data/dovecot/users.old \
      > /data/dovecot/users
fi

Basically the grep -v command will fail because after removing the email in question, there are no lines left.

Oooohhhh I see, the problem is the exit value of grep -v:

$ echo abcd:efg > xxx
$ grep -v '^abcd:' xxx; echo $?
1
$ grep -v '^pppp:' xxx; echo $?
abcd:efg
0

Thank you, now I get where the problem is. sed -i seems like a more practical way to implement this.

I'll have one comment inline on the patch, but this seems fine to me, thank you!

The removal of the : was not intended, and I've added it back and updated the PR.

Great, thanks! Glad to rule that out too.

[albertito]

Thanks a lot for the explanation and the updated patches!

I've merged them to next with minor edits to the commit messages, please take a look and tell me if there's something you prefer me to change. They are a0f09308ed2e29c1585e2bc3c7b7a26f5241008d and 0ce84a3b5d14da831011d0489b5a0f335c562cdc.

Thanks again!

[erjoalgo]

Looks great to me. Thanks for the quick response.