-
Notifications
You must be signed in to change notification settings - Fork 101
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
MimeBodyPart RFC2231 Outlook issue: Allow to override internal static fields encodeFileName in constructor instead of SystemProperties #513
Comments
I think there is a more generalized solution that should be explored when dealing with system properties vs session properties. One of the reasons system properties are using is that it can be tricky to gain access to the session. However, for this case JakartaMail has an interface MessageAware that is designed to gain access to the session. For instance, MimeMultipart use this to set the parent. I think a similar approach should be applied to the MimeBodyPart by adding a single new constructor Next step would be to then add a For testing we would then have to check that in the JakartaMail itself if we need to move from MimeBodyPart(InputStream) MimeBodyPart(DataSource) to ensure the right settings are applied for other uses. There are more details to workout but this outlines the general approach. |
Thanks for the detailed suggestion.
Let me try to repeat in my words to see if I got that correctly: So is the idea to put the configuration like And then let Question: Why do you prefer putting MimeBodyPart-specific options in session properties - as opposed to set the directly on MimeBodyPart? Is this for consistency, because the system properties already exist? I mean the scope is smaller than system properties, but still larger. I am not doubting your approach, just trying to understand. |
You have captured the idea of my intent. I'm sure once it is coded up, the flaws of my hand waving will come to light :). The topic of session vs. system properties as come up before in the past. Unfortunately, I didn't draft up this approach and explain to @bshannon but were are his thoughts on a related topic:
When I read that I see two trade-offs at play when the original code was developed:
My thought is that going with "MessageAware DataSource" is actually inline with what has already been done in the past. E.G. MimeMultipart(DataSource). If needed my approach could work without adding new methods as getParent and setDataHandler already exist on the MimeBodyPart. My initial thoughts are that my solution would satisfy both trade offs above and make the behavior more like what has been done before already in this codebase. Adding the setters has merit but, not sure we need to be able to change the properties at that scope. The setter approach also means that we have to split methods in to classes in the Jakarta namespace and the com.sun namespace based on if they are spec or reference implementation. |
I have done another commit to get the properties from session. I found that we cannot get the InputStream from the DataSource because, although originally it comes from the DataSource, it changes during the MimeMultipart#parse. For this reason I created a new constructor having |
@jbescos Reviewing this is still on my todo list. I'll get you a proper review once I can give this a test drive. |
Is your feature request related to a problem? Please describe.
Some background: The reason behind this issue is our attempt to make it possible to send an Attachment to "Outlook client which has problems displaying filenames with german umlauts (ä, ü, ö etc.) correctly. Search the internet for "outlook rfc2231 rfc2047".
In the past we were using JavaMail 1.4.3 which worked out of the box with outlook. But now we are on 1.6.2 (yes, JavaMail, but we consider switching to JakartaMail. But JakartaMail still has the same problem as JavaMail 1.6.2)
Currently MimeBodyPart uses static variables for certain customisations like this:
The problem:
SystemProperties are global for the whole application.
We would like to use this only in some very specific cases where a customer can e.g. pass a flag to decide wether or not to encode the filename etc.
The magic currently happens in #setFileName(part, name) which unfortunately is package private so it hard / impossible to customize it.
You can see it in the comments like:
Describe the solution you'd like
Is it possible to make the private static fields such as
encodeFileName
,setContentTypeFileName
actual class members add a new constructor where you can pass the fields?This would allow us to use this constructor if we want to have full control over all parameters.
Otherwise we just use the default constructors which work as it is.
e.g. example pseudo code:
I assume this would also be backwards compatible when this is a new constructor.
Describe alternatives you've considered
An alternative was to create a customized version in our own
package jakarta.mail.internet;
and overridejavax.mail.internet.MimeBodyPartSynesty.setFileName(String)
Unfortunatelly then we have to copy lots of code from #setFileName(part, name)We have not used this alternative yet, it was just a POC.
e.g.
Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: