Re: emacs: reply subject sanitization

2017-09-26 Thread David Edmondson
On Tuesday, 2017-09-26 at 21:26:06 +0300, Jani Nikula wrote:

> v2 of id:20170915155716.19597-1-j...@nikula.org, now with test.

Looks good.

dme.
-- 
I can't explain, you would not understand. This is not how I am.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 5/6] cli/new: support // in new.ignore

2017-09-26 Thread Jani Nikula
On Mon, 25 Sep 2017, David Bremner  wrote:
> Jani Nikula  writes:
>
>> +  A regular expression delimited with // that will be matched
>> +  against the path of the file or directory relative to the
>> +  database path. The beginning and end of string must be
>> +  explictly anchored. For example, /.*/foo$/ would match
>> +  "bar/foo" and "bar/baz/foo", but not "foo" or "bar/foobar".
>
> Is it worth remarking that '/' does not need to be escaped? or more
> interestingly, what happens if it is escaped, do things break?

It just gets passed down to regcomp() with the escape and all. I'm not
sure it's worth trying to exhaustively explain everything.

>>  
>> +static notmuch_bool_t
>> +_setup_ignore (notmuch_config_t *config, add_files_state_t *state)
>> +{
>
> Would be nice to document what this return value means.

Something like:

/* Jani forgot to do anything useful with the return value */

I think it was originally meant to return false on illegal input
(e.g. opening '/' without closing '/') or regcomp() failing, but then I
opted for the more lax approach. xregcomp() warns about failures though.

>
>> +const char **ignore_list, **ignore;
>> +int nregex = 0, nverbatim = 0;
>> +const char **verbatim = NULL;
>> +regex_t *regex = NULL;
>> +
>> +ignore_list = notmuch_config_get_new_ignore (config, NULL);
>> +if (! ignore_list)
>> +return TRUE;
>> +
>> +for (ignore = ignore_list; *ignore; ignore++) {
>> +const char *s = *ignore;
>> +size_t len = strlen (s);
>> +
>> +if (len > 2 && s[0] == '/' && s[len - 1] == '/') {
>
> One thing we eventually settled on in the query parser is that an
> opening '/' without a trailing '/' is an errror. But perhaps it's fine
> to take a more permissive approach here.

I'm fine either way, I just chose to be permissive.

So do I make the function void and drop the return values, or make it
check and return errors?

>
>> +
>> +if (! state->ignore_regex_length)
>> +return FALSE;
>
> It's a nitpick, even by the standards of this review, but I'd prefer an
> explicit '> 0' check.

ITYM (state->ignore_regex_length == 0) but ack.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/2] test: add emacs reply test for subjects with TAB

2017-09-26 Thread Jani Nikula
Expect TABs to be sanitized from the subject line. Known broken.
---
 test/T310-emacs.sh | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index fde11790a600..2ef566bac490 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -401,6 +401,29 @@ Notmuch Test Suite  writes:
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Reply within emacs to a message with TAB in subject"
+test_subtest_known_broken
+test_emacs '(let ((message-hidden-headers ''()))
+   (notmuch-search 
"id:1258471718-6781-1-git-send-email-dotted...@dottedmag.net")
+   (notmuch-test-wait)
+   (notmuch-search-show-thread)
+   (notmuch-test-wait)
+   (notmuch-show-reply-sender)
+   (test-output))'
+sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
+sed -i -e 's/^References: <.*>$/References: /' OUTPUT
+sed -i -e '/^--text follows this line--$/q' OUTPUT
+cat 
+To: Mikhail Gusarov 
+Subject: Re: [notmuch] [PATCH 1/2] Close message file after parsing message 
headers
+In-Reply-To: 
+Fcc: ${MAIL_DIR}/sent
+References: 
+--text follows this line--
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "Reply from alternate address within emacs"
 add_message '[from]="Sender "' \
 [to]=test_suite_ot...@notmuchmail.org
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/2] emacs: sanitize subject in replies

2017-09-26 Thread Jani Nikula
Commit a7964c86d125 ("emacs: Sanitize authors and subjects in search
and show") added sanitization of header information for display. Do
the same for reply subjects.

This fixes the long-standing annoying artefact of certain versions of
mailman using tab as folding whitespace, leading to tabs in reply
subjects.
---
 emacs/notmuch-mua.el | 2 +-
 test/T310-emacs.sh   | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index fd64b362b542..7a341ebf0588 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -218,7 +218,7 @@ mutiple parts get a header."
 else
 collect pair)))
  (notmuch-mua-mail (plist-get reply-headers :To)
-   (plist-get reply-headers :Subject)
+   (notmuch-sanitize (plist-get reply-headers 
:Subject))
(notmuch-headers-plist-to-alist reply-headers)
nil (notmuch-mua-get-switch-function
 
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 2ef566bac490..4456bc659158 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -402,7 +402,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Reply within emacs to a message with TAB in subject"
-test_subtest_known_broken
 test_emacs '(let ((message-hidden-headers ''()))
(notmuch-search 
"id:1258471718-6781-1-git-send-email-dotted...@dottedmag.net")
(notmuch-test-wait)
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


emacs: reply subject sanitization

2017-09-26 Thread Jani Nikula
v2 of id:20170915155716.19597-1-j...@nikula.org, now with test.

BR,
Jani.

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Problem forwarding messages

2017-09-26 Thread Rafael Avila de Espindola

Not sure if this is an emacs or notmuch problem.

I can read mime messages with the notmuch major mode, but when I try to
forward it the only thing that gets sent is:



Return-Path: 
Received: from localhost ([2607:c000:813e:da00:f636:b7a4:7922:316d])
by smtp.gmail.com with ESMTPSA id 70sm18497736pfh.63.2017.09.26.10.14.39
for 
(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
Tue, 26 Sep 2017 10:14:39 -0700 (PDT)
From: Rafael Avila de Espindola 
To: name 
Subject: the subject
Date: Tue, 26 Sep 2017 10:14:38 -0700
Message-ID: <87efqts7pd.fsf@dell.i-did-not-set--mail-host-address--so-tickle-me>
MIME-Version: 1.0
Content-Type: message/rfc822
Content-Disposition: inline

--==-=-=--
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary="==-=-="


Cheers,
Rafael
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch