Bug 21874 - Encoding broken in list and cart email subjects
Summary: Encoding broken in list and cart email subjects
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Lists (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Martin Renvoize
URL:
Keywords: rel_18_11_candidate
Depends on:
Blocks:
 
Reported: 2018-11-22 14:43 UTC by Katrin Fischer
Modified: 2019-01-02 09:39 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21874: Fix encoding of cart and list email subjects (3.19 KB, patch)
2018-11-23 10:41 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21874: Fix encoding of cart and list email subjects (3.20 KB, patch)
2018-11-23 10:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21874: Fix encoding of cart and list email subjects (3.27 KB, patch)
2018-11-23 11:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21874: (QA follow-up) Fix intranet sendshelf (1.06 KB, patch)
2018-11-23 11:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21874: (QA follow-up) Fix intranet sendshelf (1.06 KB, patch)
2018-11-23 11:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21874: (QA follow-up) Fix intranet sendshelf (1.09 KB, patch)
2018-11-23 11:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21874: Fix encoding of cart and list email subjects (3.50 KB, patch)
2018-11-23 11:24 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21874: (QA follow-up) Fix intranet sendshelf (1.09 KB, patch)
2018-11-23 11:24 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21874: Fix encoding of cart and list email subjects (3.55 KB, patch)
2018-11-23 11:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21874: (QA follow-up) Fix intranet sendshelf (1.12 KB, patch)
2018-11-23 11:35 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2018-11-22 14:43:43 UTC
To test:
- Create a list in staff or OPAC with the title "Jugendbücher" (Juvenile books)
- Send yourself the list by email
- Verify that the subject is broken "Ihre Liste: Jugendbücher"

As this is from the OPAC, it's quite an issue for libraries using languages with umlauts and other diacritics.

I haven't been able to verify in master yet (mine can't send emails and bywater sandboxes gave me an error?), but verified it still happens in our 17.11 installations. It was inititally reported for 3.22, but got lost in a comment on bug 13245.
Comment 1 Marcel de Rooy 2018-11-22 14:51:09 UTC
Will have a look tomorrow..
Comment 2 Katrin Fischer 2018-11-22 15:32:03 UTC
That would be great, thx Marcel!
Comment 3 Marcel de Rooy 2018-11-23 06:43:09 UTC
(In reply to Katrin Fischer from comment #0)
> I haven't been able to verify in master yet (mine can't send emails and
> bywater sandboxes gave me an error?), but verified it still happens in our
> 17.11 installations. It was inititally reported for 3.22, but got lost in a
> comment on bug 13245.

13245 is about sending the cart, not about lists
Comment 4 Marcel de Rooy 2018-11-23 06:44:30 UTC
I just tested a list with Jügend and sending it worked fine with 16.11 and latest master. Still looking a bit further..
Comment 5 Katrin Fischer 2018-11-23 06:52:04 UTC
(In reply to Marcel de Rooy from comment #4)
> I just tested a list with Jügend and sending it worked fine with 16.11 and
> latest master. Still looking a bit further..

On the other bug it was also reported working by someone. I got no idea what the difference could be. Our installations are packages with Plack and memcached, no changes in the list related code. Thx for investigating!
Comment 6 Marcel de Rooy 2018-11-23 06:55:02 UTC
Non-latin works fine like perl Jügend 遺跡発見 áéíóú 遱葑癱覂.
Comment 7 Katrin Fischer 2018-11-23 07:06:07 UTC
Running some additional tests it looks like not all email clients show the display issue. It appears nicely in Thunderbird and another web based GUI, but broken in Outlook.
Comment 8 Katrin Fischer 2018-11-23 07:07:07 UTC
Hm, maybe another clue: Umlauts in the DUE and DUEDGST notices work ok for us in all email clients. They translate to: Fälligkeitsbenachrichtigung and we never had any issues there.
Comment 9 Katrin Fischer 2018-11-23 07:33:57 UTC
Trying to pinpoint the difference. The queue emails have this:

1286     my $utf8   = decode('MIME-Header', $message->{'subject'} );
1287     $message->{subject}= encode('MIME-Header', $utf8);
1288     my $subject = encode('UTF-8', $message->{'subject'});

The shelf has:

 114     # Analysing information and getting mail properties
 115     if ( $template_res =~ /<SUBJECT>(.*)<END_SUBJECT>/s ) {
 116         $mail{'subject'} = Encode::encode("UTF-8", $1);
 117         $mail{subject} =~ s|\n?(.*)\n?|$1|;


Wonder if the encode('MIME-Header'... is the magical difference?
Comment 10 Marcel de Rooy 2018-11-23 08:37:42 UTC
(In reply to Katrin Fischer from comment #9)
> Trying to pinpoint the difference. The queue emails have this:
> 
> 1286     my $utf8   = decode('MIME-Header', $message->{'subject'} );
> 1287     $message->{subject}= encode('MIME-Header', $utf8);
> 1288     my $subject = encode('UTF-8', $message->{'subject'});
> 
> The shelf has:
> 
>  114     # Analysing information and getting mail properties
>  115     if ( $template_res =~ /<SUBJECT>(.*)<END_SUBJECT>/s ) {
>  116         $mail{'subject'} = Encode::encode("UTF-8", $1);
>  117         $mail{subject} =~ s|\n?(.*)\n?|$1|;
> 
> 
> Wonder if the encode('MIME-Header'... is the magical difference?

This is very nasty code. Minefield (mime..mine :)
Lines 1286 and 1287 are obscure. An undocumented tric to decode/encode. Weird that it works. My first thought would be: $message->subject is from the database and is in perl internal format, not in MIME encoding. The decode does not result in utf8 so the var name is a bad choice(!). Should not do very much? The line 1287 encodes to MIME. Line 1288 is weird after encoding to MIME.

Using still Mail::Sendmail with all its limitations in encoding etc. is another pitfall.

So we should not be too hasty here. But all this encoding/decoding before calling sendmail should be moved away to one single testable module and no leftovers in several scripts.
Comment 11 Martin Renvoize 2018-11-23 10:17:03 UTC
For reference, there was some great discussion to narrow this one down on irc: http://irc.koha-community.org/koha/2018-11-23#i_2099148
Comment 12 Marcel de Rooy 2018-11-23 10:30:41 UTC
Just a highlight:

Your list :  =?UTF-8?B?IHBlcmwgSsO8Z2VuZCDpgbrot6Hnmbroposgw6HDqcOtw7PDuiDpgbHokZHnmbE=?=  =?UTF-8?B?6KaC?=

perl Jügend 遺跡発見 áéíóú 遱葑癱覂
Comment 13 Katrin Fischer 2018-11-23 10:41:54 UTC
Created attachment 82603 [details] [review]
Bug 21874: Fix encoding of cart and list email subjects
Comment 14 Katrin Fischer 2018-11-23 10:51:40 UTC
Created attachment 82604 [details] [review]
Bug 21874: Fix encoding of cart and list email subjects
Comment 15 Marcel de Rooy 2018-11-23 11:04:41 UTC Comment hidden (obsolete)
Comment 16 Marcel de Rooy 2018-11-23 11:04:53 UTC Comment hidden (obsolete)
Comment 17 Marcel de Rooy 2018-11-23 11:08:52 UTC Comment hidden (obsolete)
Comment 18 Marcel de Rooy 2018-11-23 11:10:10 UTC Comment hidden (obsolete)
Comment 19 Marcel de Rooy 2018-11-23 11:24:29 UTC
Created attachment 82609 [details] [review]
Bug 21874: Fix encoding of cart and list email subjects

The subject of the cart and list emails is not correctly
encoded and displays incorrectly in some email clients.

To test:

Lists:
- Create a list, name it using umlauts or other diacritics
  Example:  Jugendbücher (books for youths in German)
- Add some items to your list
- Email yourself the list
- Verify that the email subject is broken

Cart:
- Install another language with non-latin characters like Greek
- Fill the cart with some titles
- Send yourself the cart
- Verify that the email subject is broken

Note: Some email clients display correctly, others not.
      It's known to be incorrect in Outlook and web.de,
      displaying nicely in Thunderbird.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested all four scripts by manipulating the cart text too in templates.
Follow-up handles intranet sendshelf.

Note: I asked Katrin to remove the encode UTF-8 statements, since we
convert to MIME and the subject line should never be UTF-8.
Comment 20 Marcel de Rooy 2018-11-23 11:24:33 UTC
Created attachment 82610 [details] [review]
Bug 21874: (QA follow-up) Fix intranet sendshelf

Move encode line below regex line.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2018-11-23 11:28:38 UTC
(In reply to Marcel de Rooy from comment #19)
> Note: I asked Katrin to remove the encode UTF-8 statements, since we
> convert to MIME and the subject line should never be UTF-8.

Hope that was clear. No UTF-8 in the header lines.
Comment 22 Martin Renvoize 2018-11-23 11:35:23 UTC
Created attachment 82612 [details] [review]
Bug 21874: Fix encoding of cart and list email subjects

The subject of the cart and list emails is not correctly
encoded and displays incorrectly in some email clients.

To test:

Lists:
- Create a list, name it using umlauts or other diacritics
  Example:  Jugendbücher (books for youths in German)
- Add some items to your list
- Email yourself the list
- Verify that the email subject is broken

Cart:
- Install another language with non-latin characters like Greek
- Fill the cart with some titles
- Send yourself the cart
- Verify that the email subject is broken

Note: Some email clients display correctly, others not.
      It's known to be incorrect in Outlook and web.de,
      displaying nicely in Thunderbird.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested all four scripts by manipulating the cart text too in templates.
Follow-up handles intranet sendshelf.

Note: I asked Katrin to remove the encode UTF-8 statements, since we
convert to MIME and the subject line should never be UTF-8.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Martin Renvoize 2018-11-23 11:35:26 UTC
Created attachment 82613 [details] [review]
Bug 21874: (QA follow-up) Fix intranet sendshelf

Move encode line below regex line.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize 2018-11-23 11:37:00 UTC
This fix had lots of eyes on it :)

No tests required as it's all in scripts and not modules.. however, we really should look at centralising this code sooner rather than later, but that effort should not hold off this bugfix.

Passing QA.
Comment 25 Nick Clemens 2018-11-26 12:55:17 UTC
Awesome work all!

Pushed to master for 18.11
Comment 26 Jesse Maseto 2018-12-13 20:51:15 UTC
Pushed to stable for 18.05.07
Comment 27 Fridolin SOMERS 2018-12-19 12:46:31 UTC
Pushed to 17.11.x for 17.11.13