Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FILEINFO_MIME_TYPE support (fix incorrect bugfix) #623

Merged
merged 2 commits into from
Nov 26, 2021

Conversation

maeln0r
Copy link

@maeln0r maeln0r commented Nov 22, 2021

In src/PhpImap/IncomingMailAttachment.php

if ((FILEINFO_MIME == $fileinfo_const) && (false != $this->mimeType)) {
            return $this->mimeType;
}

$this->mimeType - currently never set, condition always false. Deleted as unnecessary.

Added default field definition in src/PhpImap/Mailbox.php

$attachment->mimeType = $attachment->getFileInfo(FILEINFO_MIME_TYPE);

Also i found incorrect use of mime type in src/PhpImap/IncomingMail.php -> embedImageAttachments()

$contentType = (string) $attachment->getFileInfo(FILEINFO_MIME);
$base64encoded = \base64_encode($contents);
$replacement = 'data:'.$contentType.';base64, '.$base64encoded;

For example for jpeg image FILEINFO_MIME is image/jpeg; charset=binary, and for correct base64 need only mime type (without encoding) FILEINFO_MIME_TYPE -> image/jpeg

@Sebbo94BY
Copy link
Collaborator

Can you please update your description of this pull request with a english text?

Some before and after examples would be also good.

@Sebbo94BY
Copy link
Collaborator

Thanks for the update.

If the MIM type usage in embedImageAttachments() is also incorrect implemented, wouldn't it make sense to update it with this merge request as well? Or do you think, this will break something?

@maeln0r
Copy link
Author

maeln0r commented Nov 26, 2021

Thanks for the update.

If the MIM type usage in embedImageAttachments() is also incorrect implemented, wouldn't it make sense to update it with this merge request as well? Or do you think, this will break something?

That's right, I fixed it here: c181db6

@Sebbo94BY Sebbo94BY changed the base branch from master to develop November 26, 2021 10:13
@Sebbo94BY Sebbo94BY merged commit 73f521e into barbushin:develop Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants