Hi, when you send an email with a base64 image in the body, the image is corrupted by Mime Mail module.

In my case I'm sending an email from webform and the body contains an image like:

<img src="data:image ..." />

The mimeMailExtractFiles() function regex should skip images with "data:image" on the "src" attribute in order to skip this case.

I've created a simple patch to fix this bug.

Comments

FiNeX created an issue. See original summary.

tr’s picture

Status: Active » Needs review

Setting to "Needs review" so DrupalCI will test the patch.

tr’s picture

StatusFileSize
new5.45 KB

when you send an email with a base64 image in the body, the image is corrupted by Mime Mail module.

You didn't specify how the image was corrupted or why you think it's corrupted, but from my testing of this problem I think it's because the mimeMailUrl() method was URL-decoding your base64 data, which replaced '+' with a space, destroying the base64 encoding because '+' is an actual allowed character in base64.

Regardless, I don't like your proposed fix. First, it only works in the case of "data:image", and it works by skipping over these URIs entirely. We don't skip any other URI schemes like this. Second, it makes the already-fragile regular expression more fragile and harder to understand. I would like to see all these regular expressions removed, rather than enhanced. Third, I don't think mimeMailExtractFile() is the correct place to handle URIs with a data: scheme. Fourth, it doesn't modify the documentation comments or add a test case to explain and demonstrate your changes.

Instead, what I propose is making changes to mimeMailFile(). Specifically, the purpose of mimeMailFile() is to recognize URIs that identify content that needs to be inlined into the message, then replace that URI in the HTML with a reference to the part of the multipart MIME mail message that contains the inlined content. So http: and https: URLs, because they point to external resources, don't need to be replaced with references and don't need the content at those URLs included in the email. But .jpg images stored on the local (sending) file system DO need to be inlined because they may not be publicly accessible via a URL. Likewise, images that are encoded with a data: scheme do not need to be inlined because the URI with the data: scheme contains the complete encoding of the image.

So, my change simple ensures that data: schemes are explicitly recognized as content that doesn't need to be inlined. It also comments out the URL-decoding that is done in mimeMailUrl(), because that appears to be just wrong - URLs should never be encoded here, so we should not need to decode them ever. I have left this commented out because there is no documentation about WHY the decoding was put in there, so I'm not quite sure if this will break something else. I enhanced the tests for mimeMailUrl() to reflect this change, so if someone DOES encounter a problem we have a test case that can be used to reproduce the problem.

I am in the process of writing test cases to completely test mimeMailFile(), including a test case for embedding base64 encoded images, but that is a large project that is being handled in #3145400: Add test cases for MimeMailFormatHelper::mimeMailFile().

PLEASE TEST THIS PATCH and let me know if it fixes your problem.

tr’s picture

StatusFileSize
new5.46 KB

Changed whitespace.

tr’s picture

@FiNeX: Can you test the patch please?

Can anyone else test and review the patch also?

  • TR committed 05ec6989 on 8.x-1.x
    Issue #3325765 by TR, FiNeX: Corrupted base64 images using Mime Mail...
tr’s picture

Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.