need help tracking down a subtle bug with unrmail/mail-strip-quoted-names

need help tracking down a subtle bug with unrmail/mail-strip-quoted-names

[Mark Lillibridge]

    I've been working on tracking down the cause of a bug with unrmail
(versions 23.1, 24) and have reached the point where I need some help.

    The original symptoms were that unrmail only processes some messages
in certain BABYL files.  Making a long story short, messages with nested
from lines like:

From: Knut Are Romann-Aas <kromanna@sn.no> (by way of justicek@edge.ercnet.com (Kim

cause unrmail to stop after processing them.  (This is a very serious
bug as it causes the loss of mail in practice.)  The relevant code in
unrmail from mail/unrmail.el appears to be:

(defun unrmail (file to-file)
  ...
  (with-temp-buffer
    ...
    (let ((temp-buffer (get-buffer-create " unrmail"))
          (from-buffer (current-buffer)))

      ;; Process the messages one by one.
      (while (re-search-forward "^\^_\^l" nil t)
          ...
          (with-current-buffer temp-buffer
            ...
            (save-restriction
              (narrow-to-region
               (point-min)
               (save-excursion (search-forward "\n\n" nil 'move) (point)))

              ;; Fetch or construct what we should use in the `From ' line.
              (setq mail-from (or (let ((from (mail-fetch-field "Mail-From")))
                                    ;; mail-mbox-from (below) returns a
                                    ;; string that ends in a newline, but
                                    ;; but mail-fetch-field does not, so
                                    ;; we append a newline here.
                                    (if from
                                        (format "%s\n" from)))
                                  (mail-mbox-from)))
            )
            ;; Write it to the output file, suitably encoded.
            (let ((coding-system-for-write coding))
              (write-region (point-min) (point-max) to-file t
                            'nomsg)))))
      (kill-buffer temp-buffer))
    (message "Writing messages to %s...done" to-file)))


Point seems to incorrectly change to 1 when the with-current-buffer call
finishes iff the bug occurs.  If I turn the call to (mail-mbox-from) to
"dummy", the bug goes away (all messages are processed).  The relevant
code from that function from mail-utils.el:

(defun mail-mbox-from ()
  "Return an mbox \"From \" line for the current message.
The buffer should be narrowed to just the header."
  (let ((from (or (mail-fetch-field "from")
                  (mail-fetch-field "really-from")
                  (mail-fetch-field "sender")
                  "unknown"))
        (date (mail-fetch-field "date")))
    (format "From %s %s\n" (mail-strip-quoted-names from)
            (or (and date
                     (ignore-errors
                      (current-time-string (date-to-time date))))
                (current-time-string)))))

If I turn (mail-strip-quoted-names from) into "dummy" again, the bug
goes away.  The relevant code for that function (same file) is:

(defun mail-strip-quoted-names (address)
  (if (null address)
      nil
    (if mail-use-rfc822
        (progn (require 'rfc822)
               (mapconcat 'identity (rfc822-addresses address) ", "))
      (let (pos)

       ;; Detect nested comments.
       (if (string-match "[ \t]*(\\([^)\\]\\|\\\\.\\|\\\\\n\\)*(" address)
           ;; Strip nested comments.
           (with-current-buffer (get-buffer-create " *temp*")
             ...
             (erase-buffer))
           ...
       ... ; code not involving buffers
)

In particular, if I replace the call in unrmail to mail-mbox-from
(really changing the relevant setq to setq mail-from "dummy" and adding
the following line) with the following:

              (with-current-buffer (get-buffer-create " *temp*") (erase-buffer))

it causes the bug at the first message.  However, the following variant
does not cause the bug for any message:

              (with-current-buffer (get-buffer-create " *temp*") "dummy")

This is the point where I need help; I don't just understand why erasing
a temporary buffer should move point in a different buffer.

- Thanks,
  Mark
PS, in case it makes it clearer, the revised code looks like:

(defun unrmail (file to-file)
  ...
  (with-temp-buffer
    ...
    (let ((temp-buffer (get-buffer-create " unrmail"))
          (from-buffer (current-buffer)))

      ;; Process the messages one by one.
      (while (re-search-forward "^\^_\^l" nil t)
          ...
          (with-current-buffer temp-buffer
            ...
            (save-restriction
              (narrow-to-region
               (point-min)
               (save-excursion (search-forward "\n\n" nil 'move) (point)))

              (with-current-buffer (get-buffer-create " *temp*") (erase-buffer))
            )
            ...
            ;; Write it to the output file, suitably encoded.
            (let ((coding-system-for-write coding))
              (write-region (point-min) (point-max) to-file t
                            'nomsg))
          ) ; end with-current-buffer
          ; POINT BECOMES 1 HERE for some reason
      ))
      (kill-buffer temp-buffer))
    (message "Writing messages to %s...done" to-file)))

Re: need help tracking down a subtle bug with unrmail/mail-strip-quoted-names

[Andreas Schwab]

Mark Lillibridge <mark.lillibridge@hp.com> writes:

> From: Knut Are Romann-Aas <kromanna@sn.no> (by way of justicek@edge.ercnet.com (Kim

This is not a valid header line, it contains an unterminated comment.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: need help tracking down a subtle bug with unrmail/mail-strip-quoted-names

[Mark Lillibridge]

>  Mark Lillibridge <mark.lillibridge@hp.com> writes:
>  
>  > From: Knut Are Romann-Aas <kromanna@sn.no> (by way of justicek@edge.ercnet.com (Kim
>  
>  This is not a valid header line, it contains an unterminated comment.
>  
>  Andreas.


Sorry, my opps; the full header is:

From: Knut Are Romann-Aas <kromanna@sn.no> (by way of justicek@edge.ercnet.com (Kim
 Justice))

(yes, that's 2 lines, the second line indented.)  And yes, this is a
real header line from a real message I received in 1996.  I have a few
other messages that trickle this bug.

- Mark

Re: need help tracking down a subtle bug with unrmail/mail-strip-quoted-names

[Mark Lillibridge]

    Ah!  I found the bug.  I will submit a bug report with a patch, but
for those curious, here's what the problem was:

unrmail starts with a with-temp-buffer, which switches to the buffer
" *temp*" because it does not yet exist:

  (defmacro with-temp-buffer (&rest body)
    "Create a temporary buffer, and evaluate BODY there like `progn'.
  See also `with-temp-file' and `with-output-to-string'."
    (declare (indent 0) (debug t))
    (let ((temp-buffer (make-symbol "temp-buffer")))
      `(let ((,temp-buffer (generate-new-buffer " *temp*")))
         ;; FIXME: kill-buffer can change current-buffer in some odd cases.
         (with-current-buffer ,temp-buffer
           (unwind-protect
  	     (progn ,@body)
             (and (buffer-name ,temp-buffer)
                  (kill-buffer ,temp-buffer)))))))

When mail-strip-quoted-names does the following:

	   (with-current-buffer (get-buffer-create " *temp*")
	     (erase-buffer)
	     ...

it erases that buffer!  The code above is simply wrong as it erases a
possibly existing buffer.  It should be:

	   (with-temp-buffer
	      ...

This will not step on any existing buffers and avoids the need for
erasing any buffers.

- Mark