Re: [PATCH 01/13] test: fix passwd_sanitize()

Subject: Re: [PATCH 01/13] test: fix passwd_sanitize()

Date: Sat, 01 May 2021 23:01:24 +0300

To: Felipe Contreras, notmuch@notmuchmail.org

Cc:

From: Tomi Ollila


On Sat, May 01 2021, Felipe Contreras wrote:

> If any of the variables is empty the output is completely messed up,
> because replace("", "FOO") puts "FOO" before every single character.
>
> I don't have my full name configured, and this is what I get:
>
>   USER_FULL_NAME=USER_FULL_NAME=USER_FULL_NAME USER_FULL_NAMEsUSER_FULL_NAMEtUSER_FULL_NAMEdUSER_FULL_NAMEoUSER_FULL_NAMEuUSER_FULL_NAMEtUSER_FULL_NAME USER_FULL_NAME=USER_FULL_NAME=USER_FULL_NAME
>
> Let's check for empty strings before doing any replace.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  test/test-lib.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 4c9f2a21..e13797a7 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -711,7 +711,12 @@ name = pw.pw_gecos.partition(",")[0]
>  fqdn = socket.getfqdn()
>  
>  for l in sys.stdin:
> -    l = l.replace(user, "USERNAME").replace(fqdn, "FQDN").replace(".(none)","").replace(name, "USER_FULL_NAME")
> +    if user:
> +        l = l.replace(user, "USERNAME")
> +    if fqdn:
> +        l = l.replace(fqdn, "FQDN").replace(".(none)","")
> +    if name:
> +        l = l.replace(name, "USER_FULL_NAME")

This looks like a good change.

This made me think of something. When I quickly deviced the initial code of
this I thinkoed that str.replace() replaces only the first match -- now
that I tested (in python3 repl) it replaces *all* matches...

In my home machines I usually have both "username" and "user_full_name"
(using the terms used in the sanitizer) as 'too'...

Currently all is lost in USER_FULL_NAME replacement -- all 'too's are
replaced with USERNAME and no USER_FULL_NAME replacements happen.
If we had l.replace(user, "USERNAME", 1) then only the first match were
replaced -- and if both matches are expected to happen in same line --
the "full name" replacement later in line, then this change would help
in such a cases. If these replacements are to be done in different lines
then the USERNAME replacement would always be done and nothing helps
there (except more specific replacement code)...

And, now as this chance of having empty username come into our
understanding, instead of empty, but some short (or why not longer) strings 
that just happen to be (sub)strings of the text it gets as input we get
unwanted replacements and test failures... :/

Tomi


>      sys.stdout.write(l)
>  '
>  }
> -- 
> 2.31.0
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: