fix(message-parser): match parts on mime_type so multipart/related survives tracking - #3597
Open
cl77 wants to merge 1 commit into
Open
fix(message-parser): match parts on mime_type so multipart/related survives tracking#3597cl77 wants to merge 1 commit into
cl77 wants to merge 1 commit into
Conversation
…rvives tracking MessageParser#parse_parts matched on part.content_type, which is the full header value including parameters. A multipart/related container carrying the RFC 2387 type="text/html" parameter (emitted by PHPMailer and other senders for HTML mails with inline CID images) therefore matched the /text\/html/ branch first: the container's raw source was decoded and reassigned as a text body, and re-serialization rebuilt it as a broken multipart/alternative whose inline images no client renders. Matching on part.mime_type (bare MIME type, no parameters) routes such containers into the multipart branch, consistent with how generate() already inspects @mail.mime_type for non-multipart messages. Adds a regression spec covering the nested related container with a type parameter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MessageParser#parse_parts matches parts with case part.content_type, which returns the full Content-Type header value including parameters. A multipart/related container that carries the RFC 2387 type="text/html" parameter - which PHPMailer and other mainstream senders emit for HTML mails with inline CID images - therefore matches the first when /text/html/ branch instead of the multipart branch.
The consequences for any tracked message with that structure (multipart/alternative containing multipart/related with inline images):
Since the images then live in an alternative container rather than a related one, no mail client resolves the cid: references anymore - the message arrives without inline images, and the tracking pixel never reaches the real HTML part either. We hit this in production sending via the send/raw API with open tracking enabled.
Fix
Match on part.mime_type, the bare MIME type without parameters. This is consistent with generate(), which already inspects @mail.mime_type for non-multipart messages. No behavior change for any part whose Content-Type carries no parameters that collide with the matchers.
Repro / validation
Standalone reproduction with the mail gem (structure of the re-serialized message after parse_parts):
Before (case part.content_type):
multipart/alternative > [text/plain, multipart/alternative > [text/plain (raw container source), text/html, image/png]] - related container destroyed, Content-ID lost, no pixel in HTML.
After (case part.mime_type):
multipart/alternative > [text/plain, multipart/related > [text/html (pixel injected), image/png with Content-ID]] - structure intact.
A regression spec is included: it feeds a multipart/alternative message with a nested multipart/related; type="text/html" container through the parser with a track domain enabled and asserts the related container, its image part's Content-ID and the injected tracking pixel in the HTML leaf all survive.