Change how charsets are detected for emails #5882

miaulalala · 2021-12-20T15:19:12Z

lib/Model/IMAPMessage.php

GretaD · 2021-12-22T17:39:30Z

this looks good for me too

st3iny

Throws an error Could not detect charset for message on a harmless message that worked fined before.

Test email as sent from Thunderbird. It contains an image from Unsplash. I could reproduce it using other images too so this seems to be an issue with attached images.

Error from the console

{
  "app": "mail",
  "uid": "admin",
  "error": {
    "isError": true,
    "debug": true,
    "type": "OCA\\Mail\\Exception\\ServiceException",
    "code": 0,
    "message": "Could not detect charset for message",
    "trace": [
      {
        "file": "/srv/http/dev_apps/mail/lib/Model/IMAPMessage.php",
        "line": 566,
        "function": "loadBodyData",
        "class": "OCA\\Mail\\Model\\IMAPMessage"
      },
      {
        "file": "/srv/http/dev_apps/mail/lib/Model/IMAPMessage.php",
        "line": 437,
        "function": "handleTextMessage",
        "class": "OCA\\Mail\\Model\\IMAPMessage"
      },
      {
        "file": "/srv/http/dev_apps/mail/lib/Model/IMAPMessage.php",
        "line": 377,
        "function": "getPart",
        "class": "OCA\\Mail\\Model\\IMAPMessage"
      },
      {
        "file": "/srv/http/dev_apps/mail/lib/Model/IMAPMessage.php",
        "line": 102,
        "function": "loadMessageBodies",
        "class": "OCA\\Mail\\Model\\IMAPMessage"
      },
      {
        "file": "/srv/http/dev_apps/mail/lib/IMAP/MessageMapper.php",
        "line": 242,
        "function": "__construct",
        "class": "OCA\\Mail\\Model\\IMAPMessage"
      },
      {
        "function": "OCA\\Mail\\IMAP\\{closure}",
        "class": "OCA\\Mail\\IMAP\\MessageMapper"
      },
      {
        "file": "/srv/http/dev_apps/mail/lib/IMAP/MessageMapper.php",
        "line": 256,
        "function": "array_map"
      },
      {
        "file": "/srv/http/dev_apps/mail/lib/IMAP/MessageMapper.php",
        "line": 69,
        "function": "findByIds",
        "class": "OCA\\Mail\\IMAP\\MessageMapper"
      },
      {
        "file": "/srv/http/dev_apps/mail/lib/Service/MailManager.php",
        "line": 180,
        "function": "find",
        "class": "OCA\\Mail\\IMAP\\MessageMapper"
      },
      {
        "file": "/srv/http/dev_apps/mail/lib/Controller/MessagesController.php",
        "line": 249,
        "function": "getImapMessage",
        "class": "OCA\\Mail\\Service\\MailManager"
      },
      {
        "file": "/srv/http/lib/private/AppFramework/Http/Dispatcher.php",
        "line": 217,
        "function": "getBody",
        "class": "OCA\\Mail\\Controller\\MessagesController"
      },
      {
        "file": "/srv/http/lib/private/AppFramework/Http/Dispatcher.php",
        "line": 126,
        "function": "executeController",
        "class": "OC\\AppFramework\\Http\\Dispatcher"
      },
      {
        "file": "/srv/http/lib/private/AppFramework/App.php",
        "line": 157,
        "function": "dispatch",
        "class": "OC\\AppFramework\\Http\\Dispatcher"
      },
      {
        "file": "/srv/http/lib/private/Route/Router.php",
        "line": 302,
        "function": "main",
        "class": "OC\\AppFramework\\App"
      },
      {
        "file": "/srv/http/lib/base.php",
        "line": 1006,
        "function": "match",
        "class": "OC\\Route\\Router"
      },
      {
        "file": "/srv/http/index.php",
        "line": 36,
        "function": "handleRequest",
        "class": "OC"
      }
    ]
  }
}

miaulalala · 2022-01-17T12:47:54Z

Throws an error Could not detect charset for message on a harmless message that worked fined before.

Test email as sent from Thunderbird. It contains an image from Unsplash. I could reproduce it using other images too so this seems to be an issue with attached images.

Error from the console

Ah interesting. Attached, not embedded as base64?

st3iny · 2022-01-18T08:37:21Z

Throws an error Could not detect charset for message on a harmless message that worked fined before.
Test email as sent from Thunderbird. It contains an image from Unsplash. I could reproduce it using other images too so this seems to be an issue with attached images.
Error from the console

Ah interesting. Attached, not embedded as base64?

I added the image as a regular attachment. At least in Thunderbird it is detected as an attachment and is not shown inline.

Here is the relevant excerpt from the .eml:

--------------LrBOmGA0xhgKeGGopkJKdw1w
Content-Type: image/jpeg; name="ilya-bronskiy-__Z8VvhbIZ4-unsplash.jpg"
Content-Disposition: attachment;
 filename="ilya-bronskiy-__Z8VvhbIZ4-unsplash.jpg"
Content-Transfer-Encoding: base64

miaulalala · 2022-03-14T07:12:04Z

#1037 is also related

miaulalala · 2022-03-18T00:08:07Z

Throws an error Could not detect charset for message on a harmless message that worked fined before.
Test email as sent from Thunderbird. It contains an image from Unsplash. I could reproduce it using other images too so this seems to be an issue with attached images.
Error from the console

Ah interesting. Attached, not embedded as base64?

I added the image as a regular attachment. At least in Thunderbird it is detected as an attachment and is not shown inline.

Here is the relevant excerpt from the .eml:
--------------LrBOmGA0xhgKeGGopkJKdw1w
Content-Type: image/jpeg; name="ilya-bronskiy-__Z8VvhbIZ4-unsplash.jpg"
Content-Disposition: attachment;
 filename="ilya-bronskiy-__Z8VvhbIZ4-unsplash.jpg"
Content-Transfer-Encoding: base64

Figured it out. PHP implicit bools were fooling me. As the content is $data = "" - i. e. an empty string, my boolean check on !$data returned a false value even though $data is correct as "". Early returns FTW here.

roukmoute · 2022-07-21T12:11:46Z

Would it be possible to use symfony/unicode or hoa/ustring to avoid encoding problems?

ChristophWurst

This is a bit scary. Let's add tests to cover the edge cases.

👍 otherwise

miaulalala · 2023-10-09T08:31:29Z

This is a bit scary. Let's add tests to cover the edge cases.

👍 otherwise

Not quite sure how to test this as the complete function chain is private.

ChristophWurst · 2023-10-09T10:51:35Z

Not quite sure how to test

Move the charset logic into a new class, add unit tests for it and use it in the other complex class.

tests/Unit/IMAP/Charset/ConverterTest.php

ChristophWurst · 2023-10-10T17:09:32Z

lib/IMAP/Charset/Converter.php

+
+		// Still UTF8, no need to convert
+		if($charset !== null && strtoupper($charset) === 'UTF-8') {
+			return $data;


none of the tests is able to reach a line below this

this is still the case. either the charset parameter is set and used, or mb detects the string as UTF-8

lib/IMAP/Charset/Converter.php

tests/Unit/IMAP/Charset/ConverterTest.php

ChristophWurst · 2023-12-06T17:49:37Z

tests/Unit/IMAP/Charset/ConverterTest.php

+		$iso2022jpMimePart_noCharset = new Horde_Mime_Part();
+		$iso2022jpMimePart_noCharset->setContents('外せ園査リツハワ題');
+		// Korean - not in mb nor iconv
+		// $iso106461MimePart = new Horde_Mime_Part();


ISO 10646-1 is impossible to get working for me. The coding is not mentioned in any report. Can we skip this?

ChristophWurst

👍 we can get this in

lib/IMAP/Charset/Converter.php

Signed-off-by: Anna Larch <[email protected]>

miaulalala added bug 2. developing labels Dec 20, 2021

miaulalala self-assigned this Dec 20, 2021

ChristophWurst reviewed Dec 20, 2021

View reviewed changes

lib/Model/IMAPMessage.php Outdated Show resolved Hide resolved

miaulalala changed the title ~~Checnge how charsets are detected for emails~~ Change how charsets are detected for emails Dec 20, 2021

miaulalala requested a review from ChristophWurst December 22, 2021 16:30

miaulalala mentioned this pull request Dec 22, 2021

Hebrew (all RTL?) Language Error #5834

Closed

miaulalala marked this pull request as ready for review January 3, 2022 11:58

miaulalala added 3. to review and removed 2. developing labels Jan 3, 2022

This comment has been minimized.

Sign in to view

miaulalala force-pushed the fix/detect-char-encoding branch from 4bda549 to fac906a Compare January 11, 2022 16:42

miaulalala requested review from GretaD and st3iny January 11, 2022 18:35

This comment has been minimized.

Sign in to view

st3iny reviewed Jan 14, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

adarnimrod mentioned this pull request Mar 3, 2022

mb_convert_encoding does not support windows-1250. #1037

Open

miaulalala requested a review from st3iny March 18, 2022 00:19

miaulalala added this to the v1.11.8 milestone Mar 18, 2022

ChristophWurst removed this from the v1.11.8 milestone Apr 27, 2022

ChristophWurst reviewed Sep 29, 2023

View reviewed changes

miaulalala force-pushed the fix/detect-char-encoding branch from 368320d to b557369 Compare October 9, 2023 08:52

ChristophWurst reviewed Oct 10, 2023

View reviewed changes

tests/Unit/IMAP/Charset/ConverterTest.php Show resolved Hide resolved

ChristophWurst reviewed Oct 10, 2023

View reviewed changes

ChristophWurst mentioned this pull request Nov 29, 2023

Christoph's advent of code #9105

Closed

24 tasks

ChristophWurst reviewed Dec 6, 2023

View reviewed changes

tests/Unit/IMAP/Charset/ConverterTest.php Show resolved Hide resolved

ChristophWurst reviewed Dec 6, 2023

View reviewed changes

lib/IMAP/Charset/Converter.php Outdated Show resolved Hide resolved

fix(encoding): better character encoding

176e074

Signed-off-by: Anna Larch <[email protected]>

ChristophWurst force-pushed the fix/detect-char-encoding branch from 0cbdb53 to 176e074 Compare February 27, 2024 18:32

ChristophWurst approved these changes Feb 27, 2024

View reviewed changes

ChristophWurst enabled auto-merge February 27, 2024 18:32

ChristophWurst merged commit ce0e4bd into main Feb 28, 2024
37 checks passed

ChristophWurst deleted the fix/detect-char-encoding branch February 28, 2024 00:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change how charsets are detected for emails #5882

Change how charsets are detected for emails #5882

miaulalala commented Dec 20, 2021

GretaD commented Dec 22, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

st3iny left a comment •

edited

Loading

This comment has been minimized.

miaulalala commented Jan 17, 2022

st3iny commented Jan 18, 2022 •

edited

Loading

miaulalala commented Mar 14, 2022

miaulalala commented Mar 18, 2022

roukmoute commented Jul 21, 2022

ChristophWurst left a comment

miaulalala commented Oct 9, 2023

ChristophWurst commented Oct 9, 2023

ChristophWurst Oct 10, 2023 •

edited

Loading

ChristophWurst Dec 6, 2023

ChristophWurst Dec 6, 2023

ChristophWurst left a comment

Change how charsets are detected for emails #5882

Change how charsets are detected for emails #5882

Conversation

miaulalala commented Dec 20, 2021

GretaD commented Dec 22, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

st3iny left a comment • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

miaulalala commented Jan 17, 2022

st3iny commented Jan 18, 2022 • edited Loading

miaulalala commented Mar 14, 2022

miaulalala commented Mar 18, 2022

roukmoute commented Jul 21, 2022

ChristophWurst left a comment

Choose a reason for hiding this comment

miaulalala commented Oct 9, 2023

ChristophWurst commented Oct 9, 2023

ChristophWurst Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

ChristophWurst Dec 6, 2023

Choose a reason for hiding this comment

ChristophWurst Dec 6, 2023

Choose a reason for hiding this comment

ChristophWurst left a comment

Choose a reason for hiding this comment

st3iny left a comment •

edited

Loading

st3iny commented Jan 18, 2022 •

edited

Loading

ChristophWurst Oct 10, 2023 •

edited

Loading