all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
@ 2010-12-28  0:17 Mark Lillibridge
  2011-01-02  2:43 ` Glenn Morris
  2011-01-02  3:21 ` Stefan Monnier
  0 siblings, 2 replies; 8+ messages in thread
From: Mark Lillibridge @ 2010-12-28  0:17 UTC (permalink / raw)
  To: 7746


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)))





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
  2010-12-28  0:17 bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail Mark Lillibridge
@ 2011-01-02  2:43 ` Glenn Morris
  2011-01-02  3:21 ` Stefan Monnier
  1 sibling, 0 replies; 8+ messages in thread
From: Glenn Morris @ 2011-01-02  2:43 UTC (permalink / raw)
  To: 7746-done

Version: 23.3

Thanks; applied.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
  2010-12-28  0:17 bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail Mark Lillibridge
  2011-01-02  2:43 ` Glenn Morris
@ 2011-01-02  3:21 ` Stefan Monnier
  2011-01-03  0:18   ` Mark Lillibridge
  1 sibling, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2011-01-02  3:21 UTC (permalink / raw)
  To: mark.lillibridge; +Cc: 7746

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)






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
  2011-01-02  3:21 ` Stefan Monnier
@ 2011-01-03  0:18   ` Mark Lillibridge
  2011-01-03  3:36     ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Lillibridge @ 2011-01-03  0:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 7746


>  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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
  2011-01-03  0:18   ` Mark Lillibridge
@ 2011-01-03  3:36     ` Stefan Monnier
  2011-01-03  4:13       ` Mark Lillibridge
  0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2011-01-03  3:36 UTC (permalink / raw)
  To: mark.lillibridge; +Cc: 7746

> 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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
  2011-01-03  3:36     ` Stefan Monnier
@ 2011-01-03  4:13       ` Mark Lillibridge
  2011-01-03 19:17         ` Mark Lillibridge
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Lillibridge @ 2011-01-03  4:13 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 7746


    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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
  2011-01-03  4:13       ` Mark Lillibridge
@ 2011-01-03 19:17         ` Mark Lillibridge
  2011-01-05 21:32           ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Lillibridge @ 2011-01-03 19:17 UTC (permalink / raw)
  To: monnier, 7746


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

- Mark





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail
  2011-01-03 19:17         ` Mark Lillibridge
@ 2011-01-05 21:32           ` Stefan Monnier
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Monnier @ 2011-01-05 21:32 UTC (permalink / raw)
  To: mark.lillibridge; +Cc: 7746

>     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





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-01-05 21:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-28  0:17 bug#7746: SERIOUS BUG: mail-strip-quoted-names bug causing unrmail to lose mail Mark Lillibridge
2011-01-02  2:43 ` Glenn Morris
2011-01-02  3:21 ` Stefan Monnier
2011-01-03  0:18   ` Mark Lillibridge
2011-01-03  3:36     ` Stefan Monnier
2011-01-03  4:13       ` Mark Lillibridge
2011-01-03 19:17         ` Mark Lillibridge
2011-01-05 21:32           ` Stefan Monnier

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.