bug in message.el, message-forward-make-body-plain

bug in message.el, message-forward-make-body-plain

[Ted Stern]

message-forward ignores message-forward-ignored-headers when the prefix
argument is non-nil.

To demonstrate:  Change the value of message-forward-ignored-headers globally,

(setq message-forward-ignored-headers
      (concat "^"
              (regexp-opt '(
                            "Content-"
                            "X"
                            "Path"
                            "NTTP"
                            "User"
                            "Received"
                            "Return"
                            "Message"
                            "MIME"
                            "References"
                            "In-"
                            "Status"
                            "Lines"
                            "List") t) ".*:"))

Now enter any gnus group, and attempt to forward a message using

    C-u C-c C-f

Instead of removing all but 4 or 5 relevant headers, nothing is changed.

The same error occurs with

    M-4 C-c C-f
-- 
 Ted Stern                                 Applications Group
 Cray Inc.                               office: 206-701-2182
 411 First Avenue South, Suite 600         cell: 206-383-1049
 Seattle, WA 98104-2860                     FAX: 206-701-2500

 Frango ut patefaciam -- I break that I may reveal
 (The Paleontological Society motto, equally apropos for debugging)

Re: bug in message.el, message-forward-make-body-plain

[Katsumi Yamaoka]

>>>>> In <yccsm6r9k26.fsf@monkeypuzzle.wc.cray.com> Ted Stern wrote:

> message-forward ignores message-forward-ignored-headers when the prefix
> argument is non-nil.

> To demonstrate:  Change the value of message-forward-ignored-headers globally,

[...]

> Now enter any gnus group, and attempt to forward a message using
>     C-u C-c C-f
> Instead of removing all but 4 or 5 relevant headers, nothing is changed.
> The same error occurs with
>     M-4 C-c C-f

I wrote the code for it, and then, I found a note when I was
going to write the ChangeLog file.

2002-07-27  Simon Josefsson  <jas@extundo.com>

	* message.el (message-forward-make-body): Don't use
	`message-forward-ignored-headers' when doing a "raw" followup (it
	is important to preserve e.g. CTE).

To forbid removing any headers will be overkill, however
message-forward-ignored-headers contains CTE, such an important
header, even now by default.  I think excluding CTE from m-f-i-h
may not solve the problem.  It is because m-f-i-h is a user
option and users can include any headers in it.  Any ideas?

Re: bug in message.el, message-forward-make-body-plain

[Ted Stern]

Thank you, Yamaoka-san, for your prompt response!  Comments below.

On 1 Dec 2004 at 00:19 PST, Katsumi Yamaoka wrote:
>>>>>> In <yccsm6r9k26.fsf@monkeypuzzle.wc.cray.com> Ted Stern wrote:
>
>> message-forward ignores message-forward-ignored-headers when the prefix
>> argument is non-nil.
>
>> To demonstrate: Change the value of message-forward-ignored-headers globally,
>
> [...]
>
>> Now enter any gnus group, and attempt to forward a message using
>>     C-u C-c C-f
>> Instead of removing all but 4 or 5 relevant headers, nothing is changed.
>> The same error occurs with
>>     M-4 C-c C-f
>
> I wrote the code for it, and then, I found a note when I was
> going to write the ChangeLog file.
>
> 2002-07-27  Simon Josefsson  <jas@extundo.com>
>
> 	* message.el (message-forward-make-body): Don't use
> 	`message-forward-ignored-headers' when doing a "raw" followup (it
> 	is important to preserve e.g. CTE).
>
> To forbid removing any headers will be overkill, however
> message-forward-ignored-headers contains CTE, such an important
> header, even now by default.  I think excluding CTE from m-f-i-h
> may not solve the problem.  It is because m-f-i-h is a user
> option and users can include any headers in it.  Any ideas?

The stated intent of Simon's change was to ignore
message-forward-ignored-headers with raw followups, not with forwarding.  So
his change, which affects forwarding as well as followups, looks like an
unintended bug, which apparently hasn't been objectionable enough to other
users to be found before now.  I guess I'm just picky.

Plus, the bug I describe above takes effect in
message-forward-make-body-plain, not message-forward-make-body.

The cases Simon addressed should be possible to differentiate using the prefix
argument, no?  Specifically, if the prefix indicates inline 
   arg=nil and (message-forward-as-mime=nil or message-forward-show-mml=nil)
   arg=1
   arg=4
instead of rfc822 MIME.  Or am I still missing something?

C-c C-f runs the command gnus-summary-mail-forward
   which is an interactive compiled Lisp function in `gnus-msg'.
(gnus-summary-mail-forward &optional ARG POST)

Forward the current message(s) to another user.
If process marks exist, forward all marked messages;
if ARG is nil, see `message-forward-as-mime' and `message-forward-show-mml';
if ARG is 1, decode the message and forward directly inline;
if ARG is 2, forward message as an rfc822 MIME section;
if ARG is 3, decode message and forward as an rfc822 MIME section;
if ARG is 4, forward message directly inline;
otherwise, use flipped `message-forward-as-mime'.
If POST, post instead of mail.
For the `inline' alternatives, also see the variable
`message-forward-ignored-headers'.

Ted
-- 
 Ted Stern                                 Applications Group
 Cray Inc.                               office: 206-701-2182
 411 First Avenue South, Suite 600         cell: 206-383-1049
 Seattle, WA 98104-2860                     FAX: 206-701-2500

 Frango ut patefaciam -- I break that I may reveal
 (The Paleontological Society motto, equally apropos for debugging)

Re: bug in message.el, message-forward-make-body-plain

[Romain Francoise]

The fix looks simple enough; the problem appears to be in this change:

<URL: http://quimby.gnus.org/cgi-bin/cvsweb.cgi/gnus/lisp/message.el.diff?r1=6.377&r2=6.378>

With the following ChangeLog entry:

2003-10-29  Simon Josefsson  <jas@extundo.com>

	* message.el (message-forward-make-body-plain): Remove ignored
	headers.  Tiny patch from Andre Srinivasan <andre@e2open.com>.
	(message-forward-make-body-plain): Fix ARG=1.

The test is weird:

(when (and (not current-prefix-arg)
           message-forward-ignored-headers)

how can current-prefix-arg be nil since you call this function from C-u
C-c C-f?  Its value is '(4) here... unless I'm missing something.

Index: message.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/message.el,v
retrieving revision 7.63
diff -u -r7.63 message.el
--- message.el	26 Nov 2004 03:07:29 -0000	7.63
+++ message.el	1 Dec 2004 19:46:41 -0000
@@ -6257,8 +6263,7 @@
        (buffer-string)))
     (setq e (point))
     (insert
-    (when (and (not current-prefix-arg)
-	       message-forward-ignored-headers)
+    (when message-forward-ignored-headers
       (save-restriction
 	(narrow-to-region b e)
 	(goto-char b)

-- 
Romain Francoise <romain@orebokech.com> | Shine the headlight, straight
it's a miracle -- http://orebokech.com/ | into my eyes.

Re: bug in message.el, message-forward-make-body-plain

[Ted Stern]

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

On 1 Dec 2004 at 11:48 PST, Romain Francoise wrote:
> The fix looks simple enough; the problem appears to be in this change:
>
> <URL: http://quimby.gnus.org/cgi-bin/cvsweb.cgi/gnus/lisp/message.el.diff?r1=6.377&r2=6.378>
>
> With the following ChangeLog entry:
>
> 2003-10-29  Simon Josefsson  <jas@extundo.com>
>
> 	* message.el (message-forward-make-body-plain): Remove ignored
> 	headers.  Tiny patch from Andre Srinivasan <andre@e2open.com>.
> 	(message-forward-make-body-plain): Fix ARG=1.
>
> The test is weird:
>
> (when (and (not current-prefix-arg)
>            message-forward-ignored-headers)
>
> how can current-prefix-arg be nil since you call this function from C-u
> C-c C-f?  Its value is '(4) here... unless I'm missing something.
>
> Index: message.el
> ===================================================================
> RCS file: /usr/local/cvsroot/gnus/lisp/message.el,v
> retrieving revision 7.63
> diff -u -r7.63 message.el
> --- message.el	26 Nov 2004 03:07:29 -0000	7.63
> +++ message.el	1 Dec 2004 19:46:41 -0000
> @@ -6257,8 +6263,7 @@
>         (buffer-string)))
>      (setq e (point))
>      (insert
> -    (when (and (not current-prefix-arg)
> -	       message-forward-ignored-headers)
> +    (when message-forward-ignored-headers
>        (save-restriction
>  	(narrow-to-region b e)
>  	(goto-char b)

Thanks Romain,

This patch works for me, but the offsets are wrong.  After making the change
manually I am able to advise gnus-summary-mail-forward correctly to do what I
want (remove unwanted headers when not forwarding as rfc822 MIME).

I did my own diff and am attaching it as inline MIME to avoid space problems.

Ted
-- 
 Ted Stern                                 Applications Group
 Cray Inc.                               office: 206-701-2182
 411 First Avenue South, Suite 600         cell: 206-383-1049
 Seattle, WA 98104-2860                     FAX: 206-701-2500

 Frango ut patefaciam -- I break that I may reveal
 (The Paleontological Society motto, equally apropos for debugging)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: message.el patch for message-forward-make-body-plain bug --]
[-- Type: text/x-patch, Size: 606 bytes --]

Index: message.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/message.el,v
retrieving revision 7.63
diff -u -r7.63 message.el
--- message.el	26 Nov 2004 03:07:29 -0000	7.63
+++ message.el	1 Dec 2004 21:37:22 -0000
@@ -6258,8 +6258,7 @@
     (setq e (point))
     (insert
      "\n-------------------- End of forwarded message --------------------\n")
-    (when (and (not current-prefix-arg)
-	       message-forward-ignored-headers)
+    (when message-forward-ignored-headers
       (save-restriction
 	(narrow-to-region b e)
 	(goto-char b)

Re: bug in message.el, message-forward-make-body-plain

[Romain Francoise]

Ted Stern <stern+gnus@cray.com> writes:

> This patch works for me, but the offsets are wrong.

Sorry about this, I edited the patch since it mixed with other local
changes I have in message.el; glad to know it solves the problem for
you.  Katsumi, what do you think?

-- 
Romain Francoise <romain@orebokech.com> | They're nothing but cold
it's a miracle -- http://orebokech.com/ | little demons.

Re: bug in message.el, message-forward-make-body-plain

[Katsumi Yamaoka]

>>>>> In <87hdn5y970.fsf@orebokech.com> Romain Francoise wrote:

> Katsumi, what do you think?

I think your patch is the best solution.  I've installed it.

>>>>> In <87y8ghyf39.fsf@orebokech.com> Romain Francoise wrote:

> 2003-10-29  Simon Josefsson  <jas@extundo.com>

> 	* message.el (message-forward-make-body-plain): Remove ignored
> 	headers.  Tiny patch from Andre Srinivasan <andre@e2open.com>.
> 	(message-forward-make-body-plain): Fix ARG=1.

> The test is weird:

> (when (and (not current-prefix-arg)
>            message-forward-ignored-headers)

> how can current-prefix-arg be nil since you call this function from C-u
> C-c C-f?  Its value is '(4) here... unless I'm missing something.

It is likely if a user has set message-forward-as-mime as nil
and uses the `C-c C-f' command.  I guess that was the mere copy
of an old code.  In addition, I noticed there's also a problem
similar to this when the `3 C-c C-f' command is used, and fixed
it.  Anyway, I'm thankful to Ted and Romain who gave me a good
understanding.

BTW, even for raw forwarding, isn't there a demand to remove
some headers?  For instance, a lot of Received headers will
usually be unnecessary.  Furthermore, if a header such as the
following

Xref: localhost trash:99999

is forwarded, the person who wrote the message may be offended.
Of course, we can remove them manually, though.

Re: bug in message.el, message-forward-make-body-plain

[Romain Francoise]

Katsumi Yamaoka <yamaoka@jpl.org> writes:

> I think your patch is the best solution.  I've installed it.

Thanks!

> It is likely if a user has set message-forward-as-mime as nil
> and uses the `C-c C-f' command.  I guess that was the mere copy
> of an old code.

Right, that's probably why.

> BTW, even for raw forwarding, isn't there a demand to remove
> some headers?  For instance, a lot of Received headers will
> usually be unnecessary.

I don't think we should remove them by default: they're useful when
forwarding spam to an abuse address, or for debugging mail loops, etc.
An inexperienced user might not know how to change the value of m-f-i-h
to get all headers back.

> Furthermore, if a header such as the following

> Xref: localhost trash:99999

> is forwarded, the person who wrote the message may be offended.
> Of course, we can remove them manually, though.

Yes, I usually remove all such headers manually.  But removing Xref
could be useful since it's internal to Gnus and not useful to anyone
apart from the sender.

-- 
Romain Francoise <romain@orebokech.com> | This is a man's man's man's
it's a miracle -- http://orebokech.com/ | world. --James Brown

Re: bug in message.el, message-forward-make-body-plain

[Kai Grossjohann]

Romain Francoise <romain@orebokech.com> writes:

> The test is weird:
>
> (when (and (not current-prefix-arg)
>            message-forward-ignored-headers)
>
> how can current-prefix-arg be nil since you call this function from C-u
> C-c C-f?  Its value is '(4) here... unless I'm missing something.

There is a variable message-forward-as-mime which inverts the meaning
of the prefix arg.

I think there is still a problem in the code, though.  Hm.

Kai

Re: bug in message.el, message-forward-make-body-plain

[Ted Stern]

On 19 Dec 2004 at 14:26 PST, Kai Grossjohann wrote:
> Romain Francoise <romain@orebokech.com> writes:
>
>> The test is weird:
>>
>> (when (and (not current-prefix-arg)
>>            message-forward-ignored-headers)
>>
>> how can current-prefix-arg be nil since you call this function from C-u
>> C-c C-f?  Its value is '(4) here... unless I'm missing something.
>
> There is a variable message-forward-as-mime which inverts the meaning
> of the prefix arg.
>
> I think there is still a problem in the code, though.  Hm.
>
> Kai

Have you tried the CVS version lately?  It's working for me.  Maybe I haven't
hit the pathological case yet, though.

Ted
-- 
 Ted Stern                                 Applications Group
 Cray Inc.                               office: 206-701-2182
 411 First Avenue South, Suite 600         cell: 206-383-1049
 Seattle, WA 98104-2860                     FAX: 206-701-2500

 Frango ut patefaciam -- I break that I may reveal
 (The Paleontological Society motto, equally apropos for debugging)