[PATCH 1/2] test: Add broken test for Emacs boolean term escaping

[PATCH 1/2] test: Add broken test for Emacs boolean term escaping

[Austin Clements]

The current term escaper gets most of these right, but fails to escape
things containing Unicode "fancy quotes" or things containing
non-whitespace control characters.
---
 test/T310-emacs.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 00ae96a..6c18bbd 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -953,4 +953,16 @@ test_emacs '(notmuch-search "subject:\"search race test\" -subject:two")
 output=$(notmuch search --output=messages 'tag:search-global-race-tag')
 test_expect_equal "$output" "id:$gen_msg_id_1"
 
+test_begin_subtest "Term escaping"
+test_subtest_known_broken
+output=$(test_emacs "(mapcar 'notmuch-escape-boolean-term (list
+	\"\"
+	\"abc\`~\!@#\$%^&*-=_+123\"
+	\"(abc\"
+	\")abc\"
+	\"\\\"abc\"
+	\"\x01xyz\"
+	\"\\x201cxyz\\x201d\"))")
+test_expect_equal "$output" '("\"\"" "abc`~!@#$%^&*-=_+123" "\"(abc\"" "\")abc\"" "\"\"\"abc\"" "\"'$'\x01''xyz\"" "\"“xyz”\"")'
+
 test_done
-- 
1.8.4.rc3

[PATCH 2/2] emacs: Use whitelist instead of blacklist for term escaping

[Austin Clements]

Previously, the term escaper used a blacklist of characters that
needed escaping.  This blacklist turned out to be somewhat incomplete;
for example, it did not contain non-whitespace ASCII control
characters or Unicode "fancy quotes", both of which do require the
term to be escaped.

Switch to a whitelist of characters that are definitely safe to leave
unquoted.  This fixes the broken test introduced by the previous
patch.
---
 emacs/notmuch-lib.el | 5 ++++-
 test/T310-emacs.sh   | 1 -
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 2fefdad..b071b2f 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -426,7 +426,10 @@ user-friendly queries."
 
   (save-match-data
     (if (or (equal term "")
-	    (string-match "[ ()]\\|^\"" term))
+	    ;; To be pessimistic, only pass through terms composed
+	    ;; entirely of ASCII printing characters other than ", (,
+	    ;; and ).
+	    (string-match "[^!#-'*-~]" term))
 	;; Requires escaping
 	(concat "\"" (replace-regexp-in-string "\"" "\"\"" term t t) "\"")
       term)))
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 6c18bbd..ac966e5 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -954,7 +954,6 @@ output=$(notmuch search --output=messages 'tag:search-global-race-tag')
 test_expect_equal "$output" "id:$gen_msg_id_1"
 
 test_begin_subtest "Term escaping"
-test_subtest_known_broken
 output=$(test_emacs "(mapcar 'notmuch-escape-boolean-term (list
 	\"\"
 	\"abc\`~\!@#\$%^&*-=_+123\"
-- 
1.8.4.rc3

Re: [PATCH 2/2] emacs: Use whitelist instead of blacklist for term escaping

[Tomi Ollila]

On Wed, Mar 12 2014, Austin Clements <amdragon@MIT.EDU> wrote:

> Previously, the term escaper used a blacklist of characters that
> needed escaping.  This blacklist turned out to be somewhat incomplete;
> for example, it did not contain non-whitespace ASCII control
> characters or Unicode "fancy quotes", both of which do require the
> term to be escaped.
>
> Switch to a whitelist of characters that are definitely safe to leave
> unquoted.  This fixes the broken test introduced by the previous
> patch.
> ---

LGTM.

Tomi

>  emacs/notmuch-lib.el | 5 ++++-
>  test/T310-emacs.sh   | 1 -
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 2fefdad..b071b2f 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -426,7 +426,10 @@ user-friendly queries."
>  
>    (save-match-data
>      (if (or (equal term "")
> -	    (string-match "[ ()]\\|^\"" term))
> +	    ;; To be pessimistic, only pass through terms composed
> +	    ;; entirely of ASCII printing characters other than ", (,
> +	    ;; and ).
> +	    (string-match "[^!#-'*-~]" term))
>  	;; Requires escaping
>  	(concat "\"" (replace-regexp-in-string "\"" "\"\"" term t t) "\"")
>        term)))
> diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
> index 6c18bbd..ac966e5 100755
> --- a/test/T310-emacs.sh
> +++ b/test/T310-emacs.sh
> @@ -954,7 +954,6 @@ output=$(notmuch search --output=messages 'tag:search-global-race-tag')
>  test_expect_equal "$output" "id:$gen_msg_id_1"
>  
>  test_begin_subtest "Term escaping"
> -test_subtest_known_broken
>  output=$(test_emacs "(mapcar 'notmuch-escape-boolean-term (list
>  	\"\"
>  	\"abc\`~\!@#\$%^&*-=_+123\"
> -- 
> 1.8.4.rc3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/2] test: Add broken test for Emacs boolean term escaping

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> The current term escaper gets most of these right, but fails to escape
> things containing Unicode "fancy quotes" or things containing
> non-whitespace control characters.

series pushed,

d