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

[BUG] Embeded images get replaced nok #710

Open
bpali opened this issue Apr 25, 2023 · 1 comment
Open

[BUG] Embeded images get replaced nok #710

bpali opened this issue Apr 25, 2023 · 1 comment
Labels
needs investigation This will be tested / debugged or checked out.

Comments

@bpali
Copy link

bpali commented Apr 25, 2023

Environment (please complete the following information):

  • PHP IMAP version: 5.0.1
  • PHP Version: 8.0.28
  • Type of execution: CLI

Describe the bug
This bug was introduced by #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 #569 (comment)_

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 bpali added the needs investigation This will be tested / debugged or checked out. label Apr 25, 2023
@bpali bpali changed the title [BUG] A short description of what the bug is [BUG] Embeded images get replaced nok Apr 25, 2023
bpali added a commit to bpali/php-imap that referenced this issue Apr 25, 2023
@bpali
Copy link
Author

bpali commented Apr 25, 2023

I've created a pull-request bpali#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation This will be tested / debugged or checked out.
Projects
None yet
Development

No branches or pull requests

1 participant