sanitization of args notmuch-cli in notmuch-emacs

sanitization of args notmuch-cli in notmuch-emacs

[David Bremner]

Antoine Beaupré found a bug when notmuch-saved-searches contains 
newlines:

        http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=737496

We can remove newlines with something like
          
(mapcar (lambda (arg) 
	  (replace-regexp-in-string "\n" " " arg))
	args)

I wonder if we should do some other sanitization at the same time?

d

          

   

Re: sanitization of args notmuch-cli in notmuch-emacs

[Tomi Ollila]

On Mon, Feb 03 2014, David Bremner <david@tethera.net> wrote:

> Antoine Beaupré found a bug when notmuch-saved-searches contains 
> newlines:
>
>         http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=737496
>
> We can remove newlines with something like
>           
> (mapcar (lambda (arg) 
> 	  (replace-regexp-in-string "\n" " " arg))
> 	args)
>
> I wonder if we should do some other sanitization at the same time?

It took a while to reproduce...

$ echo $'foo\nbar' | notmuch count --batch 
665
631

$ echo $'foo\n and bar' | notmuch count --batch
665
A Xapian exception occurred: Syntax: <expression> AND <expression>
Query string was:  and bar
0

Therefore: (wrong-type-argument number-or-marker-p A)

(I run non-byte-compiled version of (one-) notmuch.el and
got this as a backtrace:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p A)
  >(A 0)
  (or (plist-get options :show-empty-searches) (> message-count 0))
...
  (notmuch-remove-if-not
...
  notmuch-hello-query-counts
...

simpler way to reproduce:

$ notmuch count 'and bar'
A Xapian exception occurred: Syntax: <expression> AND <expression>
Query string was: and bar
0

Maybe the cli should be fixed ? (and/or make emacs MUA resilient to
this kind of result)

>
> d
>

Tomi

Re: sanitization of args notmuch-cli in notmuch-emacs

[Tomi Ollila]

On Mon, Feb 03 2014, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Mon, Feb 03 2014, David Bremner <david@tethera.net> wrote:
>
>> Antoine Beaupré found a bug when notmuch-saved-searches contains 
>> newlines:
>>
>>         http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=737496
>>
>> We can remove newlines with something like
>>           
>> (mapcar (lambda (arg) 
>> 	  (replace-regexp-in-string "\n" " " arg))
>> 	args)
>>
>> I wonder if we should do some other sanitization at the same time?
>
> It took a while to reproduce...
>
> $ echo $'foo\nbar' | notmuch count --batch 
> 665
> 631
>
> $ echo $'foo\n and bar' | notmuch count --batch
> 665
> A Xapian exception occurred: Syntax: <expression> AND <expression>
> Query string was:  and bar
> 0
>
> Therefore: (wrong-type-argument number-or-marker-p A)
>
> (I run non-byte-compiled version of (one-) notmuch.el and
> got this as a backtrace:
>
> Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p A)
>   >(A 0)
>   (or (plist-get options :show-empty-searches) (> message-count 0))
> ...
>   (notmuch-remove-if-not
> ...
>   notmuch-hello-query-counts
> ...
>
> simpler way to reproduce:
>
> $ notmuch count 'and bar'
> A Xapian exception occurred: Syntax: <expression> AND <expression>
> Query string was: and bar
> 0
>
> Maybe the cli should be fixed ? (and/or make emacs MUA resilient to
> this kind of result)

of yes, the notmuch count --batch gets borken with \n -- from one
query there are 2 new count queries (and counts should be all wrong
from that point on (if there were no Xapian exception))

We would not have noticed this (as easily) if the Xapian execption did not
happen.

So, whether (or not?) some fixing is done in CLI the batch query emacs
sends needs to be sanitized exactly as David suggested above.

>
>>
>> d
>>
>
> Tomi

Tomi

[PATCH] emacs: remove newlines from input to notmuch count --batch

[David Bremner]

Since a newline starts a new query in batch mode, this causes
mysterious crashes in the emacs interface if saved searches contain
newlines.  See the discussion at

      id:87wqhcxb5j.fsf@maritornes.cs.unb.ca

In general newlines seem to be just whitespace to the xapian query
parser, so this should be mainly harmless.
---
 emacs/notmuch-hello.el | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 7b3d76b..e325cd3 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -399,10 +399,12 @@ options will be handled as specified for
 			     (third elem)
 			   (cdr elem))))
 	(insert
-	 (notmuch-hello-filtered-query count-query
-				       (or (plist-get options :filter-count)
-					   (plist-get options :filter)))
-	 "\n")))
+	 (replace-regexp-in-string
+	  "\n" " "
+	  (notmuch-hello-filtered-query count-query
+					(or (plist-get options :filter-count)
+					    (plist-get options :filter))))
+	  "\n")))
 
     (unless (= (call-process-region (point-min) (point-max) notmuch-command
 				    t t nil "count" "--batch") 0)
-- 
1.8.5.2

Re: [PATCH] emacs: remove newlines from input to notmuch count --batch

[Tomi Ollila]

On Sat, Feb 08 2014, David Bremner <david@tethera.net> wrote:

> Since a newline starts a new query in batch mode, this causes
> mysterious crashes in the emacs interface if saved searches contain
> newlines.  See the discussion at
>
>       id:87wqhcxb5j.fsf@maritornes.cs.unb.ca
>
> In general newlines seem to be just whitespace to the xapian query
> parser, so this should be mainly harmless.
> ---

LGTM.

Tomi


>  emacs/notmuch-hello.el | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index 7b3d76b..e325cd3 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -399,10 +399,12 @@ options will be handled as specified for
>  			     (third elem)
>  			   (cdr elem))))
>  	(insert
> -	 (notmuch-hello-filtered-query count-query
> -				       (or (plist-get options :filter-count)
> -					   (plist-get options :filter)))
> -	 "\n")))
> +	 (replace-regexp-in-string
> +	  "\n" " "
> +	  (notmuch-hello-filtered-query count-query
> +					(or (plist-get options :filter-count)
> +					    (plist-get options :filter))))
> +	  "\n")))
>  
>      (unless (= (call-process-region (point-min) (point-max) notmuch-command
>  				    t t nil "count" "--batch") 0)
> -- 
> 1.8.5.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: remove newlines from input to notmuch count --batch

[David Bremner]

David Bremner <david@tethera.net> writes:

> Since a newline starts a new query in batch mode, this causes
> mysterious crashes in the emacs interface if saved searches contain
> newlines.  See the discussion at
>
>       id:87wqhcxb5j.fsf@maritornes.cs.unb.ca
>

pushed, 

d

Re: sanitization of args notmuch-cli in notmuch-emacs

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> Maybe the cli should be fixed ? (and/or make emacs MUA resilient to
> this kind of result)
>

Would it make any sense to output errors in structured format?  I guess
the downside is it would be harder for a human user to read.

d

Re: sanitization of args notmuch-cli in notmuch-emacs

[Mark Walters]

On Tue, 04 Feb 2014, David Bremner <david@tethera.net> wrote:
> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>
>> Maybe the cli should be fixed ? (and/or make emacs MUA resilient to
>> this kind of result)
>>
>
> Would it make any sense to output errors in structured format?  I guess
> the downside is it would be harder for a human user to read.


If we want to do something like this would something like
--structured-errors be possible?

Best wishes

Mark

Re: sanitization of args notmuch-cli in notmuch-emacs

[Tomi Ollila]

On Tue, Feb 04 2014, David Bremner <david@tethera.net> wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>
>> Maybe the cli should be fixed ? (and/or make emacs MUA resilient to
>> this kind of result)
>>
>
> Would it make any sense to output errors in structured format?  I guess
> the downside is it would be harder for a human user to read.

notmuch count --batch outputs just numbers separated by newline -- and
with emacs stdout & stderr are in the same stream (in this case too)
For that we'd need structured like notmuch count --batch --format=sexp
and then write errors there...

I don't think we can find SomeOne(tm) to do this -- or the überversion
like 'notmuch execute' which takes sexp/json document in stdin and
spits out sexp/json document for programs to parse ;D

> d

Romi

Re: sanitization of args notmuch-cli in notmuch-emacs

[David Bremner]

David Bremner <david@tethera.net> writes:

> Antoine Beaupré found a bug when notmuch-saved-searches contains 
> newlines:
>
>         http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=737496
>
> We can remove newlines with something like
>           
> (mapcar (lambda (arg) 
> 	  (replace-regexp-in-string "\n" " " arg))
> 	args)
>
> I wonder if we should do some other sanitization at the same time?
>

This should be fixed in commit 75d84df

d