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

Multiple Bodyparts of same Content-Type not supported for text/html & text/plain within Multipart/mixed or Multipart/alternative #139

Closed
pmaedel opened this issue May 2, 2018 · 9 comments

Comments

@pmaedel
Copy link
Contributor

pmaedel commented May 2, 2018

Current implementation (conditional null check) leads to only the first occurrence of a body part with the respective Content-Type to be parsed. All further occurrences are omitted. Correct behavior should be to either provide a List per Content-Type or concatenate the Strings.

if (isMimeType(currentPart, "text/plain") && parsedComponents.plainContent == null && !Part.ATTACHMENT.equalsIgnoreCase(disposition)) {
parsedComponents.plainContent = parseContent(currentPart);
} else if (isMimeType(currentPart, "text/html") && parsedComponents.htmlContent == null && !Part.ATTACHMENT.equalsIgnoreCase(disposition)) {
parsedComponents.htmlContent = parseContent(currentPart);

Multiple occurrences of body parts with the same Content-Type is valid as per RFC1341, section 7.2.2:

[...] the body parts are independent and intended to be displayed serially [...]

At least the Apple Mail client makes extensive use of this feature when replying or forwarding mails.

@pmaedel pmaedel changed the title Multiple Bodyparts of same Content-Type not supported for text/html & text/plain within Multipart/mixed or Multirpart/alternative Multiple Bodyparts of same Content-Type not supported for text/html & text/plain within Multipart/mixed or Multipart/alternative May 2, 2018
bbottema added a commit that referenced this issue May 4, 2018
#139: Make MimeMessageParser support multiple body parts of same Content-Type for text/plain and text/html
@bbottema
Copy link
Owner

What I'm worried about is we are losing information in the conversion process (the grouping of content) as we are simply merging the body parts of the same mimetype.

For example, when parsing a MimeMessage to Email this way and sending that email will not result in the same MimeMessage. I'm not sure what the consequence is here or whether this is an issue that should be addressed.

@pmaedel
Copy link
Contributor Author

pmaedel commented May 24, 2018

I have not had a look at how an email is sent, but if the htmlContent & plainContent fields are used instead of the underlying MimeMessage body parts, the previous obmission of body parts of the same content-type within these fields would have breaking the equality of representations too, for the mentioned MimeMessage -> Email -> MimeMessage scenario.

If these fields are just used for convenient access to the body parts a simple concatenation should suffice. Otherwise Lists of Strings might be a better representation for these values, as these would be better suited as a base for sending out emails that more closely resemble the original MimeMessage.

Repository owner deleted a comment from pmaedel Jun 3, 2018
@bbottema
Copy link
Owner

bbottema commented Jun 3, 2018

Did you run into a problem because of this issue?

Reason I'm asking is because I'm not sure in which use case multiple body parts would have the same content type for plain text or HTML text. Forwarded emails have a separate content type (message/rfc822, which is discarded when parsing a MimeMessage that contains one, since it is a proprietary format). That leaves replied-to emails. When replying, Simple Java Mail quotes the replied-to email inline and not as a separate bodypart. In this case it would make sense to simply concatenate incoming bodyparts of the same content type (text or HTML) when parsing a MimeMessage.

As it stands, I think concatenation is sufficient.

@bbottema bbottema added this to the 6.0.0 milestone Jun 3, 2018
@pmaedel
Copy link
Contributor Author

pmaedel commented Jun 7, 2018

We ran into this problem when parsing forwarded mails. I can send you an example (by mail) if you want to.

Repository owner deleted a comment from pmaedel Jun 30, 2018
@bbottema
Copy link
Owner

bbottema commented Jul 1, 2018

I've tried the EML you sent in a couple of viewers and they all seem to have an issue with this type of sparse multipart part structure. Most of the content is completely missing. Haven't seen this before (but then again, I don't use Apple products).

I sent the EML you gave to myself using Simple Java Mail, which shows a lot more (with your fix). Seems good enough to me. Closing this issue.

@pmaedel
Copy link
Contributor Author

pmaedel commented Sep 28, 2018

Hi, how does it come that the PR has been merged but the changes are neither part of the master branch nor of the 5.0.4 release?

@bbottema
Copy link
Owner

bbottema commented Sep 28, 2018

I switched to a proper GIT workflow and what was previously master is now develop. Master now represents what is actually released to Maven Central. So the changes for this issue are in the develop branch until 6.0.0 is released.

@bbottema
Copy link
Owner

Released in 6.0.0-rc1!

@bbottema
Copy link
Owner

6.0.0 has released as well, finally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants