Re: iso-8859-2 characters are broken in forwarded messages.

Re: iso-8859-2 characters are broken in forwarded messages.

[Reiner Steib]

On Thu, Oct 16 2003, Maciej Matysiak wrote:

> i've just noticed that when i c-c c-f an article with iso-8859-2
> characters in the body, they are changed (into iso-8859-1 i think)
> in newly created *mail* buffer. that makes forwarding such messages
> not possible without correcting the content :(
>
> (few iso-8859-2 to make that message vulnerable to this bug: ęąśżźćńłó).

I cannot reproduce the bug with "Gnus/5.1003 (Gnus v5.10.3) Emacs/21.3
(gnu/linux)".  (I've forwarded your mail to you.)

> i'm using very fresh cvs gnus.

From 2003-10-13 or newer?  Newer then 2003-09-17?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo--- PGP key available via WWW   http://rsteib.home.pages.de/

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Maciej Matysiak <phoner.ding@blah.pl> writes:

> On the 16th of October 2003 at 00:26, Reiner Steib <4.uce.03.r.s#nurfuerspam.de> wrote:
>
>> I cannot reproduce the bug with "Gnus/5.1003 (Gnus v5.10.3) Emacs/21.3
>> (gnu/linux)".
>
> i'm confused. i'm using xemacs 21.4 (patch 6) "common lisp" [lucid]
> (i386-debian-linux, mule), but i'm not sure if that matters much.
>
> an example. my previous message to this list looks like:
>  http://administrat.org/gnus-forward-problem-1.png
> after i press c-c c-f, i get buffer which looks like:
>  http://administrat.org/gnus-forward-problem-2.png
> bad.

Could you explain what is bad?  The second buffer look fine to me:
there is a charset=iso-8859-2 tag and the characters are QP encoded.
I get the same look when I press C-c C-f on your original message
here, and if I preview that message, the ISO-8859-2 characters look
fine.

> what could cause such behaviour? i can't think of any changes in my .gnus
> file, that could affect message composing and stuff.

Perhaps specific to XEmacs?  OTOH, since the buffer contain
pre-processed MIME, I suspect there is some other problem.

> talking about edebug, how does one undo the "edebug-defun"? so the function
> would not be watched anymore?

I usually locate the function and press C-x C-e after it, to
re-evaluate it.

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Maciej Matysiak <phoner.ding@blah.pl> writes:

> the cause of my confusion is: message-forward-show-mml. i have written a lot
> of explaination and made few more screenshots before finding it and setting to
> t. is there any reason that the default value is nil? it's not backward
> compatible that way and confuses old people who don't like changes, like me ;)
There should definitely be a NEWS entry for this change in defaults.
I added the entry below to GNUS-NEWS.  Thanks.  (Perhaps it should
detect if MIME->MML->MIME will corrupt the forward, and decode MIME to
MML when it is safe to do so.)

** The default for message-forward-show-mml changed to nil.

The reason is that decoding MIME, showing MML, and later re-encode the
MIME does not work properly for, e.g., PGP/MIME signed messages.

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> ** The default for message-forward-show-mml changed to nil.
>
> The reason is that decoding MIME, showing MML, and later re-encode the
> MIME does not work properly for, e.g., PGP/MIME signed messages.

Why doesn't it work properly for PGP/MIME signed messages?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> ** The default for message-forward-show-mml changed to nil.
>>
>> The reason is that decoding MIME, showing MML, and later re-encode the
>> MIME does not work properly for, e.g., PGP/MIME signed messages.
>
> Why doesn't it work properly for PGP/MIME signed messages?

PGP/MIME compute hashes over the MIME headers.  If the headers change,
which is quite likely during MIME->MML->MIME, the signature is
invalid.

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> PGP/MIME compute hashes over the MIME headers.  If the headers change,
> which is quite likely during MIME->MML->MIME, the signature is
> invalid.

Hm; I see.  But couldn't one just re-sign the message when
re-miming it?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> PGP/MIME compute hashes over the MIME headers.  If the headers change,
>> which is quite likely during MIME->MML->MIME, the signature is
>> invalid.
>
> Hm; I see.  But couldn't one just re-sign the message when
> re-miming it?

What if you are forwarding someone else's message?  It would be
difficult for Gnus to re-sign that.

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> What if you are forwarding someone else's message?  It would be
> difficult for Gnus to re-sign that.

Indeed; that would be most difficult...

Hm...  it is nice to be able to edit the MML stuff when editing,
though.  Could we add something to not do MML when forwarding a PGP
message, but do it otherwise?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Hm...  it is nice to be able to edit the MML stuff when editing,
> though.  Could we add something to not do MML when forwarding a PGP
> message, but do it otherwise?

Yes, that sounds like a good idea.  Never MIME->MML convert
multipart/{signed,encrypted} MIME parts.

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> Yes, that sounds like a good idea.  Never MIME->MML convert
> multipart/{signed,encrypted} MIME parts.

What would be the simplest fix for `message-forward-make-body' to
make it detect whether there are signed/encrypted parts in the
message?

(`message-forward-as-mime' should have a third value,
`all-except-signed' or something to trigger the "guessing" code.) 

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> Yes, that sounds like a good idea.  Never MIME->MML convert
>> multipart/{signed,encrypted} MIME parts.
>
> What would be the simplest fix for `message-forward-make-body' to
> make it detect whether there are signed/encrypted parts in the
> message?

Try the MIME->MML conversion and re-search for '<#part
type=multipart/(signed|encrypted)' and fall back to raw MIME found?

> (`message-forward-as-mime' should have a third value,
> `all-except-signed' or something to trigger the "guessing" code.) 

I think it should be `message-forward-show-mml' instead of m-f-a-m.

The function look somewhat convoluted, I can't think of a minimal
patch that solves the problem.  I think the best would be to split up
the code into message-forward-make-body-digest,
message-forward-make-body-mime, message-forward-make-body-mml and
message-forward-make-body-plain and make the top-level
message-forward-make-body contain some logic and then call one of the
real functions.

And can't we remove the following as well?  I think it would be nice
to get rid of all of these. IMHO, they should only be used in the
(very few) places where it is actually required.

	  (goto-char (point-min))
	  (when (looking-at "From ")
	    (replace-match "X-From-Line: "))
	  (goto-char (point-max)))))

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> Try the MIME->MML conversion and re-search for '<#part
> type=multipart/(signed|encrypted)' and fall back to raw MIME found?

Yes...

> I think it should be `message-forward-show-mml' instead of m-f-a-m.

Yup.

> The function look somewhat convoluted, I can't think of a minimal
> patch that solves the problem.  I think the best would be to split up
> the code into message-forward-make-body-digest,
> message-forward-make-body-mime, message-forward-make-body-mml and
> message-forward-make-body-plain and make the top-level
> message-forward-make-body contain some logic and then call one of the
> real functions.

I think that sounds like a much cleaner solution; yes.

> And can't we remove the following as well?  I think it would be nice
> to get rid of all of these. IMHO, they should only be used in the
> (very few) places where it is actually required.
>
> 	  (goto-char (point-min))
> 	  (when (looking-at "From ")
> 	    (replace-match "X-From-Line: "))
> 	  (goto-char (point-max)))))

They are there to protect against confused MTAs and mail readers, I
guess...

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

>> The function look somewhat convoluted, I can't think of a minimal
>> patch that solves the problem.  I think the best would be to split up
>> the code into message-forward-make-body-digest,
>> message-forward-make-body-mime, message-forward-make-body-mml and
>> message-forward-make-body-plain and make the top-level
>> message-forward-make-body contain some logic and then call one of the
>> real functions.
>
> I think that sounds like a much cleaner solution; yes.

How about this?  It doesn't contain anything regarding
multipart/{signed,encrypted}, but that could be added once this has
been added.

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

	* message.el (message-forward-make-body-plain)
	(message-forward-make-body-mime, message-forward-make-body-mml)
	(message-forward-make-body-digest-plain)
	(message-forward-make-body-digest-mime)
	(message-forward-make-body-digest): New, derived from
	message-forward-make-body.
	(message-forward-make-body): Use them.

Index: message.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/message.el,v
retrieving revision 6.366
diff -u -p -u -w -r6.366 message.el
--- message.el	16 Oct 2003 18:36:52 -0000	6.366
+++ message.el	18 Oct 2003 01:38:07 -0000
@@ -5977,27 +5977,38 @@ Optional DIGEST will use digest to forwa
       (message-mail nil subject))
     (message-forward-make-body cur digest)))
 
-;;;###autoload
-(defun message-forward-make-body (forward-buffer &optional digest)
-  ;; Put point where we want it before inserting the forwarded
-  ;; message.
-  (if message-forward-before-signature
-      (message-goto-body)
+(defun message-forward-make-body-plain (forward-buffer)
+  (insert
+   "\n-------------------- Start of forwarded message --------------------\n")
+  (let ((b (point)) e)
+    (save-restriction
+      (narrow-to-region (point) (point))
+      (mml-insert-buffer forward-buffer)
+      (goto-char (point-min))
+      (when (looking-at "From ")
+	(replace-match "X-From-Line: "))
     (goto-char (point-max)))
-  (if message-forward-as-mime
-      (if digest
-	  (insert "\n<#multipart type=digest>\n")
-	(if message-forward-show-mml
+    (setq e (point))
+    (insert
+     "\n-------------------- End of forwarded message --------------------\n")))
+
+(defun message-forward-make-body-mime (forward-buffer)
+  (insert "\n\n<#part type=message/rfc822 disposition=inline raw=t>\n")
+  (let ((b (point)) e)
+    (save-restriction
+      (narrow-to-region (point) (point))
+      (mml-insert-buffer forward-buffer)
+      (goto-char (point-min))
+      (when (looking-at "From ")
+	(replace-match "X-From-Line: "))
+      (goto-char (point-max)))
+    (setq e (point))
+    (insert "<#/part>\n")))
+
+(defun message-forward-make-body-mml (forward-buffer)
 	    (insert "\n\n<#mml type=message/rfc822 disposition=inline>\n")
-	  (insert "\n\n<#part type=message/rfc822 disposition=inline raw=t>\n")))
-    (insert "\n-------------------- Start of forwarded message --------------------\n"))
   (let ((b (point)) e)
-    (if digest
-	(if message-forward-as-mime
-	    (insert-buffer-substring forward-buffer)
-	  (mml-insert-buffer forward-buffer))
-      (if (and message-forward-show-mml
-	       (not message-forward-decoded-p))
+    (if (not message-forward-decoded-p)
 	  (insert
 	   (with-temp-buffer
 	     (mm-disable-multibyte)
@@ -6016,32 +6027,59 @@ Optional DIGEST will use digest to forwa
 	  (goto-char (point-min))
 	  (when (looking-at "From ")
 	    (replace-match "X-From-Line: "))
-	  (goto-char (point-max)))))
+	(goto-char (point-max))))
     (setq e (point))
-    (if message-forward-as-mime
-	(if digest
-	    (insert "<#/multipart>\n")
-	  (if message-forward-show-mml
 	      (insert "<#/mml>\n")
-	    (insert "<#/part>\n")))
-      (insert "\n-------------------- End of forwarded message --------------------\n"))
-    (if (and digest message-forward-as-mime)
+    (when (and (not current-prefix-arg)
+	       message-forward-ignored-headers)
 	(save-restriction
 	  (narrow-to-region b e)
 	  (goto-char b)
 	  (narrow-to-region (point)
 			    (or (search-forward "\n\n" nil t) (point)))
-	  (delete-region (point-min) (point-max)))
-      (when (and (not current-prefix-arg)
-		 message-forward-ignored-headers
-		 ;; don't remove CTE, X-Gnus etc when doing "raw" forward:
-		 message-forward-show-mml)
+	(message-remove-header message-forward-ignored-headers t)))))
+
+(defun message-forward-make-body-digest-plain (forward-buffer)
+  (insert
+   "\n-------------------- Start of forwarded message --------------------\n")
+  (let ((b (point)) e)
+    (mml-insert-buffer forward-buffer)
+    (setq e (point))
+    (insert
+     "\n-------------------- End of forwarded message --------------------\n")))
+
+(defun message-forward-make-body-digest-mime (forward-buffer)
+  (insert "\n<#multipart type=digest>\n")
+  (let ((b (point)) e)
+    (insert-buffer-substring forward-buffer)
+    (setq e (point))
+    (insert "<#/multipart>\n")
 	(save-restriction
 	  (narrow-to-region b e)
 	  (goto-char b)
 	  (narrow-to-region (point)
 			    (or (search-forward "\n\n" nil t) (point)))
-	  (message-remove-header message-forward-ignored-headers t)))))
+      (delete-region (point-min) (point-max)))))
+
+(defun message-forward-make-body-digest (forward-buffer)
+  (if message-forward-as-mime
+      (message-forward-make-body-digest-mime forward-buffer)
+    (message-forward-make-body-digest-plain forward-buffer)))
+
+;;;###autoload
+(defun message-forward-make-body (forward-buffer &optional digest)
+  ;; Put point where we want it before inserting the forwarded
+  ;; message.
+  (if message-forward-before-signature
+      (message-goto-body)
+    (goto-char (point-max)))
+  (if message-forward-as-mime
+      (if digest
+	  (message-forward-make-body-digest forward-buffer)
+	(if message-forward-show-mml
+	    (message-forward-make-body-mml forward-buffer)
+	  (message-forward-make-body-mime forward-buffer)))
+    (message-forward-make-body-plain forward-buffer))
   (message-position-point))
 
 ;;;###autoload

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> How about this?  It doesn't contain anything regarding
> multipart/{signed,encrypted}, but that could be added once this has
> been added.

Looks good to me; please check in.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> How about this?  It doesn't contain anything regarding
>> multipart/{signed,encrypted}, but that could be added once this has
>> been added.
>
> Looks good to me; please check in.  :-)

Done.

So if forwarding messages stops working for anyone, please holler.

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

I discovered a bug related to forwarding as well: if you forward a
message as MML that contain MML tags in some text part, those MML tags
are not quoted, so they will be interpreted again when you send the
message.  I don't know how to fix it, or if it is important.

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> I discovered a bug related to forwarding as well: if you forward a
> message as MML that contain MML tags in some text part, those MML tags
> are not quoted, so they will be interpreted again when you send the
> message.  I don't know how to fix it, or if it is important.

`mml-insert-buffer' calls `mml-quote-region', which should quote any
MML tags in the text inserted...

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> I discovered a bug related to forwarding as well: if you forward a
>> message as MML that contain MML tags in some text part, those MML tags
>> are not quoted, so they will be interpreted again when you send the
>> message.  I don't know how to fix it, or if it is important.
>
> `mml-insert-buffer' calls `mml-quote-region', which should quote any
> MML tags in the text inserted...

Doesn't seem to work here, try C-c C-f on
<iluad7znx2w.fsf@extundo.com> in this thread and notice that the MML
tags inside the patched code aren't escaped.

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> Doesn't seem to work here, try C-c C-f on
> <iluad7znx2w.fsf@extundo.com> in this thread and notice that the MML
> tags inside the patched code aren't escaped.

Hm...  it worked for me.  The part tags became (er, how to put this):

<HASH-SIGN EXCLAMATION-POINT part type=message/rfc822 disposition=inline raw=t>

and that second character is the quoting...

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> Doesn't seem to work here, try C-c C-f on
>> <iluad7znx2w.fsf@extundo.com> in this thread and notice that the MML
>> tags inside the patched code aren't escaped.
>
> Hm...  it worked for me.  The part tags became (er, how to put this):
>
> <HASH-SIGN EXCLAMATION-POINT part type=message/rfc822 disposition=inline raw=t>
>
> and that second character is the quoting...

Also with the (new) default value for message-forward-show-mml?

Re: iso-8859-2 characters are broken in forwarded messages.

[Lars Magne Ingebrigtsen]

Simon Josefsson <jas@extundo.com> writes:

> Also with the (new) default value for message-forward-show-mml?

Hm...  no.  I've now fixed that.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: iso-8859-2 characters are broken in forwarded messages.

[Romain FRANCOISE]

Simon Josefsson <jas@extundo.com> writes:

> So if forwarding messages stops working for anyone, please holler.

There is a small logic error: it does both m-f-make-body-mml and
m-f-make-body-plain, resulting in a strange message buffer.  The
following patch fixes things for me.

(I hand-edited this patch because I have other uncommitted changes in
message.el, it may not work.)

-- 
Romain FRANCOISE <romain@orebokech.com> | They're nothing but scared
it's a miracle -- http://orebokech.com/ | little mice.

Index: message.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/message.el,v
retrieving revision 6.370
diff -u -r6.370 message.el
--- message.el	18 Oct 2003 03:27:46 -0000	6.370
+++ message.el	18 Oct 2003 09:54:22 -0000
@@ -6091,8 +6090,8 @@
 			      "Content-Type: *multipart/\\(signed\\|encrypted\\)"
 			      nil t)))))
 	    (message-forward-make-body-mml forward-buffer)
-	  (message-forward-make-body-mime forward-buffer)))
-    (message-forward-make-body-plain forward-buffer))
+	  (message-forward-make-body-mime forward-buffer))
+      (message-forward-make-body-plain forward-buffer)))
   (message-position-point))
 
 ;;;###autoload

Re: iso-8859-2 characters are broken in forwarded messages.

[Simon Josefsson]

I have committed the patch below, and updated the GNUS-NEWS as
follows:

** The default for message-forward-show-mml changed to symbol best.

The behaviour for the `best' value is to show MML (i.e., convert MIME
to MML) when appropriate.  MML will not be used when forwarding signed
or encrypted messages, as the conversion invalidate the digital
signature.

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

	* message.el (message-forward-show-mml): New default 'best.
	(message-forward-make-body): Support it.

Index: message.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/message.el,v
retrieving revision 6.368
diff -u -p -r6.368 message.el
--- message.el	18 Oct 2003 01:51:50 -0000	6.368
+++ message.el	18 Oct 2003 02:27:48 -0000
@@ -467,12 +467,19 @@ Otherwise, directly inline the old messa
   :group 'message-forwarding
   :type 'boolean)
 
-(defcustom message-forward-show-mml nil
-  "*Non-nil means show forwarded messages as mml.
-Otherwise, forwarded messages are unchanged."
+(defcustom message-forward-show-mml 'best
+  "*Non-nil means show forwarded messages as MML (decoded from MIME).
+Otherwise, forwarded messages are unchanged.
+Can also be the symbol `best' to indicate that MML should be
+used, except when it is a bad idea to use MML.  One example where
+it is a bad idea is when forwarding a signed or encrypted
+message, because converting MIME to MML would invalidate the
+digital signature."
   :version "21.1"
   :group 'message-forwarding
-  :type 'boolean)
+  :type '(choice (const :tag "use MML" t)
+		 (const :tag "don't use MML " nil)
+		 (const :tag "use MML when appropriate" best)))
 
 (defcustom message-forward-before-signature t
   "*Non-nil means put forwarded message before signature, else after."
@@ -6076,7 +6083,13 @@ Optional DIGEST will use digest to forwa
   (if message-forward-as-mime
       (if digest
 	  (message-forward-make-body-digest forward-buffer)
-	(if message-forward-show-mml
+	(if (and message-forward-show-mml
+		 (not (and (eq message-forward-show-mml 'best)
+			   (with-current-buffer forward-buffer
+			     (goto-char (point-min))
+			     (re-search-forward
+			      "Content-Type: *multipart/\\(signed\\|encrypted\\)"
+			      nil t)))))
 	    (message-forward-make-body-mml forward-buffer)
 	  (message-forward-make-body-mime forward-buffer)))
     (message-forward-make-body-plain forward-buffer))