GNU bug report logs - #7746
SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail

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 7746 in the body.
You can then email your comments to 7746 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: Mark Lillibridge <mark.lillibridge <at> hp.com>
To: bug-gnu-emacs <at> gnu.org
Subject: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
Date: Mon, 27 Dec 2010 16:17:20 -0800

mail-strip-quoted-names in mail/mail-utils.el (at least version 23.1
onwards) contains the following code:

mail-utils.el:187:
	   (with-current-buffer (get-buffer-create " *temp*")
	     (erase-buffer)
	     ...
	     (erase-buffer))

This code erases the buffer " *temp*" even if it is being used by
another piece of code!  This is particularly bad because this is the
first buffer used by with-temp-buffer.

A simple fix is to switch to using with-temp-buffer, which always
creates and destroys a new buffer (patch at end):

	   (with-temp-buffer
	      ...
	      )


    This bug breaks unrmail badly, causing it to discard all messages
after one containing a from line that causes that block of code to be
executed (roughly, from lines that contain nested comments).  This is
because unrmail loads the file to be converted into a buffer created
with with-temp-buffer, which is in turn erased prematurely by
mail-strip-quoted-names.  [There is a longer discussion about this on
the developers mailing list.]

    As this bug causes the permanent loss of e-mail, it should probably
be fixed posthaste, including in version 23.

- Mark

ts-rhel5 [158]% diff mail-utils.el new-mail-utils.el
187,188c187
<          (with-current-buffer (get-buffer-create " *temp*")
<            (erase-buffer)
---
>          (with-temp-buffer
201,202c200
<            (setq address (buffer-string))
<            (erase-buffer))
---
>            (setq address (buffer-string)))

Message #10 received at 7746-done <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 7746-done <at> debbugs.gnu.org
Subject: Re: bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing
	unrmail to lose mail
Date: Sat, 01 Jan 2011 21:43:59 -0500

Version: 23.3

Thanks; applied.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: mark.lillibridge <at> hp.com
Cc: 7746 <at> debbugs.gnu.org
Subject: Re: bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing
	unrmail to lose mail
Date: Sat, 01 Jan 2011 22:21:18 -0500

close 7746
thanks

> This code erases the buffer " *temp*" even if it is being used by
> another piece of code!  This is particularly bad because this is the
> first buffer used by with-temp-buffer.

Indeed, that's wrong, thanks for spotting it.  I've installed your
suggested patch (see below) into the emacs-23 branch (for Emacs-23.3).

BTW, I'm wondering why the code handles nesting in this way.  Can you
try the second patch below (you may need to hand-apply it since it's
based on the new code I just installed), to confirm that it works just
as well?


        Stefan


--- lisp/mail/mail-utils.el	2010-10-09 00:41:03 +0000
+++ lisp/mail/mail-utils.el	2011-01-02 03:03:38 +0000
@@ -189,8 +189,7 @@
        ;; Detect nested comments.
        (if (string-match "[ \t]*(\\([^)\\]\\|\\\\.\\|\\\\\n\\)*(" address)
 	   ;; Strip nested comments.
-	   (with-current-buffer (get-buffer-create " *temp*")
-	     (erase-buffer)
+	   (with-temp-buffer
 	     (insert address)
 	     (set-syntax-table lisp-mode-syntax-table)
 	     (goto-char 1)
@@ -203,8 +202,7 @@
 				    (forward-sexp 1)
 				  (error (goto-char (point-max))))
 				  (point))))
-	     (setq address (buffer-string))
-	     (erase-buffer))
+	     (setq address (buffer-string)))
 	 ;; Strip non-nested comments an easier way.
 	 (while (setq pos (string-match
 			    ;; This doesn't hack rfc822 nested comments


Second patch to simplify the code:

                            
=== modified file 'lisp/mail/mail-utils.el'
--- lisp/mail/mail-utils.el	2011-01-02 03:16:03 +0000
+++ lisp/mail/mail-utils.el	2011-01-02 03:19:53 +0000
@@ -186,30 +186,13 @@
 	       (mapconcat 'identity (rfc822-addresses address) ", "))
       (let (pos)
 
-       ;; Detect nested comments.
-       (if (string-match "[ \t]*(\\([^)\\]\\|\\\\.\\|\\\\\n\\)*(" address)
-	   ;; Strip nested comments.
-	   (with-temp-buffer
-	     (insert address)
-	     (set-syntax-table lisp-mode-syntax-table)
-	     (goto-char 1)
-	     (while (search-forward "(" nil t)
-	       (forward-char -1)
-	       (skip-chars-backward " \t")
-	       (delete-region (point)
-			      (save-excursion
-				(condition-case ()
-				    (forward-sexp 1)
-				  (error (goto-char (point-max))))
-				  (point))))
-	     (setq address (buffer-string)))
-	 ;; Strip non-nested comments an easier way.
+        ;; Strip comments.
 	 (while (setq pos (string-match
 			    ;; This doesn't hack rfc822 nested comments
 			    ;;  `(xyzzy (foo) whinge)' properly.  Big deal.
-			    "[ \t]*(\\([^)\\]\\|\\\\.\\|\\\\\n\\)*)"
+                          "[ \t]*(\\([^()\\]\\|\\\\.\\|\\\\\n\\)*)"
 			    address))
-	   (setq address (replace-match "" nil nil address 0))))
+          (setq address (replace-match "" nil nil address 0)))
 
        ;; strip surrounding whitespace
        (string-match "\\`[ \t\n]*" address)

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

From: Mark Lillibridge <mark.lillibridge <at> hp.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 7746 <at> debbugs.gnu.org
Subject: Re: bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing
	unrmail to lose mail
Date: Sun, 02 Jan 2011 16:18:29 -0800

>  close 7746
>  thanks
>  
>  > This code erases the buffer " *temp*" even if it is being used by
>  > another piece of code!  This is particularly bad because this is the
>  > first buffer used by with-temp-buffer.
>  
>  Indeed, that's wrong, thanks for spotting it.  I've installed your
>  suggested patch (see below) into the emacs-23 branch (for Emacs-23.3).
>  
>  BTW, I'm wondering why the code handles nesting in this way.  Can you
>  try the second patch below (you may need to hand-apply it since it's
>  based on the new code I just installed), to confirm that it works just
>  as well?

    I looked up rfc822 comments on the web and found at
http://www.w3.org/Protocols/rfc822/3_Lexical.html:

comment     =  "(" *(ctext / quoted-pair / comment) ")"

ctext       =  <any CHAR excluding "(",     ; => may be folded
                ")", "\" & CR, & including
                linear-white-space>

quoted-pair =  "\" CHAR                     ; may quote any char


After some thought, I figured out why you your code doesn't work:

  it turns "( \(  )" into "( \"


You might be able to fix this problem using subgroups, but it's going to
be fairly tricky code.

- Mark

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: mark.lillibridge <at> hp.com
Cc: 7746 <at> debbugs.gnu.org
Subject: Re: bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing
	unrmail to lose mail
Date: Sun, 02 Jan 2011 22:36:55 -0500

> After some thought, I figured out why you your code doesn't work:
>   it turns "( \(  )" into "( \"

Hmm... not in my test:

       (let ((address "(sadffds \\( adf)") pos)
         (while (setq pos (string-match
                          ;; This doesn't hack rfc822 nested comments
                          ;;  `(xyzzy (foo) whinge)' properly.  Big deal.
                          "[ \t]*(\\([^()\\]\\|\\\\.\\|\\\\\n\\)*)"
                          address))
           (setq address (replace-match "" nil nil address 0)))
         address)

returns "".


        Stefan

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

From: Mark Lillibridge <mark.lillibridge <at> hp.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 7746 <at> debbugs.gnu.org
Subject: Re: bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing
	unrmail to lose mail
Date: Sun, 02 Jan 2011 20:13:13 -0800

    Opps.  My bad.  On closer look, I can't see any way to trip up the
code.  I'm it running against my e-mail corpus now.

- Mark

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

From: Mark Lillibridge <mark.lillibridge <at> hp.com>
To: monnier <at> iro.umontreal.ca, 7746 <at> debbugs.gnu.org
Subject: Re: bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing
	unrmail to lose mail
Date: Mon, 03 Jan 2011 11:17:16 -0800

    I ran the change against my e-mail corpus and my sanity checker
didn't find any problems for what it's worth.

- Mark

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: mark.lillibridge <at> hp.com
Cc: 7746 <at> debbugs.gnu.org
Subject: Re: bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing
	unrmail to lose mail
Date: Wed, 05 Jan 2011 16:32:19 -0500

>     I ran the change against my e-mail corpus and my sanity checker
> didn't find any problems for what it's worth.

Thanks for confirming.
I've installed that change on the trunk (i.e. for Emacs-24).


        Stefan

Previous Next

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