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

[Feature Request] Avoid fetching attachments if it's already downloaded #585

Closed
yrccondor opened this issue Mar 17, 2021 · 3 comments
Closed
Labels
enhancement This will enhance this library. needs investigation This will be tested / debugged or checked out.

Comments

@yrccondor
Copy link

I'm using this library for a while, and it's really helpful!

I noticed that when I dealing with mails with multiple inline iamges (cid:...), IncommingMail::embedImageAttachments() took a very long time to process (I've set $attachmentsDir to null). I found that for
every call to IncomingMailAttachment::getContents() will eventually call imap_fetchbody in Imap::fetchbody(), which took a long time to fetch.

Since we already have attachment data in DataPartInfo::data, can we just return it in DataPartInfo::fetch() if data is set or add a getter for DataPartInfo::data?

I'm not really familiar with imap, sorry if I'm wrong.

@yrccondor yrccondor added the needs investigation This will be tested / debugged or checked out. label Mar 17, 2021
@yrccondor
Copy link
Author

I've made a quick fix. This works for me.

DataPartInfo::fetch():

public function fetch(): string
{
    if (0 === $this->part) {
        $this->data = Imap::body($this->mail->getImapStream(), $this->id, $this->options);
    } else {
        if (null !== $this->data) {
            return $this->data;
        }
        $this->data = Imap::fetchbody($this->mail->getImapStream(), $this->id, $this->part, $this->options);
    }

    return $this->decodeAfterFetch();
}

BTW, since only a "Content-ID" is also enough:

/**
* 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)) {

It's may better to change this line to if ($attachment->contentId == $cid) {, otherwise in some cases we'll get a wrong image.

Repository owner deleted a comment from sunnyzss928 Nov 23, 2021
@Sebbo94BY Sebbo94BY added the enhancement This will enhance this library. label Nov 26, 2021
Sebbo94BY added a commit that referenced this issue Jan 8, 2022
…-when-already-downloaded

#585: Avoid fetching attachments when they are already downloaded
@Sebbo94BY
Copy link
Collaborator

Thank you very much for this code improvement @yrccondor!

I've implemented and tested it.

@Sebbo94BY
Copy link
Collaborator

This feature/change has been released in version 4.5.0.

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

No branches or pull requests

2 participants