Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
On Fri, May 21, 2021 at 1:22 PM Tomi Ollila wrote: > > On Fri, May 21 2021, Felipe Contreras wrote: > > > On Thu, May 20, 2021 at 2:43 AM Tomi Ollila wrote: > >> > >> On Wed, May 19 2021, Felipe Contreras wrote: > >> > >> > On Wed, May 19, 2021 at 12:34 PM Tomi Ollila wrote: > >> > > >> >> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean > >> >> reading python script from stdin don't work (perl has __DATA__ ;). > >> >> (bitten again, I did and tested the change... :D). > >> > > >> > That can be fixed with: > >> > > >> > python /dev/fd/3 3< >> > EOF > >> > >> According to > >> > >> https://unix.stackexchange.com/questions/123602/portability-of-file-descriptor-links > >> > >> that solution could be portable enough. > > > > What the operating system does doesn't really matter, bash emulates > > /dev/fd/x: > > In this case, /dev/fd/3 is given as a parameter to a command, not part of > redirections -- bash cannot know how the program is going to use the > arguments it gets... Right. I did try doing the redirection inside the python code. That also works fine, but I don't think there's a need of doing that unless there's an actual issue with /dev/fd/3. > > And as far as I know the testing framework only works correctly on bash... > > So... > > > >> Another way still using -c ... I've played to look how it actually looks is > >> (diff since patch v3) > >> > >> -sq = chr(39) # single quote > >> -l = l.replace(sq + name, sq + "USER_FULL_NAME", 1) > >> +l = l.replace("'\''" + name, "'\''USER_FULL_NAME", 1) > > > > Yes, that works too. But that's what I said in another mail that is > > weird stuff. I had to read it again three times and then copy to a > > proper text editor with monospace font to see if it was correct. > > To me '\'' is /idiomatic/ way to embed ' in the middle of a long argument > string... Maybe. I've written a lot of shell code and whenever that happens I switch to double quotes. "foo 'quote' bar" is much more readable to me than 'foo '\''quote'\'' bar'. Cheers. -- Felipe Contreras ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
On Fri, May 21 2021, Felipe Contreras wrote: > On Thu, May 20, 2021 at 2:43 AM Tomi Ollila wrote: >> >> On Wed, May 19 2021, Felipe Contreras wrote: >> >> > On Wed, May 19, 2021 at 12:34 PM Tomi Ollila wrote: >> > >> >> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean >> >> reading python script from stdin don't work (perl has __DATA__ ;). >> >> (bitten again, I did and tested the change... :D). >> > >> > That can be fixed with: >> > >> > python /dev/fd/3 3<> > EOF >> >> According to >> >> https://unix.stackexchange.com/questions/123602/portability-of-file-descriptor-links >> >> that solution could be portable enough. > > What the operating system does doesn't really matter, bash emulates /dev/fd/x: In this case, /dev/fd/3 is given as a parameter to a command, not part of redirections -- bash cannot know how the program is going to use the arguments it gets... Anyway, the [PATCH v4] in id:20210520134628.11653-1-tomi.oll...@iki.fi implemented your suggestion ${NOTMUCH_PYTHON} /dev/fd/3 3<<'EOF' ... > "If the operating system on which Bash is running provides these > special files, bash will use them; otherwise it will emulate them > internally with the behavior described below." > > https://www.gnu.org/software/bash/manual/html_node/Redirections.html ... "Bash handles several filenames specially when they are used in redirections, as described in the following table." is just before your snippet >;D > > And as far as I know the testing framework only works correctly on bash... > So... > >> Another way still using -c ... I've played to look how it actually looks is >> (diff since patch v3) >> >> -sq = chr(39) # single quote >> -l = l.replace(sq + name, sq + "USER_FULL_NAME", 1) >> +l = l.replace("'\''" + name, "'\''USER_FULL_NAME", 1) > > Yes, that works too. But that's what I said in another mail that is > weird stuff. I had to read it again three times and then copy to a > proper text editor with monospace font to see if it was correct. To me '\'' is /idiomatic/ way to embed ' in the middle of a long argument string... >> Tested the above. That python /dev/fd/3 3<> have to be tested to see how it looks like and behaves... :D > > Yeap. Took me a while to find the right documentation to figure that > out, but in my opinion it's better to write a helper for the tests > once, and then forget about it and just re-use it for all. > > -- > Felipe Contreras ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
On Thu, May 20, 2021 at 2:43 AM Tomi Ollila wrote: > > On Wed, May 19 2021, Felipe Contreras wrote: > > > On Wed, May 19, 2021 at 12:34 PM Tomi Ollila wrote: > > > >> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean > >> reading python script from stdin don't work (perl has __DATA__ ;). > >> (bitten again, I did and tested the change... :D). > > > > That can be fixed with: > > > > python /dev/fd/3 3< > EOF > > According to > > https://unix.stackexchange.com/questions/123602/portability-of-file-descriptor-links > > that solution could be portable enough. What the operating system does doesn't really matter, bash emulates /dev/fd/x: "If the operating system on which Bash is running provides these special files, bash will use them; otherwise it will emulate them internally with the behavior described below." https://www.gnu.org/software/bash/manual/html_node/Redirections.html And as far as I know the testing framework only works correctly on bash... So... > Another way still using -c ... I've played to look how it actually looks is > (diff since patch v3) > > -sq = chr(39) # single quote > -l = l.replace(sq + name, sq + "USER_FULL_NAME", 1) > +l = l.replace("'\''" + name, "'\''USER_FULL_NAME", 1) Yes, that works too. But that's what I said in another mail that is weird stuff. I had to read it again three times and then copy to a proper text editor with monospace font to see if it was correct. > Tested the above. That python /dev/fd/3 3< have to be tested to see how it looks like and behaves... :D Yeap. Took me a while to find the right documentation to figure that out, but in my opinion it's better to write a helper for the tests once, and then forget about it and just re-use it for all. -- Felipe Contreras ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
On Wed, May 19 2021, Felipe Contreras wrote: > On Wed, May 19, 2021 at 12:34 PM Tomi Ollila wrote: > >> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean >> reading python script from stdin don't work (perl has __DATA__ ;). >> (bitten again, I did and tested the change... :D). > > That can be fixed with: > > python /dev/fd/3 3< EOF According to https://unix.stackexchange.com/questions/123602/portability-of-file-descriptor-links that solution could be portable enough. Another way still using -c ... I've played to look how it actually looks is (diff since patch v3) -sq = chr(39) # single quote -l = l.replace(sq + name, sq + "USER_FULL_NAME", 1) +l = l.replace("'\''" + name, "'\''USER_FULL_NAME", 1) Tested the above. That python /dev/fd/3 3< > -- > Felipe Contreras Tomi ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
On Wed, May 19, 2021 at 12:34 PM Tomi Ollila wrote: > Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean > reading python script from stdin don't work (perl has __DATA__ ;). > (bitten again, I did and tested the change... :D). That can be fixed with: python /dev/fd/3 3<
Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
On Wed, May 19 2021, Tomi Ollila wrote: > On Wed, May 19 2021, Felipe Contreras wrote: > >> >> Why not use a heredoc? >> >> python <<-EOF >> .. >> EOF > > tldr: I'll post change to use heredoc. ... which did not work ... > Probably my bias against heredoc's when there are alternatives > -- although this is much more tolerable than cat < 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). Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean reading python script from stdin don't work (perl has __DATA__ ;). (bitten again, I did and tested the change... :D). I'd rather keep the same "filter" style here as in everywhere else in our test scripts, and chr(39) instead of exiting out from the single quoted string temporarily in order to add that/those single quote(s) to the argument for python3 -c ... So [PATCH v3] in id:20210518055443.21964-1-tomi.oll...@iki.fi is my fix proposal. Tomi > > 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 ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
On Wed, May 19 2021, Felipe Contreras wrote: > On Tue, May 18, 2021 at 12:55 AM Tomi Ollila 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.oll...@iki.fi >> v1: id:20210502181535.31292-1-tomi.oll...@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 <;) ) 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 ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
On Tue, May 18, 2021 at 12:55 AM Tomi Ollila 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.oll...@iki.fi > v1: id:20210502181535.31292-1-tomi.oll...@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 > +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 ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org