barbushin / php-imap

Manage mailboxes, filter/get/delete emails in PHP (supports IMAP/POP3/NNTP)
MIT License
1.65k stars 459 forks source link

[BUG] Embeded images get replaced nok #710

Open bpali opened 1 year ago

bpali commented 1 year ago

Environment (please complete the following information):

Describe the bug This bug was introduced by https://github.com/barbushin/php-imap/issues/569 When an email has several embeded images all images will get replaced with the same one and not the corresponding ones. This is because the fact that an && condition was changed into a || at line so now cid doesn't matter anymore

    if ($attachment->contentId == $cid || 'inline' == \mb_strtolower((string) $attachment->disposition)) {

see @kurznet in https://github.com/barbushin/php-imap/issues/569#issuecomment-988939759_

Steps To Reproduce Just fetch any email that has more than one embeded image

The used code:

/**
     * Embed inline image attachments as base64 to allow for
     * email html to display inline images automatically.
     */
    public function embedImageAttachments(): void
    {
        $fetchedHtml = $this->__get('textHtml');

        \preg_match_all("/\bcid:[^'\"\s]{1,256}/mi", $fetchedHtml, $matches);

        if (isset($matches[0]) && \is_array($matches[0]) && \count($matches[0])) {
            /** @var list<string> */
            $matches = $matches[0];
            $attachments = $this->getAttachments();
            foreach ($matches as $match) {
                $cid = \str_replace('cid:', '', $match);

                foreach ($attachments as $attachment) {
                    /**
                     * Inline images can contain a "Content-Disposition: inline", but only a "Content-ID" is also enough.
                     * See https://github.com/barbushin/php-imap/issues/569.
                     */
                    if ($attachment->contentId == $cid || 'inline' == \mb_strtolower((string) $attachment->disposition)) {
                        $contents = $attachment->getContents();
                        $contentType = $attachment->getFileInfo(FILEINFO_MIME_TYPE);

                        if (!\strstr($contentType, 'image')) {
                            continue;
                        } elseif (!\is_string($attachment->id)) {
                            throw new InvalidArgumentException('Argument 1 passed to '.__METHOD__.'() does not have an id specified!');
                        }

                        $base64encoded = \base64_encode($contents);
                        $replacement = 'data:'.$contentType.';base64, '.$base64encoded;

                        $this->textHtml = \str_replace($match, $replacement, $this->textHtml);

                        $this->removeAttachment($attachment->id);
                    }
                }
            }
        }
    }

Expected behavior Each different embeded images should be replaced by it's own content.

Solution Remove the || 'inline' == \mb_strtolower((string) $attachment->disposition) from the if condition and keep only

 if ($attachment->contentId == $cid)

The $attachment->disposition needs not to be checked if it's inline because images that are not embeded, but only attached, will not be replaced because their CID's are not present in the textHtml string

bpali commented 1 year ago

I've created a pull-request https://github.com/bpali/php-imap/pull/1