[PATCH 0/2] Escape message ID queries in Emacs

[PATCH 0/2] Escape message ID queries in Emacs

[Austin Clements]

Currently, Emacs does not escape message ID queries and is
inconsistent about quoting them.  This patch centralizes this in one
function that always produces a properly quoted and escaped message ID
query.

With this, Emacs no longer gets confused by Tomi's crazy message,
  id:"id:""1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi"""

[PATCH 1/2] test: Add Emacs test for messages with quotes in their message ID

[Austin Clements]

Currently this is broken because Emacs doesn't properly escape double
quotes in message IDs.
---
 test/emacs |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 8a28705..62eaedb 100755
--- a/test/emacs
+++ b/test/emacs
@@ -139,6 +139,18 @@ test_emacs '(notmuch-search "id:\"123..456@example\"")
 output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
 
+test_begin_subtest "Message with quote in Message-Id:"
+test_subtest_known_broken
+add_message '[id]="\"quote\"@example"' '[subject]="Message with quote in Message-Id"'
+test_emacs '(notmuch-search "subject:\"Message with quote\"")
+	    (notmuch-test-wait)
+	    (execute-kbd-macro "+search-add")
+            (notmuch-search-show-thread)
+	    (notmuch-test-wait)
+	    (execute-kbd-macro "+show-add")'
+output=$(notmuch search 'id:"""quote""@example"' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with quote in Message-Id (inbox search-add show-add)"
+
 test_begin_subtest "Sending a message via (fake) SMTP"
 emacs_deliver_message \
     'Testing message sent via SMTP' \
-- 
1.7.7.2

[PATCH 2/2] emacs: Escape all message ID queries

[Austin Clements]

This adds a lib function to turn a message ID into a properly escaped
message ID query and uses this function wherever we previously
hand-constructed ID queries.  Wherever this new function is used,
documentation has been clarified to refer to "id: queries" instead of
"message IDs".

This fixes the broken test introduced by the previous patch.
---
 emacs/notmuch-lib.el     |    6 +++++-
 emacs/notmuch-message.el |    2 +-
 emacs/notmuch-show.el    |   14 +++++++-------
 test/emacs               |    1 -
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c146748..2e367b5 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -144,6 +144,10 @@ the user hasn't set this variable with the old or new value."
 	"[No Subject]"
       subject)))
 
+(defun notmuch-id-to-query (id)
+  "Return a query that matches the message with id ID."
+  (concat "id:\"" (replace-regexp-in-string "\"" "\"\"" id t t) "\""))
+
 ;;
 
 (defun notmuch-common-do-stash (text)
@@ -230,7 +234,7 @@ the given type."
 
 (defun notmuch-get-bodypart-content (msg part nth process-crypto)
   (or (plist-get part :content)
-      (notmuch-get-bodypart-internal (concat "id:" (plist-get msg :id)) nth process-crypto)))
+      (notmuch-get-bodypart-internal (notmuch-id-to-query (plist-get msg :id)) nth process-crypto)))
 
 (defun notmuch-plist-to-alist (plist)
   (loop for (key value . rest) on plist by #'cddr
diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
index 264a5b9..3010281 100644
--- a/emacs/notmuch-message.el
+++ b/emacs/notmuch-message.el
@@ -44,7 +44,7 @@ the \"inbox\" and \"todo\", you would set
 				(concat "+" str)
 			      str))
 			  notmuch-message-replied-tags)))
-	(apply 'notmuch-tag (concat "id:" (car (car rep))) tags)))))
+	(apply 'notmuch-tag (notmuch-id-to-query (car (car rep))) tags)))))
 
 (add-hook 'message-send-hook 'notmuch-message-mark-replied)
 
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0cd7d82..6d3fe62 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -613,7 +613,7 @@ current buffer, if possible."
 	  ;; times (hundreds!), which results in many calls to
 	  ;; `notmuch part'.
 	  (unless content
-	    (setq content (notmuch-get-bodypart-internal (concat "id:" message-id)
+	    (setq content (notmuch-get-bodypart-internal (notmuch-id-to-query message-id)
 							      part-number notmuch-show-process-crypto))
 	    (with-current-buffer w3m-current-buffer
 	      (notmuch-show-w3m-cid-store-internal url
@@ -1325,16 +1325,16 @@ Some useful entries are:
     (plist-get props prop)))
 
 (defun notmuch-show-get-message-id (&optional bare)
-  "Return the Message-Id of the current message.
+  "Return an id: query for the Message-Id of the current message.
 
 If optional argument BARE is non-nil, return
-the Message-Id without prefix and quotes."
+the Message-Id without id: prefix and escaping."
   (if bare
       (notmuch-show-get-prop :id)
-    (concat "id:\"" (notmuch-show-get-prop :id) "\"")))
+    (notmuch-id-to-query (notmuch-show-get-prop :id))))
 
 (defun notmuch-show-get-messages-ids ()
-  "Return all message ids of messages in the current thread."
+  "Return all id: queries of messages in the current thread."
   (let ((message-ids))
     (notmuch-show-mapc
      (lambda () (push (notmuch-show-get-message-id) message-ids)))
@@ -1401,7 +1401,7 @@ current thread."
 ;; thread.
 
 (defun notmuch-show-get-message-ids-for-open-messages ()
-  "Return a list of all message IDs for open messages in the current thread."
+  "Return a list of all id: queries for open messages in the current thread."
   (save-excursion
     (let (message-ids done)
       (goto-char (point-min))
@@ -1805,7 +1805,7 @@ thread from search."
   (notmuch-common-do-stash (notmuch-show-get-from)))
 
 (defun notmuch-show-stash-message-id ()
-  "Copy message ID of current message to kill-ring."
+  "Copy id: query matching the current message to kill-ring."
   (interactive)
   (notmuch-common-do-stash (notmuch-show-get-message-id)))
 
diff --git a/test/emacs b/test/emacs
index 62eaedb..8b92d0a 100755
--- a/test/emacs
+++ b/test/emacs
@@ -140,7 +140,6 @@ output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
 
 test_begin_subtest "Message with quote in Message-Id:"
-test_subtest_known_broken
 add_message '[id]="\"quote\"@example"' '[subject]="Message with quote in Message-Id"'
 test_emacs '(notmuch-search "subject:\"Message with quote\"")
 	    (notmuch-test-wait)
-- 
1.7.7.2

Re: [PATCH 0/2] Escape message ID queries in Emacs

[Tomi Ollila]

On Tue, Mar 27 2012, Austin Clements wrote:

> Currently, Emacs does not escape message ID queries and is
> inconsistent about quoting them.  This patch centralizes this in one
> function that always produces a properly quoted and escaped message ID
> query.
>
> With this, Emacs no longer gets confused by Tomi's crazy message,
>   id:"id:""1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi"""

LGTM. Things work!


One observation, though:

In search bar the following queries return one match:

id:id:"1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi"
id:"id:""1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi"""

but

id:"id:"1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi""

return 0 matches.

It looks like the (above) search strings goes verbatim to command line

i.e. 'notmuch' 'search' '--sort=oldest-first' 'id:...'

can be used to compare... so that would be CLI issue if there
is ever need to do do anything with it.

+1

Thanks for fixing this.

Tomi

Re: [PATCH 0/2] Escape message ID queries in Emacs

[Austin Clements]

On Tue, 27 Mar 2012, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Tue, Mar 27 2012, Austin Clements wrote:
>
>> Currently, Emacs does not escape message ID queries and is
>> inconsistent about quoting them.  This patch centralizes this in one
>> function that always produces a properly quoted and escaped message ID
>> query.
>>
>> With this, Emacs no longer gets confused by Tomi's crazy message,
>>   id:"id:""1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi"""
>
> LGTM. Things work!
>
>
> One observation, though:
>
> In search bar the following queries return one match:
>
> id:id:"1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi"
> id:"id:""1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi"""
>
> but
>
> id:"id:"1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi""
>
> return 0 matches.
>
> It looks like the (above) search strings goes verbatim to command line
>
> i.e. 'notmuch' 'search' '--sort=oldest-first' 'id:...'
>
> can be used to compare... so that would be CLI issue if there
> is ever need to do do anything with it.

I'm not sure I would consider this a CLI "issue".  It may not be
intuitive, but it is an intentional part of the Xapian query parser's
grammar.  If a boolean prefix is followed immediately by a quote, then
the term is between that quote and the next quote, modulo an escaping
rule that lets you include a literal quote in the term by doubling it in
the query string.  Together, these rules let you reliably put anything
in a boolean search term.

> +1
>
> Thanks for fixing this.
>
> Tomi

Re: [PATCH 0/2] Escape message ID queries in Emacs

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Mon, Mar 26 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> Currently, Emacs does not escape message ID queries and is
> inconsistent about quoting them.  This patch centralizes this in one
> function that always produces a properly quoted and escaped message ID
> query.
>
> With this, Emacs no longer gets confused by Tomi's crazy message,
>   id:"id:""1332281811-24710-2b-git-send-email-tomi.ollila@iki.fi"""

LGTM.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 0/2] Escape message ID queries in Emacs

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> Currently, Emacs does not escape message ID queries and is
> inconsistent about quoting them.  This patch centralizes this in one
> function that always produces a properly quoted and escaped message ID
> query.

Pushed, 

d