Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()

[Tomi Ollila <tomi.ollila@iki.fi>]

On Wed, May 19 2021, Felipe Contreras wrote:

> On Tue, May 18, 2021 at 12:55 AM Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>
>> notmuch_passwd_sanitize() in test-lib.sh is too generic, it cannot
>> work in many cases...
>>
>> The more specific version _libconfig_sanitize() replaces it in
>> T590-libconfig.sh and the code that uses it is modified to output
>> the keys (ascending numbers printed in hex) so the sanitizer knows
>> what to sanitize in which lines...
>>
>> "@" + fqdn -> "@FQDN" replacement is used as fqdn could --
>> in theory -- be substring of 'USERNAME'.
>>
>> 'user -> 'USER_FULL_NAME replacement to work in cases where user
>> is empty -- as only first ' is replaced that works as expected.
>>
>> In addition to ".(none)" now also ".localdomain" is filtered from
>> USERNAME@FQDN.
>> ---
>>
>> Changes to [v2]:
>>
>> * work in cases of empty user (e.g. in passwd gecos field)
>> * replace only 1st match; e.g. fqdn could contain substring of user
>>
>> v2: id:20210517193315.11343-1-tomi.ollila@iki.fi
>> v1: id:20210502181535.31292-1-tomi.ollila@iki.fi
>>
>> When tried w/ one replacement and w/o sq usage and emptied gecos, got
>>
>> .  @@ -9,5 +9,5 @@
>> .  7: 'true'
>> .  8: 'USERNAME@FQDN'
>> .  9: 'NULL'
>> . -a: 'USER_FULL_NAME'
>> . +USER_FULL_NAMEa: ''
>>
>>  test/T590-libconfig.sh | 97 +++++++++++++++++++++++++-----------------
>>  test/test-lib.sh       | 20 ---------
>>  2 files changed, 59 insertions(+), 58 deletions(-)
>>
>> diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
>> index 745e1bb4..42cbe6e0 100755
>> --- a/test/T590-libconfig.sh
>> +++ b/test/T590-libconfig.sh
>> @@ -5,6 +5,26 @@ test_description="library config API"
>>
>>  add_email_corpus
>>
>> +_libconfig_sanitize() {
>> +    ${NOTMUCH_PYTHON} -c '
>> +import os, sys, pwd, socket
>
> Why not use a heredoc?
>
>   python <<-EOF
>   ..
>   EOF

tldr: I'll post change to use heredoc.

Probably my bias against heredoc's when there are alternatives
-- although this is much more tolerable than cat <<EOF ... EOF
to write stuff to stdout ;)

Also, I did not recall it is this simple to read python code
from stdin to be executed (right, that loses stdin for user
input -- I was once bitten by that so that contributes to 
my bias).

Here the ability to use "'" (for clarity) is compelling reason
to use heredoc.

(alternatives would have been:

 * l.replace("'\''" + name ...

 * l.replace("'"'"'" + name ... ;D

 * use other delimiter than ' (but not unicode quotes >;)
)

While testing this option I looked (once again) how dash and
bash do heredocs (in linux) (just to update my knowledge):

dash creates pipe and dup2's fd[0] to 0 
     (that makes stdin not seekable)

bash clones subprocess; in subprocess it creates temporary file,
     writes data there, closes it, opens it for reading, 
     unlinks it from fs (could be problematic on windows), 
     dup2()'s it to stdin, closes the dupped fd and finally
     execve's python

zsh works like bash (i.e. these 2 provide seekable stdin)

$ (strace -f -ofile zsh heredoc-test.sh)

Tomi

>
>> +pw = pwd.getpwuid(os.getuid())
>> +user = pw.pw_name
>> +name = pw.pw_gecos.partition(",")[0]
>> +fqdn = socket.getaddrinfo(socket.gethostname(), 0, 0,
>> +                          socket.SOCK_STREAM, 0, socket.AI_CANONNAME)[0][3]
>> +for l in sys.stdin:
>> +    if l[:3] == "8: ":
>> +        l = l.replace(user, "USERNAME", 1).replace("@" + fqdn, "@FQDN", 1)
>> +        l = l.replace(".(none)", "", 1).replace(".localdomain", "", 1)
>> +    elif l[:3] == "a: ":
>> +        sq = chr(39) # single quote
>> +        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)
>
> Then we can simply do:
>
> l.replace("'" + name, "'USER_FULL_NAME", 1)
>
> The rest looks fine to me.
>
> -- 
> Felipe Contreras