Fix #10172 Emails don't show subject MIME headers #10269 Email sync …

chris001 commented 8 months ago

…breaking on MIME decoding.
A commonly reported bug report on the forum: https://community.suitecrm.com/t/subject-line-missing-from-inbound-emails/88059 Also fixes this issue https://github.com/salesagility/SuiteCRM-Core/issues/363 ..and fixes https://github.com/salesagility/SuiteCRM-Core/issues/420 ..all similar reports about the same bug - a lack of resilience in decoding the MIME headers, resulting in broken inbox emails. Credit also to @DBRenny for putting together a first version of this: https://github.com/salesagility/SuiteCRM-Core/pull/366

Description

Motivation and Context

Inbound email was broken on Bitnami containers, and on any installation that is missing the php-imap extension. Bitnami intentionally leaves out the php-imap extension, because of the long time since it last received security fixes.

How To Test This

Try inbound email on a bitnami container. There should be no subjects, for servers that return MIME encoded headers (languages with accented characters outside of US-ASCII character set, e.g. French, German, etc). Then from inside the container, get the patch with wget and apply the patch git apply, try inbound email again, and it should show email Subjects in the inbox. Other MIME encoded headers may also display properly as UTF-8, such as From, To, CC, Bcc.

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

[x] My code follows the code style of this project found here.
[ ] My change requires a change to the documentation.
[x] I have read the How to Contribute guidelines.

SuiteBot commented 8 months ago

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/subject-line-missing-from-inbound-emails/88059/27

Porhoken commented 6 months ago

Hello Chris, thank you for your efforts so far. Can you tell me if this fix is for a specific version? I tried to apply this patch on my bitnami/suitecrm Docker Container (Suite CRM Version 8.5.1) but this breaks my menus completely. All menu entries vanished, only the home button remains. The profilebutton leaves me with 'logout' as only option. I checked with an administrator user. Have you tried this patch in version 8.x? I installed the german translation also, but this should not break the menus in conjunction with your fix. But then I thought the same of the E-mail decoding module... ;) I'd like to help here, if you can point me where to look at.

Best Ken

UPDATE: I tried to find some related messages in the error-log, but couldn't find anything. Then I tried switching the language, as this is the problem here in the first place, but I also logged in to no menus, not even Icons. I looked in the error-log again, but this time, I found a lot of messages that are def. related:

" Fri Mar 22 07:26:36 2024 [689][1][FATAL] Bean file does not exist in path: modules/InboundEmail/InboundEmail.php

Fri Mar 22 07:27:59 2024 [840][1][WARN] ModuleNameMapper | mapName | 'Feeds' not mapped to 'frontend' Fri Mar 22 07:27:59 2024 [840][1][WARN] ModuleNameMapper | mapName | 'iFrames' not mapped to 'frontend' .... Fri Mar 22 07:27:59 2024 [840][1][WARN] Cannot find bean file for module: CustomFields Fri Mar 22 07:27:59 2024 [840][1][WARN] Cannot find bean file for module: CustomFields Fri Mar 22 07:27:59 2024 [840][1][WARN] Cannot find bean file for module: Versions Fri Mar 22 07:27:59 2024 [840][1][WARN] Cannot find bean file for module: Versions Fri Mar 22 07:27:59 2024 [840][1][WARN] SugarBean constructor error: Object has not fields in dictionary. Object name was: Audit ... Fri Mar 22 07:28:08 2024 [525][1][WARN] Undefined data index ID for list view data.

Those look as if they have something to do with the menus not generated.

Best Ken

chris001 commented 6 months ago

You'd need either the php-imap or the php-iconv module loaded, as this calls a function in one of those modules to decode the MIME to UTF-8.

Porhoken commented 6 months ago

Oh, I thought the bitnami container has the php-iconv module activated out of the box. I will check that.

Porhoken commented 6 months ago

So i checked my bitnami container. The iconv module is loaded, version 8.1.27. This happens to be the php version in the latest bitnami suitecrm image. I will try your patch today in a suitecrm v 7.14.3 container. See how that behaves.

Porhoken commented 6 months ago

Same behavior with the suitecrm 7.14.3 image. Applying the patch breaks the inbound email view completely (blank screen). Patch is applied manually (copy paste to php file, no git diff). But I can not get a sensible error message in the log, at what point the whole thing breaks.

EDIT: Applied the patch via git apply. Got an error message about file permissions (has type 100644, expected 100755) Patch was applied, same error. Blank screen on entering inbound email menu. For now, I'm lost.

Porhoken commented 6 months ago

SOLVED: Looking in the actual php and mysql error logs, showed

"syntax error, unexpected identifier "function_exists", expecting "(" in bitnami/suitecrm/modules/InboundEmail/InboundEmail.php on line 4869"

which led me to missing brackets around the second "function_exists" call. I added brackets like this (function_exists()) and everthing works. These brackets exist in the first call of this function in line 4865.

I hope this helps someone, sorry for flooding the comment section.

chris001 commented 6 months ago

Fixed. Thanks!

jack7anderson7 commented 5 months ago

Hi @chris001

Thank you for your PR.

We have been analysing and testing this PR. If we are not mistaken this happens when the imap php module is not installed. Do you agree?

The reason we ask is due to other changes that would need to be made if we were to support not having the imap module.

Thanks, Jack

chris001 commented 5 months ago

Yes, and this is why bitnami (owned by VMware), maintainer of the Suite docker containers, removed it:

I'm sorry but we recently removed support for the php-imap module in our images due to the fact that it was not receiving any updates at all. Its latest release dates from 2011, and being this remarkably outdated has a direct impact in the security of the images.

how to install the php-imap package?

Unfortunately, this can't be achieved as PHP itself needs to be compiled together with support for this module.

Anyone who needs a high security Suite installation may also opt to avoid php-imap.

Alternatives:

This PR - uses php-iconv.
PHP-Mailparse which requires php-mbstring.
mail-mime-parser, already in composer.json.

pgorod commented 5 months ago

Google just told me there's also this one

Docs: https://www.php-imap.com/ Github: webklex/php-imap

How extensive is our dependency on the old php-imap? Which functions does it provide, how can we search our codebase for those dependencies?

jack7anderson7 commented 5 months ago

Hi @chris001 @pgorod

Thank you for the suggestions.

@chris001 great, that matches what we were expecting. Now, in order to not rely on php imap module there are some more changes that need to be done.

Both the changes on this PR and the mentioned changes should be done on the Imap2Handler.

We are going to move this PR into test and we will try to bring it into the codebase but in the future we will have to move the code from this PR to Imap2Handler.

Regarding the suggested libs, and the way to implement these changes, the Imap2 lib is a wrapper around roundcube which is a well-known and battle-tested email client. Therefore, we are thinking of using what we can from the roundcube lib.

Any thoughts or suggestions on using the roundcube lib are welcome.

Thanks, Jack

chris001 commented 5 months ago

Here's some thoughts from another open source PHP application that faced the same challenge, they moved first to the roundcube lib, then to curl. The PHP code using these alternative methods, is shown here.

johnM2401 commented 4 months ago

Hey @chris001 ! Thank you for your PR !

During testing and discussions with @clemente-raposo, we've noticed that imap_mime_header_decode can sometimes still be found, even when php-imap is disabled.

This results in the records rendering with encoded values, such as:

We've tried making the following amendment to your first if statement:

        if (function_exists('imap_mime_header_decode') && in_array('imap', get_loaded_extensions(), true)) {   // function_exists() should be moved to MimeHeaderDecode().
            $subjectDecoded = $this->getImap()->MimeHeaderDecode($subject); // returns array or string.
        } else {

(ie: adding: && in_array('imap', get_loaded_extensions(), true) )

Which appears to work in all tested scenarios

For example:

Would you like us to push this change, or would you like to make this change yourself?

Please let us know how you would like to proceed?

Thanks!

chris001 commented 4 months ago

Thanks @johnM2401 @clemente-raposo @jack7anderson7 for your amendment! I've updated with this change!

SuiteBot commented 4 months ago

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/subject-line-missing-from-inbound-emails/88059/31

SuiteBot commented 4 months ago

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/imap-email-import-from-outlook-does-not-populate-the-to-from-subject/92925/6

salesagility / SuiteCRM