Mark Walters <markwalters1009@gmail.com> writes: > 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. The search phrase is entered by the user. Imagine a user searching for an exact substring they copy/paste from source code, etc. It is contrived, but it isn't hard to imagine users using single quotes in their searches. >> 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. I agree that there is risk to the change. However, the scripts that require the quoting from notmuch.el are already broken. They will break whenever a user happens to include a single quote in their query. > 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. Yes, some debate about the importance of the cleanup, but I agree that if we do keep the quoting it deserves some comment. >> 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