* [PATCH 1/2] test: Add broken test for Emacs boolean term escaping
@ 2014-03-11 22:19 Austin Clements
2014-03-11 22:19 ` [PATCH 2/2] emacs: Use whitelist instead of blacklist for " Austin Clements
2014-03-26 0:34 ` [PATCH 1/2] test: Add broken test for Emacs boolean " David Bremner
0 siblings, 2 replies; 4+ messages in thread
From: Austin Clements @ 2014-03-11 22:19 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] emacs: Use whitelist instead of blacklist for term escaping
2014-03-11 22:19 [PATCH 1/2] test: Add broken test for Emacs boolean term escaping Austin Clements
@ 2014-03-11 22:19 ` Austin Clements
2014-03-24 20:38 ` Tomi Ollila
2014-03-26 0:34 ` [PATCH 1/2] test: Add broken test for Emacs boolean " David Bremner
1 sibling, 1 reply; 4+ messages in thread
From: Austin Clements @ 2014-03-11 22:19 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] emacs: Use whitelist instead of blacklist for term escaping
2014-03-11 22:19 ` [PATCH 2/2] emacs: Use whitelist instead of blacklist for " Austin Clements
@ 2014-03-24 20:38 ` Tomi Ollila
0 siblings, 0 replies; 4+ messages in thread
From: Tomi Ollila @ 2014-03-24 20:38 UTC (permalink / raw)
To: Austin Clements, notmuch
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] test: Add broken test for Emacs boolean term escaping
2014-03-11 22:19 [PATCH 1/2] test: Add broken test for Emacs boolean term escaping Austin Clements
2014-03-11 22:19 ` [PATCH 2/2] emacs: Use whitelist instead of blacklist for " Austin Clements
@ 2014-03-26 0:34 ` David Bremner
1 sibling, 0 replies; 4+ messages in thread
From: David Bremner @ 2014-03-26 0:34 UTC (permalink / raw)
To: Austin Clements, notmuch
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-26 0:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-11 22:19 [PATCH 1/2] test: Add broken test for Emacs boolean term escaping Austin Clements
2014-03-11 22:19 ` [PATCH 2/2] emacs: Use whitelist instead of blacklist for " Austin Clements
2014-03-24 20:38 ` Tomi Ollila
2014-03-26 0:34 ` [PATCH 1/2] test: Add broken test for Emacs boolean " David Bremner
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).