GNU bug report logs - #39884
27.0.50; Emacs may destroy outgoing email messages during sending

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39884 in the body.
You can then email your comments to 39884 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Rainer Gemulla" <rgemulla <at> gmx.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; Emacs may destroy outgoing email messages during sending
Date: Tue, 03 Mar 2020 15:36:37 +0000

[Message part 1 (text/plain, inline)]

I am composing emails using Emacs message mode. Shortly after switching 
to Emacs 27, some people complained to me that they could not read some 
of emails I sent them. I checked in my local email archive: even my 
archived emails were not readable with Emacs any more. This is a major 
bug, and for this reason I switched back to Emacs 26. It is my current 
understanding that the bug occurs when (1) forwarding messages and (2) 
signing the forwarded messages using GPG. So not everyone seems to be 
affected. Nevertheless, I experienced this bug on multiple machines (in 
fact, on all machines that I tried).

The rest of this email explains where the bug arises and how to 
reproduce it.

Before sending an email, Emacs converts the email text to MIME. This 
conversion process is performed by a function called mml-to-mime. I 
found that this function (or it's interaction with signing) is 
responsible for Emacs 27 destroying emails: the produced MIME can be 
invalid. This happens mostly when there are multiple mml parts in the 
original email, which is the case when forwarding an email.

After some playing around, I found a way to reproduce the bug. It's 
somewhat painful, but the error consistently arises. I first list the 
steps, then the original email, then its correct MIME encoding, and the 
incorrect MIME encoding that Emacs may produce. Note that step 3+4 
encode the message for the first time (all fine), steps 5-7 add signing, 
steps 8-9 encode the orginal message again (without signing, now 
broken).

1. Run: emacs -Q

2. M-x message-mode
3. Clear scratch buffer, paste original message
4. M-: (mml-to-mime) --> gives CORRECT result

5. Clear scratch buffer, paste original message
6. Insert a new line "<#secure method=pgpmime mode=sign>" at start of 
message (after line "--text follows this line--"). This makes Emacs try 
to sign the mail.
7. M-: (mml-to-mime) --> throws (expected) signer name error

8. Clear scratch buffer, paste original message
9. M-: (mml-to-mime)
  --> broken result (first Content-Type after "text follows..." is 
wrong)

I tried to track down the bug further and found this piece of code in 
mml-parse-1, line 284:

(setq tag (list 'part '(type . "text/plain"))
      no-markup-p t
      warn t)

During step 9, this statement is executed, but afterwards, the tag 
variable is not set to the list mentioned in the statement (seriously!). 
In my case, it had value 'Content-Type: multipart/alternative; 
boundary="====-=-="' right afterwards (as in the incorrect result 
below). At this point I gave up, looks like a deeper problem.


ORIGINAL MESSAGE
From: a <at> b.ce
To: c <at> d.de
Subject: Test
--text follows this line--
Test

<#mml type=message/rfc822 disposition=inline>
<#multipart type=alternative>
<#part type=text/plain charset="UTF-8" disposition=inline nofile=yes>
Some text.
<#part type=text/html charset="UTF-8" nofile=yes>
Some more text.
<#/multipart>
<#/mml>


CORRECT RESULT
From: a <at> b.ce
To: c <at> d.de
Subject: Test
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
--text follows this line--
--=-=-=
Content-Type: text/plain

Test


--=-=-=
Content-Type: message/rfc822
Content-Disposition: inline

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

Some text.

--====-=-=
Content-Type: text/html; charset=utf-8

Some more text.

--====-=-=--

--=-=-=--


INCORRECT RESULT AFTER STEP 9
From: a <at> b.ce
To: c <at> d.de
Subject: Test
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
--text follows this line--
--=-=-=
Content-Type: multipart/alternative; boundary="====-=-="
Content-Transfer-Encoding: base64

VGV4dAoK
--=-=-=
Content-Type: message/rfc822
Content-Disposition: inline

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

Some text.

--====-=-=
Content-Type: text/html; charset=utf-8

Some more text.

--====-=-=--

--=-=-=--

[Message part 2 (text/html, inline)]

[Message part 3 (application/pgp-signature, inline)]

Message #8 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Rainer Gemulla <rgemulla <at> gmx.de>
To: 39884 <at> debbugs.gnu.org
Subject: Simple fix
Date: Wed, 15 Apr 2020 09:51:46 +0200

[Message part 1 (text/plain, inline)]

Replacing in function mml-parse-1 (mml.el, line 284):

(setq tag (list 'part '(type . "text/plain"))
       no-markup-p t
       warn t)

with

(setq tag `(part (type . ,"text/plain"))
      no-markup-p t
      warn t)

seems to fix this bug. It's not clear to me why though.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Rainer Gemulla <rgemulla <at> gmx.de>
Cc: 39884 <at> debbugs.gnu.org
Subject: Re: bug#39884: 27.0.50; Emacs may destroy outgoing email messages
 during sending
Date: Wed, 15 Apr 2020 08:24:52 -0400

[Message part 1 (text/plain, inline)]

On Wed, 15 Apr 2020 at 03:55, Rainer Gemulla <rgemulla <at> gmx.de> wrote:
>
> Replacing in function mml-parse-1 (mml.el, line 284):
>
> (setq tag (list 'part '(type . "text/plain"))
>        no-markup-p t
>        warn t)
>
> with
>
> (setq tag `(part (type . ,"text/plain"))
>       no-markup-p t
>       warn t)
>
> seems to fix this bug. It's not clear to me why though.

mml-parse-1 calls nconc on tag, so it's likely some destructive
modification of code literals is happening. However, I'm not able to
reproduce the bug following your instructions on the latest emacs-27
checkout. I'm attaching a lisp file which automates the process (I
also tried doing it manually, in case that makes a difference).

[bug-39884-bad-mml-parsing.el (text/x-emacs-lisp, attachment)]

Message #14 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Rainer Gemulla <rgemulla <at> gmx.de>
To: 39884 <at> debbugs.gnu.org
Subject: Re: bug#39884: 27.0.50; Emacs may destroy outgoing email messages
 during sending
Date: Wed, 15 Apr 2020 14:57:24 +0200

[Message part 1 (text/plain, inline)]

Perhaps just a typo: what's called "39884-correct-results-of-step-9" in your code is actually the incorrect result. I.e., step 9 should also produce bug-39884-correct-result.

I ran your lisp file via "emacs -Q -l bug-39884-bad-mml-parsing.el" using the emacs-27 branch as of this morning. At least on my machines, I am getting the incorrect result. If you don't see the bug, it may be hard to track down. I've seen it on all my machines, but I am only running Ubuntu (different versions though). So it may be related to the environment being used.

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Rainer Gemulla <rgemulla <at> gmx.de>
Cc: 39884 <at> debbugs.gnu.org
Subject: Re: bug#39884: 27.0.50; Emacs may destroy outgoing email messages
 during sending
Date: Wed, 15 Apr 2020 09:11:41 -0400

tags 39884 + confirmed
quit

Rainer Gemulla <rgemulla <at> gmx.de> writes:

> Perhaps just a typo: what's called "39884-correct-results-of-step-9"
> in your code is actually the incorrect result. I.e., step 9 should
> also produce bug-39884-correct-result.

Aha, I misread.

> I ran your lisp file via "emacs -Q -l bug-39884-bad-mml-parsing.el"
> using the emacs-27 branch as of this morning. At least on my machines,
> I am getting the incorrect result.

Yes, me too.  For some reason, I was expecting to get an error on step
9.  Upon re-reading your original report, I can't see how I thought
that.

I'll look into the code later today or tonight.

Message #22 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: "Rainer Gemulla" <rgemulla <at> gmx.de>
Cc: 39884 <at> debbugs.gnu.org
Subject: Re: bug#39884: 27.0.50; Emacs may destroy outgoing email messages
 during sending
Date: Thu, 16 Apr 2020 20:30:14 -0400

[Message part 1 (text/plain, inline)]

tags 39884 + patch
quit

"Rainer Gemulla" <rgemulla <at> gmx.de> writes:

> During step 9, this statement is executed, but afterwards, the tag
> variable is not set to the list mentioned in the statement
> (seriously!). In my case, it had value 'Content-Type:
> multipart/alternative; boundary="====-=-="' right afterwards

Yes, it's due destructive modification of a quoted literal value.  To
catch it, I modified mml.el like this:

--- i/lisp/gnus/mml.el
+++ w/lisp/gnus/mml.el
@@ -216,6 +216,8 @@ mml-parse
     (with-syntax-table mml-syntax-table
       (mml-parse-1))))
 
+(defconst mml-text/plain-tag-literal-constant '(type . "text/plain"))
+
 (defun mml-parse-1 ()
   "Parse the current buffer as an MML document."
   (let (struct tag point contents charsets warn use-ascii no-markup-p raw)
@@ -281,7 +283,7 @@ mml-parse-1
 	    (setq tag (mml-read-tag)
 		  no-markup-p nil
 		  warn nil)
-	  (setq tag (list 'part '(type . "text/plain"))
+	  (setq tag (list 'part mml-text/plain-tag-literal-constant)
 		no-markup-p t
 		warn t))
 	(setq raw (cdr (assq 'raw tag))

And added a debug check around setcdr as in the attached
bug-39884-bad-mml-parsing.el file.

[bug-39884-bad-mml-parsing.el (text/plain, attachment)]

[Message part 3 (text/plain, inline)]

This gives the backtrace:

[setcdr-backtrace.txt (text/plain, attachment)]

[Message part 5 (text/plain, inline)]

The fix is simply to make a fresh cons instead of using a quoted literal
(your backquote fix macroexpands to the same thing, but it's not
sufficiently obvious enough for a human reader to realize that), this
should be safe enough to put on the emacs-27 branch:

[0001-Don-t-let-a-code-literal-get-modified-in-mml-parsing.patch (text/x-diff, inline)]

From 16c77ed31d836c374b37cbdf03947567fc7b8581 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Thu, 16 Apr 2020 20:24:26 -0400
Subject: [PATCH] Don't let a code literal get modified in mml parsing
 (Bug#39884)

* lisp/gnus/mml.el (mml-parse-1): Make a fresh cons for the tag type,
because 'mml-generate-mime' destructively modifies it.
---
 lisp/gnus/mml.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/gnus/mml.el b/lisp/gnus/mml.el
index cdd8f3d3a5..556cf0804a 100644
--- a/lisp/gnus/mml.el
+++ b/lisp/gnus/mml.el
@@ -281,7 +281,7 @@ mml-parse-1
 	    (setq tag (mml-read-tag)
 		  no-markup-p nil
 		  warn nil)
-	  (setq tag (list 'part '(type . "text/plain"))
+	  (setq tag (list 'part (cons 'type "text/plain"))
 		no-markup-p t
 		warn t))
 	(setq raw (cdr (assq 'raw tag))
-- 
2.11.0

Message #27 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Rainer Gemulla <rgemulla <at> gmx.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 39884 <at> debbugs.gnu.org
Subject: Re: bug#39884: 27.0.50; Emacs may destroy outgoing email messages
 during sending
Date: Fri, 17 Apr 2020 09:48:54 +0200

[Message part 1 (text/plain, inline)]

> The fix is simply to make a fresh cons instead of using a quoted literal
> (your backquote fix macroexpands to the same thing, but it's not
> sufficiently obvious enough for a human reader to realize that), this
> should be safe enough to put on the emacs-27 branch:

Thanks! Your patch also works for me.

[signature.asc (application/pgp-signature, inline)]

Message #30 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Rainer Gemulla <rgemulla <at> gmx.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 39884 <at> debbugs.gnu.org
Subject: Re: bug#39884: 27.0.50; Emacs may destroy outgoing email messages
 during sending
Date: Fri, 17 Apr 2020 09:49:05 +0200

[Message part 1 (text/plain, inline)]

> The fix is simply to make a fresh cons instead of using a quoted literal
> (your backquote fix macroexpands to the same thing, but it's not
> sufficiently obvious enough for a human reader to realize that), this
> should be safe enough to put on the emacs-27 branch:

Thanks! Your patch also works for me.

[signature.asc (application/pgp-signature, inline)]

Message #33 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 39884 <at> debbugs.gnu.org, rgemulla <at> gmx.de
Subject: Re: bug#39884: 27.0.50;
 Emacs may destroy outgoing email messages during sending
Date: Fri, 17 Apr 2020 12:54:05 +0300

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Thu, 16 Apr 2020 20:30:14 -0400
> Cc: 39884 <at> debbugs.gnu.org
> 
> The fix is simply to make a fresh cons instead of using a quoted literal
> (your backquote fix macroexpands to the same thing, but it's not
> sufficiently obvious enough for a human reader to realize that), this
> should be safe enough to put on the emacs-27 branch:

Fine with me, thanks.

Message #36 received at 39884 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 39884 <at> debbugs.gnu.org, rgemulla <at> gmx.de
Subject: Re: bug#39884: 27.0.50; Emacs may destroy outgoing email messages
 during sending
Date: Sat, 25 Apr 2020 09:59:18 -0400

tags 39884 fixed
close 39884 27.1
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Date: Thu, 16 Apr 2020 20:30:14 -0400
>> Cc: 39884 <at> debbugs.gnu.org
>> 
>> The fix is simply to make a fresh cons instead of using a quoted literal
>> (your backquote fix macroexpands to the same thing, but it's not
>> sufficiently obvious enough for a human reader to realize that), this
>> should be safe enough to put on the emacs-27 branch:
>
> Fine with me, thanks.

Pushed to emacs-27.

[1: 821760fdc4]: 2020-04-25 09:55:36 -0400
  Don't let a code literal get modified in mml parsing (Bug#39884)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=821760fdc439214f57212708e23d5c87088d34ee

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.