[PATCH] emacs: notmuch-show: remove extraneous shell quoting
Mark Walters
markwalters1009 at gmail.com
Fri Sep 16 12:36:27 PDT 2016
Hi
On Fri, 16 Sep 2016, Matt Armstrong <marmstrong at 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 at 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 at notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list