bug#42029: `gnus-registry-spool-action' gets field beyond message headers

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#42029: `gnus-registry-spool-action' gets field beyond message headers

tomotaka.suwa
Hi,

I've been suffering from `mail-source-crash-box' on getting new mail.

After some debug and investigation, I noticed that
`mail-extract-address-components' was failing by invalid addresses.

The issue happened in `gnus-registry-spool-action' and invalid addresses
are passed by calling `message-fetch-field' on the buffer not narrowed
to message headers.

Below snippet reproduce the root issue:

(with-temp-buffer
  (save-excursion
    ;; mail header
    (insert "From: [hidden email]\n"
            "To: [hidden email]\n"
            "Subject: test\n")
    (newline)
    ;; mail body
    (insert "message\n"
            "Cc: >,@ <[hidden email]>\n")) ;; by incorrect decode
  (gnus-registry-spool-action 1 "test"))

In stead of `message-fetch-field', calling `message-field-value' would
solve the problem since it ensures the buffer is narrowed at first.

diff -u "d:/msys64/mingw64/share/emacs/26.3/lisp/gnus/gnus-registry.el.orig" "d:/msys64/mingw64/share/emacs/26.3/lisp/gnus/gnus-registry.el"
--- d:/msys64/mingw64/share/emacs/26.3/lisp/gnus/gnus-registry.el.orig 2020-06-24 11:10:49.458397900 +0900
+++ d:/msys64/mingw64/share/emacs/26.3/lisp/gnus/gnus-registry.el 2020-06-23 11:08:23.170050000 +0900
@@ -405,10 +405,10 @@
   (let ((to (gnus-group-guess-full-name-from-command-method group))
           (recipients (or recipients
                           (gnus-registry-sort-addresses
-                           (or (message-fetch-field "cc") "")
-                           (or (message-fetch-field "to") ""))))
-          (subject (or subject (message-fetch-field "subject")))
-          (sender (or sender (message-fetch-field "from"))))
+                           (or (message-field-value "cc") "")
+                           (or (message-field-value "to") ""))))
+          (subject (or subject (message-field-value "subject")))
+          (sender (or sender (message-field-value "from"))))
       (when (and (stringp id) (string-match "\r$" id))
         (setq id (substring id 0 -1)))
       (gnus-message 7 "Gnus registry: article %s spooled to %s"

Diff finished.  Wed Jun 24 11:13:17 2020

Gnus v5.13
GNU Emacs 26.3 (build 1, x86_64-w64-mingw32)
 of 2020-04-04



Reply | Threaded
Open this post in threaded view
|

bug#42029: `gnus-registry-spool-action' gets field beyond message headers

Eric Abrahamsen-2
Tomotaka SUWA <[hidden email]> writes:

>> Thanks for this report. It might be simpler to wrap the whole thing in a
>> single save-restriction+narrow-to-headers, since the function gets
>> called four times. What do you think?
>
> I wrote the patch paying attention to minimize side effects since I'm not
> familiar with that functionality. So if `gnus-registry-spool-action' is
> interested in only mail headers, your proposal is much better.

Okay! Would you be willing to give the attached diff a quick test?

Thanks,
Eric


protect-registry-spool-action.diff (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#42029: `gnus-registry-spool-action' gets field beyond message headers

Basil L. Contovounesios
Eric Abrahamsen <[hidden email]> writes:

> +  (save-excursion
> +    (message-narrow-to-headers-or-head)

Shouldn't this additionally or instead be wrapped in save-restriction?

Thanks,

--
Basil



Reply | Threaded
Open this post in threaded view
|

bug#42029: `gnus-registry-spool-action' gets field beyond message headers

Eric Abrahamsen-2

On 06/26/20 22:24 PM, Basil L. Contovounesios wrote:
> Eric Abrahamsen <[hidden email]> writes:
>
>> +  (save-excursion
>> +    (message-narrow-to-headers-or-head)
>
> Shouldn't this additionally or instead be wrapped in save-restriction?

Bleagh, you're right, sorry about that. It was supposed to be
`save-restriction'.

Thanks!
Eric



Reply | Threaded
Open this post in threaded view
|

bug#42029: `gnus-registry-spool-action' gets field beyond message headers

tomotaka.suwa
In reply to this post by Eric Abrahamsen-2
> Okay! Would you be willing to give the attached diff a quick test?

Sure. I gave a test with the `save-restriction' version and it
worked well.

By the way, does the change of `gnus-registry-get-article-marks'
have any relation to this issue?

Thanks,

--
Tomotaka SUWA