From: Mark Walters <markwalters1009@gmail.com>
To: Matt Armstrong <marmstrong@google.com>,
notmuch@notmuchmail.org, Tomi Ollila <tomi.ollila@iki.fi>
Subject: Re: [PATCH] emacs: notmuch-show: remove extraneous shell quoting
Date: Fri, 16 Sep 2016 20:36:27 +0100 [thread overview]
Message-ID: <87lgyrbps4.fsf@qmul.ac.uk> (raw)
In-Reply-To: <qf5sht0fkdf.fsf@marmstrong-linux.kir.corp.google.com>
Hi
On Fri, 16 Sep 2016, Matt Armstrong <marmstrong@google.com> wrote:
> Tomi, thanks for your reply asking for some motivation behind this
> patch. I can't reply directly to your message because, for some reason,
> it doesn't appear in my mailbox (I discovered your message while reading
> the mail archive on notmuchmail.org).
>
> The code dealing with this quoting issue was last touched in commit
> b57d9635f50d5e9b53092218e81f6d2c391c363e, where Carl recognizes the
> quoting is a bit of a hack and asks for a better fix. This is my
> attempt.
>
> I am motivated by a concern for code health. I saw the quoting, did not
> understand it, recognized it as probably wrong, investigated how the
> quotes were actually passed from Emacs to the shell, and still believed
> it wrong.
>
> I think this kind of flaw can be placed in the category of security fix.
> Quoting issues often are. But, I'm not a security person.
I think all the data being passed is generated by notmuch so I don't
see a security issue.
> By my reasoning, the rationale for the change is simple:
>
> a) It is the job of notmuch elisp to pass call-process the args in an
> appropriate manner for notmuch-command (which is always a local
> command). Because call-process takes a list of strings, and no shell is
> involved, using shell quotes is wrong. It just so happens that Xapian
> ignores the quotes, but taking advantage of that is not a great thing.
>
> b) If notmuch-command is doing something fancy, as is the case with the
> "remote" script on https://notmuchmail.org/remoteusage/, it is the job
> of that script to quote its own args properly for ssh. It looks like it
> already does this.
That one script does -- there are at least two others even on the wiki
(see the links at the bottom of the above page) -- they also seem to be
fine. But there could be other user scripts that do need the quoting.
So the question is do we mind breaking a few currently working setups
for the purpose of a mild cleanup. Since the current code is confusing I
think a comment would be in order if we don't apply this patch.
Best wishes
Mark
> So, the quoting is unnecessary on both accounts.
>
>
> Matt Armstrong <marmstrong@google.com> writes:
>
>> Remove shell quoting from notmuch-show--build-buffer. The args list
>> is ultimately passed to call-process, which passes them verbatim to
>> the subprocess (typically, notmuch). The quoting, intended for a
>> shell, is unnecessary and confusing.
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
next prev parent reply other threads:[~2016-09-16 19:36 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-14 6:20 [PATCH] emacs: notmuch-show: remove extraneous shell quoting Matt Armstrong
2016-09-14 9:51 ` Tomi Ollila
2016-09-16 6:07 ` Matt Armstrong
2016-09-16 19:36 ` Mark Walters [this message]
2016-09-16 23:22 ` Matt Armstrong
2016-09-16 20:44 ` Tomi Ollila
-- strict thread matches above, loose matches on Subject: below --
2016-09-14 6:17 Matt Armstrong
2016-09-14 6:28 ` Matt Armstrong
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=87lgyrbps4.fsf@qmul.ac.uk \
--to=markwalters1009@gmail.com \
--cc=marmstrong@google.com \
--cc=notmuch@notmuchmail.org \
--cc=tomi.ollila@iki.fi \
/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).