Merge lp:~porten-deactivatedaccount/eventum/devel into lp:eventum

[Harri Porten]

When decoding text encoded based on RFC 2047 the charset parameter was ignored completely. Consequently we found some fields in the GUI to display garbled content.

[Elan Ruusamäe (glen666)]

applied, but this Mime_Helper::fixEncoding is just mime header decode?

i.e. could be replaced just with call to:
http://php.net/manual/en/function.iconv-mime-decode.php

and, do you have test strings to be included to testsuite (the ones i didn't already cover)?

[lp:~porten-deactivatedaccount/eventum/devel]

4263. By Elan Ruusamäe on 2011-01-25

Respect charset when processing Mime_Helper::fixEncoding

merge request https://code.launchpad.net/~porten/eventum/devel/+merge/47336

[Harri Porten (porten-deactivatedaccount)]

I agree with the assessment that a standard function can be used. We just
did not dare to make bigger changes we had to fix a regression that
occurred when we converted our database to utf-8 encoding.

Unlike your tests our concrete case had 'Q' encoding and a koi8-r charset.
But not fundamentally different.

Btw: one of the iconv_mime_decode() calls in that class is guarded by
function_exists(). The iconv availability is a hard requirement these
days?

[Elan Ruusamäe (glen666)]

iconv + mbstring should be made hard requirement, currently setup allows them as soft dependencies
but i've seen a lot of places falling back to something else if ext is missing.

so that's some cleanup to do

also, the fixEncoding should no longer be used:
http://bazaar.launchpad.net/~eventum-developers/eventum/trunk/revision/4268

[Elan Ruusamäe (glen666)]

seems something got broken still

i'm having APP_CHARSET utf8, and such email header:

"Subject: =?iso-8859-15?Q?n=FC=FCd_ei_t=F6=F6ta_adminni_publish_nupp_?="

was decoded as:
"nd ei tta adminni publish nupp "

need to debug this

[Elan Ruusamäe (glen666)]

seems iconv_mime_decode corrupts data if it is not qp encoded (or already decoded)

Committed revision 4283 with fix.

[Elan Ruusamäe (glen666)]

sorry Committed revision 4282 (my branch diverted, so the revno changed)

[Harri Porten (porten-deactivatedaccount)]

On Tue, 1 Feb 2011, Elan Ruusamäe wrote:

> seems iconv_mime_decode corrupts data if it is not qp encoded (or already decoded)

That is what we recall seeing as well. Hence we had simply left the
original function intact besides our fix.

Harri.

[Elan Ruusamäe (glen666)]

Too bad you didn't bother reporting it then, would had known fixEncoding and decodeQuotedPrintable were not identical by the behaviour, i.e would had made the fix earlier.

[Harri Porten (porten-deactivatedaccount)]

On Tue, 1 Feb 2011, Elan Ruusamäe wrote:

> Too bad you didn't bother reporting it then, would had known fixEncoding
> and decodeQuotedPrintable were not identical by the behaviour, i.e would
> had made the fix earlier.

Sorry. I think I mentioned it on the #eventum IRC channel. But our memory
on this was really too vague to make it a statement.

=== modified file 'lib/eventum/class.mime_helper.php'
--- lib/eventum/class.mime_helper.php	2010-12-09 12:06:24 +0000
+++ lib/eventum/class.mime_helper.php	2011-01-25 00:12:50 +0000
@@ -151,6 +151,9 @@
                         $text = str_replace('='.$value, chr(hexdec($value)), $text);
                     break;
             }
+            if (!empty($charset)) {
+                $text = iconv($charset, APP_CHARSET, $text);
+            }
             $input = str_replace($encoded, $text, $input);
         }
         return $input;