bug#26918: 25.2; rmail edit corrupts mail if content-type header not displayed

bug#26918: 25.2; rmail edit corrupts mail if content-type header not displayed

[Ken Olum]

[-- Attachment #1: Type: text/plain, Size: 3898 bytes --]

If you have a message in rmail which is in MIME format with base64
encoding and consists only of a single text/plain part, and if you do
not display the "Content-Type" header (e.g. by having it in
rmail-ignored-headers), the message will get corrupted.  The problem is
this: under the circumstances above, rmail-edit-current-message allows
you to edit your view of the message (which is good, since you don't
want to edit the base64).  But when it goes to reencode the message, it
looks in the headers it gave you to edit and doesn't see the
Content-Type.  Later it does see the Content-Type in the original
headers, and the result is massive confusion.  In some circumstances it
corrupts only that message, but in others it corrupts your mail file by
merging this message with the one before.

To reproduce:

1. emacs -Q

2. Visit attached rmail-test file

3. M-x rmail-mode

4. Set variable rmail-ignored-headers to ignore "Content-Type", e.g., by
editing it in customization system.

5. Push "t" twice so that previous change takes effect.  Verify that
Content-Type is not displayed.

6. Push "e" to edit message.  Insert a character at the end.  C-c C-c to
finish.

7. Observe corrupted message on screen

I'm not sure how to reproduce the situation where it corrupts your mail
file, but it has happened to me several times.

I can provide a fix for this bug if we agree on the right strategy.

                                        Ken

In GNU Emacs 25.2.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars)
 of 2017-05-13 built on neptune
Windowing system distributor 'The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04.5 LTS

Configured features:
XPM JPEG TIFF GIF PNG SOUND NOTIFY ZLIB TOOLKIT_SCROLL_BARS LUCID X11

Important settings:
  value of $LC_ALL: C
  value of $LANG: en_US.UTF-8
  locale-coding-system: nil

Major mode: Lisp Interaction

Minor modes in effect:
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t

Recent messages:
Loading /home/kdo/emacs-init.el (source)...done
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rmail rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu
mail-prsvr mail-utils shell pcomplete comint ansi-color ring cl-macs cl
gv cl-loaddefs pcase cl-lib warnings time-date mule-util tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
inotify dynamic-setting x-toolkit x multi-tty make-network-process
emacs)

Memory information:
((conses 16 95255 6032)
 (symbols 48 20728 0)
 (miscs 40 52 117)
 (strings 32 18042 4867)
 (string-bytes 1 512076)
 (vectors 16 13426)
 (vector-slots 8 443268 2725)
 (floats 8 169 6)
 (intervals 56 189 92)
 (buffers 976 18)
 (heap 1024 39178 767))


[-- Attachment #2: rmail-test --]
[-- Type: application/octet-stream, Size: 701 bytes --]

From kdo@cosmos.phy.tufts.edu Sat May 13 20:52:34 2017
Return-path: <kdo@cosmos.phy.tufts.edu>
Envelope-to: kdo@cosmos.phy.tufts.edu
Delivery-date: Sat, 13 May 2017 20:52:34 -0400
Received: from kdo by cosmos.phy.tufts.edu ([local]:local)
	with local id 1d9hly-0002CB-M6 - Using Exim-4.84 (MandrivaLinux) MTA 
	(return-path <kdo@cosmos.phy.tufts.edu>); Sat, 13 May 2017 20:52:34 -0400
From: Ken Olum <kdo@cosmos.phy.tufts.edu>
To: kdo@cosmos.phy.tufts.edu
Subject: 
Date: Sat, 13 May 2017 20:52:34 -0400
Message-ID: <q527f1k1cq5.fsf@cosmos.phy.tufts.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: base64
X-RMAIL-ATTRIBUTES: --E-----

VGhpcyBpcyBhIHRlc3QKCg==

bug#26918: 25.2; rmail edit corrupts mail if content-type header not displayed

[Glenn Morris]

Ken Olum wrote:

> If you have a message in rmail which is in MIME format with base64
> encoding and consists only of a single text/plain part, and if you do
> not display the "Content-Type" header (e.g. by having it in
> rmail-ignored-headers), the message will get corrupted.  The problem is
> this: under the circumstances above, rmail-edit-current-message allows
> you to edit your view of the message (which is good, since you don't
> want to edit the base64).  But when it goes to reencode the message, it
> looks in the headers it gave you to edit and doesn't see the
> Content-Type.  Later it does see the Content-Type in the original
> headers, and the result is massive confusion.  In some circumstances it
> corrupts only that message, but in others it corrupts your mail file by
> merging this message with the one before.
>
> To reproduce:
>
> 1. emacs -Q
>
> 2. Visit attached rmail-test file
>
> 3. M-x rmail-mode
>
> 4. Set variable rmail-ignored-headers to ignore "Content-Type", e.g., by
> editing it in customization system.
>
> 5. Push "t" twice so that previous change takes effect.  Verify that
> Content-Type is not displayed.
>
> 6. Push "e" to edit message.  Insert a character at the end.  C-c C-c to
> finish.
>
> 7. Observe corrupted message on screen
>
> I'm not sure how to reproduce the situation where it corrupts your mail
> file, but it has happened to me several times.
>
> I can provide a fix for this bug if we agree on the right strategy.

It seems neither rmail user has an opinion. ;)
What do you suggest?
Two ideas that come to mind are:
1) force the relevant header(s) to be visible when editing a message
2) if the headers are not found in the message as edited, consult the
full unswapped message. (I wonder what happens if an edit adds a new,
duplicate Content-Type header that disagrees with the pre-existing one...)

bug#26918: 25.2; rmail edit corrupts mail if content-type header not displayed

[Ken Olum]

   From: Glenn Morris <rgm@gnu.org>
   Date: Mon, 05 Jun 2017 17:16:56 -0400

   It seems neither rmail user has an opinion. ;)

That bad, is it?

   What do you suggest?

I'd be in favor of looking first at the edited message, and if there is
no Content-Type header there, looking at the full headers of the
original message.  If Content-Type is ignored, but the user adds one in
the edit buffer, I would replace the original one in the message with the
new one supplied by the user.  I think this is the general way that
added headers are handled.

                                        Ken

bug#26918: 25.2; rmail edit corrupts mail if content-type header not displayed

[Glenn Morris]

Ken Olum wrote:

> I'd be in favor of looking first at the edited message, and if there is
> no Content-Type header there, looking at the full headers of the
> original message.  If Content-Type is ignored, but the user adds one in
> the edit buffer, I would replace the original one in the message with the
> new one supplied by the user.  I think this is the general way that
> added headers are handled.

That sounds like the correct approach to me.

bug#26918: rmail-cease-edit patches for bugs 26918 and 27353

[Ken Olum]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

Here is a patch to fix rmail editing problems associated with the
content-type header and reapplying the transfer-encoding to edited
messages.

                                        Ken

* lisp/mail/rmailedit.el (rmail-cease-edit): 
If no content-type in edited headers, look for one in original
headers and add it to edited headers (Bug #26918).
Marker to track start of new body, so that content-transfer-encoding
gets applied only to body (Bug #27353).
Ensure blank line at end of message after encoding, not before.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rmailedit.patch --]
[-- Type: text/x-diff, Size: 5074 bytes --]

--- old/lisp/mail/rmailedit.el	2017-06-13 16:03:23.148043977 -0400
+++ new/lisp/mail/rmailedit.el	2017-06-19 14:28:50.227408375 -0400
@@ -188,10 +188,6 @@
       (beginning-of-line)
       (insert ">")
       (forward-line)))
-  ;; Make sure buffer ends with a blank line so as not to run this
-  ;; message together with the following one.
-  (goto-char (point-max))
-  (rmail-ensure-blank-line)
   (let ((old rmail-old-text)
 	(pruned rmail-old-pruned)
 	(mime-state rmail-old-mime-state)
@@ -224,10 +220,9 @@
       (setq old nil)
       (goto-char (point-min))
       (search-forward "\n\n")
-      (setq headers-end (point-marker))
-      (goto-char (point-min))
+      (setq headers-end (point-marker)) ; first character of body
       (save-restriction
-	(narrow-to-region (point) headers-end)
+	(narrow-to-region (point-min) headers-end)
 	;; If they changed the message's encoding, rewrite the charset=
 	;; header for them, so that subsequent rmail-show-message
 	;; decodes it correctly.
@@ -240,6 +235,38 @@
 				      'us-ascii
 				    new-coding))))
 	       old-coding mime-beg mime-end content-type)
+          ;; If there's no content-type in the edited headers, look for one
+          ;; in the original headers and add it to the edited headers
+          ;; (Bug #26918)
+          (unless (mail-fetch-field "Content-Type")
+            (let (old-content-type
+                  (msgbeg (rmail-msgbeg rmail-current-message))
+                  (msgend (rmail-msgend rmail-current-message)))
+              (with-current-buffer rmail-view-buffer ; really the mbox buffer
+                (save-restriction
+                  (narrow-to-region msgbeg msgend)
+                  (goto-char (point-min))
+                  (setq limit (search-forward "\n\n"))
+                  (narrow-to-region (point-min) limit)
+                  (goto-char (point-min))
+                  (when (re-search-forward "^content-type:" limit t)
+                    (forward-line)
+                    (setq old-content-type (buffer-substring
+                                            (match-beginning 0) (point))))))
+              (when old-content-type
+                (save-excursion
+                  (goto-char headers-end) ; first char of body
+                  (backward-char)         ; add header before second newline
+                  (insert old-content-type)
+                  ;;Add it to rmail-old-headers as though it had been
+                  ;;there originally, to avoid rmail-edit-update-headers
+                  ;;an extra copy
+                  (let ((header (substring old-content-type 0
+                                           (length "content-type"))))
+                    (unless (assoc header rmail-old-headers)
+                      (push (cons header old-content-type) rmail-old-headers)))
+                  ))))
+          (goto-char (point-min))
 	  (if (re-search-forward rmail-mime-charset-pattern nil 'move)
 	      (setq mime-beg (match-beginning 1)
 		    mime-end (match-end 1)
@@ -281,29 +308,32 @@
 	  (setq character-coding (downcase character-coding)))
 
       (goto-char limit)
-      (let ((inhibit-read-only t))
-	(let ((data-buffer (current-buffer))
-	      (end (copy-marker (point) t)))
-	  (with-current-buffer rmail-view-buffer
-	    (encode-coding-region headers-end (point-max) coding-system
-				  data-buffer))
-	  (delete-region end (point-max)))
-
+      (let ((inhibit-read-only t)
+            (data-buffer (current-buffer))
+            (start (copy-marker (point) nil)) ; new body will be between
+            (end (copy-marker (point) t)))    ; these two markers
+        (with-current-buffer rmail-view-buffer
+          (encode-coding-region headers-end (point-max) coding-system
+                                data-buffer))
+        (delete-region end (point-max))
 	;; Apply to the mbox buffer any changes in header fields
 	;; that the user made while editing in the view buffer.
 	(rmail-edit-update-headers (rmail-edit-diff-headers
 				    rmail-old-headers new-headers))
-
 	;; Re-apply content-transfer-encoding, if any, on the message body.
 	(cond
 	 ((string= character-coding "quoted-printable")
-	  (mail-quote-printable-region (point) (point-max)))
+	  (mail-quote-printable-region start (point-max)))
 	 ((and (string= character-coding "base64") is-text-message)
-	  (base64-encode-region (point) (point-max)))
+	  (base64-encode-region start (point-max)))
 	 ((and (eq character-coding 'uuencode) is-text-message)
-	  (error "uuencoded messages are not supported"))))
+	  (error "uuencoded messages are not supported")))
+        ;; After encoding, make sure buffer ends with a blank line so as not to
+        ;; run this message together with the following one.
+        (goto-char (point-max))
+        (rmail-ensure-blank-line))
       (rmail-set-attribute rmail-edited-attr-index t))
-    ;;??? BROKEN perhaps.
+;;;??? BROKEN perhaps.
 ;;;    (if (boundp 'rmail-summary-vector)
 ;;;	(aset rmail-summary-vector (1- rmail-current-message) nil))
     (rmail-show-message)

bug#27353: rmail-cease-edit patches for bugs 26918 and 27353

[Eli Zaretskii]

> From: Ken Olum <kdo@cosmos.phy.tufts.edu>
> Date: Mon, 19 Jun 2017 14:40:24 -0400
> 
> Here is a patch to fix rmail editing problems associated with the
> content-type header and reapplying the transfer-encoding to edited
> messages.

Thanks, pushed to master.