fix Fcc of signed / encrypted messages.

fix Fcc of signed / encrypted messages.

[David Bremner]

It turns out that in the years since dkg reported this bug, larsi
fixed it in message-do-fcc. Since we in our wisdom copied and modified
that function, we missed out on the fix. This series copies the fix
from "upstream". It requires a couple of updates to the test suite.
Other points to notice: it introduces a call to
#'message-clone-locals, and moves the call to
notmuch-maildir-setup-message-for-saving.

[PATCH 1/4] test/emacs: match mml settings in emacs_{fcc,deliver}_message

[David Bremner]

This will allow the sending of signed messages via smtp in the test
suite.
---
 test/test-lib-emacs.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/test-lib-emacs.sh b/test/test-lib-emacs.sh
index dde32177..a298526d 100644
--- a/test/test-lib-emacs.sh
+++ b/test/test-lib-emacs.sh
@@ -54,8 +54,9 @@ emacs_deliver_message () {
 	   (message-goto-body)
 	   (insert \"${body}\")
 	   $*
-	   (notmuch-mua-send-and-exit))"
-
+	   (let ((mml-secure-smime-sign-with-sender t)
+		 (mml-secure-openpgp-sign-with-sender t))
+	     (notmuch-mua-send-and-exit)))"
     # In case message was sent properly, client waits for confirmation
     # before exiting and resuming control here; therefore making sure
     # that server exits by sending (KILL) signal to it is safe.
-- 
2.34.1

[PATCH 2/4] test: define test_expect_equal_message_body

[David Bremner]

This is a relatively simple sed invocation, but rather than write a
comment everywhere, give it a descriptive name.
---
 test/test-lib.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index f1275b85..661c0dd5 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -432,6 +432,20 @@ test_expect_equal_file () {
     test_diff_file_ "$1" "$2"
 }
 
+# Like test_expect_equal_file, but compare the part of the two files after the first blank line
+test_expect_equal_message_body () {
+    exec 1>&6 2>&7		# Restore stdout and stderr
+    if [ -z "$inside_subtest" ]; then
+	error "bug in the test script: test_expect_equal_file without test_begin_subtest"
+    fi
+    test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test_expect_equal_file"
+
+    expected=$(sed -n '/^$/,$p' $1)
+    output=$(sed -n '/^$/,$p' $2)
+    test_expect_equal "$expected" "$output"
+}
+
 # Like test_expect_equal, but takes two filenames. Fails if either is empty
 test_expect_equal_file_nonempty () {
     exec 1>&6 2>&7		# Restore stdout and stderr
-- 
2.34.1

Re: [PATCH 2/4] test: define test_expect_equal_message_body

[Tomi Ollila]

On Thu, Jan 20 2022, David Bremner wrote:

> This is a relatively simple sed invocation, but rather than write a
> comment everywhere, give it a descriptive name.
> ---
>  test/test-lib.sh | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index f1275b85..661c0dd5 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -432,6 +432,20 @@ test_expect_equal_file () {
>      test_diff_file_ "$1" "$2"
>  }
>  
> +# Like test_expect_equal_file, but compare the part of the two files after the first blank line
> +test_expect_equal_message_body () {
> +    exec 1>&6 2>&7		# Restore stdout and stderr
> +    if [ -z "$inside_subtest" ]; then
> +	error "bug in the test script: test_expect_equal_file without test_begin_subtest"
> +    fi
> +    test "$#" = 2 ||
> +	error "bug in the test script: not 2 parameters to test_expect_equal_file"
> +
> +    expected=$(sed -n '/^$/,$p' $1)
> +    output=$(sed -n '/^$/,$p' $2)

The above works, as we don't have whitespace ($IFS characters to be
precise) in the filenames. However, for better example, "$1" and "$2"
should be used (as is used in many of the functions in test-lib.sh
already. 
The above prints out also the leading newline (which is OK), but if
one wanted those w/o the leading newline, the sed lines would be:
sed '1,/^$/d'

(If I did not come out with this notion of extra newline I would
not have shown this -- as the original sed -n ... matches more to 
"showing body" than "deleting header" :D)

Tomi

> +    test_expect_equal "$expected" "$output"
> +}
> +
>  # Like test_expect_equal, but takes two filenames. Fails if either is empty
>  test_expect_equal_file_nonempty () {
>      exec 1>&6 2>&7		# Restore stdout and stderr
> -- 
> 2.34.1

Re: [PATCH 2/4] test: define test_expect_equal_message_body

[David Bremner]

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

> The above works, as we don't have whitespace ($IFS characters to be
> precise) in the filenames. However, for better example, "$1" and "$2"
> should be used (as is used in many of the functions in test-lib.sh
> already. 

Makes sense, done (in private git)

> The above prints out also the leading newline (which is OK), but if
> one wanted those w/o the leading newline, the sed lines would be:
> sed '1,/^$/d'

Yeah, this a tougher call, but I decided to go with your version since I
was editing the function anyway. It is (maybe) slightly easier to understand
without sed -n.

d

[PATCH 3/4] test/emacs: known broken test for matching fcc and sent message

[David Bremner]

Based on the method outlined by Daniel Kahn Gilmour in
id:87k1zm225v.fsf@fifthhorseman.net.  With a delay of 0.2 seconds the
test becomes flaky on my machine. With a 1 second delay it fails
consistently for more than 1600 iterations.
---
 test/T350-crypto.sh | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 8dbf8935..209b8a74 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -13,16 +13,30 @@ test_description='PGP/MIME signature verification and decryption'
 test_require_emacs
 add_gnupg_home
 
-test_begin_subtest "emacs delivery of signed message"
+test_begin_subtest "emacs delivery of signed message via fcc"
 test_expect_success \
 'emacs_fcc_message \
     "test signed message 001" \
     "This is a test signed message." \
     "(mml-secure-message-sign)"'
 
+test_begin_subtest "emacs delivery of signed message via fcc and smtp"
+test_subtest_known_broken
+emacs_deliver_message \
+    'signed message sent via SMTP' \
+    'This is a test that messages are sent via SMTP' \
+    "(add-hook 'message-send-mail-hook (lambda () (sleep-for 1)))
+     (mml-secure-message-sign)"
+msg_file=$(notmuch search --output=files subject:signed-message-sent-via-SMTP)
+test_expect_equal_message_body sent_message "$msg_file"
+
 test_begin_subtest "signed part content-type indexing"
-output=$(notmuch search mimetype:multipart/signed and mimetype:application/pgp-signature | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test signed message 001 (inbox signed)"
+notmuch search mimetype:multipart/signed and mimetype:application/pgp-signature | notmuch_search_sanitize > OUTPUT
+cat <<EOF >EXPECTED
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test signed message 001 (inbox signed)
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; signed message sent via SMTP (inbox signed)
+EOF
+test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "signature verification"
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
-- 
2.34.1

[PATCH 4/4] emacs: use cached encoded copy for fcc

[David Bremner]

This fixes the bug reported by dkg in [1]. The movement of the call to
n-m-setup-message-for-saving is so the cleanup of Fcc headers happens
in the encoded version (otherwise Fcc headers may be saved to disk).

[1]: id:87k1zm225v.fsf@fifthhorseman.net
---
 emacs/notmuch-maildir-fcc.el | 8 ++++++--
 test/T350-crypto.sh          | 1 -
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
index 7e177bf7..51020788 100644
--- a/emacs/notmuch-maildir-fcc.el
+++ b/emacs/notmuch-maildir-fcc.el
@@ -149,6 +149,7 @@ Otherwise set it according to `notmuch-fcc-dirs'."
 	 (buf (current-buffer))
 	 (mml-externalize-attachments message-fcc-externalize-attachments))
      (with-current-buffer (get-buffer-create " *message temp*")
+       (message-clone-locals buf) ;; for message-encoded-mail-cache
        (erase-buffer)
        (insert-buffer-substring buf)
        ,@body)))
@@ -158,7 +159,10 @@ Otherwise set it according to `notmuch-fcc-dirs'."
 
 This should be called on a temporary copy.
 This is taken from the function message-do-fcc."
-  (message-encode-message-body)
+  (if (not message-encoded-mail-cache)
+      (message-encode-message-body)
+    (erase-buffer)
+    (insert message-encoded-mail-cache))
   (save-restriction
     (message-narrow-to-headers)
     (mail-encode-encoded-word-buffer))
@@ -179,12 +183,12 @@ This is a rearranged version of message mode's message-do-fcc."
 	(setq file (message-fetch-field "fcc" t)))
       (when file
 	(with-temporary-notmuch-message-buffer
+	 (notmuch-maildir-setup-message-for-saving)
 	 (save-restriction
 	   (message-narrow-to-headers)
 	   (while (setq file (message-fetch-field "fcc" t))
 	     (push file files)
 	     (message-remove-header "fcc" nil t)))
-	 (notmuch-maildir-setup-message-for-saving)
 	 ;; Process FCC operations.
 	 (mapc #'notmuch-fcc-handler files)
 	 (kill-buffer (current-buffer)))))))
diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 209b8a74..3c6626b4 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -21,7 +21,6 @@ test_expect_success \
     "(mml-secure-message-sign)"'
 
 test_begin_subtest "emacs delivery of signed message via fcc and smtp"
-test_subtest_known_broken
 emacs_deliver_message \
     'signed message sent via SMTP' \
     'This is a test that messages are sent via SMTP' \
-- 
2.34.1

Re: fix Fcc of signed / encrypted messages.

[David Bremner]

David Bremner <david@tethera.net> writes:

> It turns out that in the years since dkg reported this bug, larsi
> fixed it in message-do-fcc. Since we in our wisdom copied and modified
> that function, we missed out on the fix. This series copies the fix
> from "upstream". It requires a couple of updates to the test suite.
> Other points to notice: it introduces a call to
> #'message-clone-locals, and moves the call to
> notmuch-maildir-setup-message-for-saving.

Updated series (with Tomi's test framework changes) applied to master.