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

Allow overriding Default FormatOptions #993

Open
alexeik opened this issue Jan 19, 2024 · 6 comments
Open

Allow overriding Default FormatOptions #993

alexeik opened this issue Jan 19, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@alexeik
Copy link

alexeik commented Jan 19, 2024

image

I have on left side letter that Outlook sees as good attachment
On right side letter builded with MimeKit and Outlook cant see this attachment name as it is...
Outlooke gives for attachmenet default name like 'New text document 0029.txt' for right side example..
Deal is about Name attribute, Outlook doesnt parse nameN collection and set it as 'New text document 00NN.txt'

How i can manage MimeKit or is it impossible?

@alexeik
Copy link
Author

alexeik commented Jan 19, 2024

ok. i have found Rfc2047 problem.
thanks

@jstedfast
Copy link
Owner

For anyone else that runs into this issue, you can force MimeKit to encode the name and filename parameters in the Content-Type and Content-Disposition headers, respectively.

When creating your attachment (once you've set a FileName), you can add the following code to your program to override MimeKit's default parameter encoding methods:

Parameter param;

if (attachment.ContentType.Parameters.TryGetValue ("name", out param))
    param.EncodingMethod = ParameterEncodingMethod.Rfc2047;

if (attachment.ContentDisposition.Parameters.TryGetValue ("filename", out param))
    param.EncodingMethod = ParameterEncodingMethod.Rfc2047;

It looks like MimeKit's FormatOptions.Default property values cannot be overridden (which used to also be the case for ParserOptions.Default), but perhaps I should loosen the restriction on that to allow developers to override this once and not have to manually override it per parameter when they are creating messages.

In other words, if I loosen the restriction, then you could do this at program startup and not have to copy&paste the above code:

MimeKit.FormatOptions.Default.ParameterEncodingMethod = ParameterEncodingMethod.Rfc2047;

@alexeik
Copy link
Author

alexeik commented Jan 19, 2024

@jstedfast just make it lovely to manage :)
nobody wants restrictions

jstedfast added a commit that referenced this issue Jan 27, 2024
@jstedfast jstedfast added the enhancement New feature or request label Jan 27, 2024
@jstedfast jstedfast changed the title Content-Type: application/octet-stream; & name collection Allow overriding Default FormatOptions Jan 27, 2024
@jstedfast
Copy link
Owner

Okay, it is now possible to override FormatOptions.Default values.

My recommendation is to set them at program startup and then leave them alone after that.

@jstedfast
Copy link
Owner

jstedfast commented Feb 4, 2024

Ooof, I may have to revert this change. Unfortunately, by changing the Default properties, it causes some issues where some code made certain assumptions about the Default values.

These aren't unfixable, it's just that in order to make everything work with well-defined behavior, I need to add a FormatOptions.ReformatHeaders property and perhaps even a ReformatContent property.

The problem with always reformatting is that it makes verification of DKIM, ARC, PGP/MIME and S/MIME digital signatures impossible because whatever settings you choose for FormatOptions will result in a different signature verification results.

This means that I need a way to make it so that FormatOptions are /not/ used in some situations.

If I revert the changes I've made so far to make this possible, I'll definitely implement this feature for v5.0 because I'd love to have this feature, but I might not be able to do it for 4.x because I don't want to break existing behavior in a 4.x release.

jstedfast added a commit that referenced this issue Feb 7, 2024
…erty values

This reverts the following commits:
* 4b77a9f: Allow changing the FormatOptions.Default values
* e3bab7b: Improve Header.GetRawValue() logic
* 22881de: Cache more of the FormatOptions in each Header
* 9570ebd: Added FormatOptions.ReformatHeaders and fixed unit tests

...And reopens issue #993.

I will be revisiting this set of changes for v5.0 when I'll have more flexibility with
behavioral changes. The problem with this set was not that it "didn't work", but rather
that it changed expected behavior. I also wasn't sure if I really liked the new
FormatOptions.ReformatHeaders property naming.
@jstedfast jstedfast reopened this Feb 7, 2024
@jstedfast
Copy link
Owner

jstedfast commented Feb 7, 2024

I had to revert this.

In fairness, it's not so much the ability to override the FormatOptions.Default property values that broke behavior, it's the fact that I made it such that Header.GetRawValue(FormatOptions) reformats the header value if any of the options used to format the cached rawValue changed since the previous formatting of said value.

The old Header.GetRawValue(FormatOptions) would only ever reformat the rawValue if the International property was set to true (FormatOptions.Default.International is/was false) and this was a hack and is why, if you configure your own custom FormatOptions that changes the ParameterEncodingMethod or the AlwaysQuoteParameterValues, for example, and then used that set of options when writing a message to a stream, the Content-Type and Content-Disposition headers don't get reformatted to obey the new formatting options.

It's technically a bug in the old (and now current again) code and the changes I made to support changing the Default values fixed this, but as a consequence, it changed the "expected behavior".

To combat this, I added a FormatOptions.ReformatHeaders property which would make it such that Header.GetRawValue(FormatOptions) would only reformat if the value was true, otherwise it would use the cached rawValue, making it possible to prevent any sort of reformatting that would cause DKIM/ARC/SMIME signature verification from failing.

That solved the main issue, but it's a bit of an oddly named property on something called FormatOptions 😆

There was also at least 1 more issue that I need to figure out:

When a developer DKIM-signs their message, for example, using dkimSigner.Sign (formatOptions, message, headers), we need a way to lock those raw header values to whatever format is used there so that when they either save the message to disk or send it via MailKit's SmtpClient, we can't assume that the user will remember to use the exact same FormatOptions.

This didn't used to be a problem because, other than the International property that I mentioned earlier, none of the FormatOptions passed to the Sign(...) method would alter the headers.

Some might argue that this isn't technically MimeKit's responsibility to guarantee that the developer using it doesn't shoot themselves in the foot, but I like to try and prevent that as much as possible if I can (even if only to reduce the number of "why isn't this working?"-type questions).

So that's where we're at.

I definitely want this because it would make tasks like what @alexeik is trying to do so much simpler and because it would get rid of some hacks in MimeKit that gnaw at me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants