unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Tomi Ollila <tomi.ollila@iki.fi>
To: Felipe Contreras <felipe.contreras@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH 01/13] test: fix passwd_sanitize()
Date: Sat, 01 May 2021 23:01:24 +0300	[thread overview]
Message-ID: <m2o8du4417.fsf@guru.guru-group.fi> (raw)
In-Reply-To: <20210501115422.483314-2-felipe.contreras@gmail.com>

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

  reply	other threads:[~2021-05-01 20:01 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-01 11:54 [PATCH 00/13] test: several fixes and improvements Felipe Contreras
2021-05-01 11:54 ` [PATCH 01/13] test: fix passwd_sanitize() Felipe Contreras
2021-05-01 20:01   ` Tomi Ollila [this message]
2021-05-01 11:54 ` [PATCH 02/13] test: unset NAME environment variable Felipe Contreras
2021-05-01 20:01   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 03/13] test: remove USER_FULL_NAME when not present Felipe Contreras
2021-05-01 20:13   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 04/13] test: use correct fqdn in passwd_sanitize() Felipe Contreras
2021-05-01 20:14   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 05/13] test: fix wrong SKIP messages Felipe Contreras
2021-05-01 20:18   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 06/13] test: add prereqs check in test_emacs_expect_t Felipe Contreras
2021-05-01 20:18   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 07/13] test: add external prereqs to many emacs tests Felipe Contreras
2021-05-01 20:19   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 08/13] test: split emacs functionality to its own file Felipe Contreras
2021-05-01 20:28   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 09/13] test: emacs: simplify missing dependencies check Felipe Contreras
2021-05-01 20:20   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 10/13] test: emacs: check for configured emacs Felipe Contreras
2021-05-01 20:20   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 11/13] test: emacs: fix a couple of shellcheck complaints Felipe Contreras
2021-05-01 20:21   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 12/13] test: trivial style cleanups Felipe Contreras
2021-05-01 20:28   ` Tomi Ollila
2021-05-01 11:54 ` [PATCH 13/13] test: more style fixes Felipe Contreras
2021-05-01 20:29   ` Tomi Ollila
2021-05-02  0:36 ` [PATCH 00/13] test: several fixes and improvements David Bremner
2021-05-02  5:08   ` Tomi Ollila

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2o8du4417.fsf@guru.guru-group.fi \
    --to=tomi.ollila@iki.fi \
    --cc=felipe.contreras@gmail.com \
    --cc=notmuch@notmuchmail.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).