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

feat: MimeMessage: Support extra Content-Type directives #6

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

iequidoo
Copy link

@iequidoo iequidoo commented Oct 2, 2023

Support extra "Content-Type" directives in MimeMessage passed in a fake "Content-Type-Deltachat-Directives" header which is then removed from the resulting message. This is needed to support adding "protected-headers="v1"" directive to the "Content-Type" header of multipart messages.

This is related to deltachat/deltachat-core-rust#2302

@iequidoo
Copy link
Author

iequidoo commented Oct 2, 2023

@link2xt, which branch should i merge this to, not to break the current versions of DC? Do we have an elaborated procedure for that?

@iequidoo iequidoo requested a review from link2xt October 2, 2023 00:13
Support extra "Content-Type" directives in MimeMessage passed in a fake
"Content-Type-Deltachat-Directives" header which is then removed from the resulting message. This is
needed to support adding "protected-headers=\"v1\"" directive to the "Content-Type" header of
multipart messages.
@iequidoo iequidoo force-pushed the iequidoo/content-type-directives branch from 428cfa6 to 347dfa9 Compare October 2, 2023 00:16
@iequidoo iequidoo changed the title feat: MimeMessage: Support extra Content-Type directives (#2302) feat: MimeMessage: Support extra Content-Type directives Oct 2, 2023
@link2xt
Copy link

link2xt commented Oct 2, 2023

Why would it break existing version? Isn't the change backwards-compatible if nobody sets this header?

There is basically only the master branch.

let mut ct_value: String;
if let Some(message_type) = self.message_type {
// Use `message_type` as `Content-Type` if present
ct_value = message_type.to_string();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@link2xt you're right, it's backward-compatible, we even never get here if self.children.len() == 0

};
}
if let Some(directives) = self.headers.get("Content-Type-Deltachat-Directives".to_string()) {
let directives: String = directives.get_value().unwrap();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unwrap looks a bit scary, but FromHeader for String never returns error and there is a similar pattern in other places of the code, so it's ok. Could still wrap this into if let Ok(..) for extra safety, dealing with panics especially on mobile is hard, better avoid them when we can.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function already did the same for "Content-Type" header, so i decided not to change this right now. But i agree that better to refactor this

@link2xt link2xt merged commit 37778c8 into master Oct 4, 2023
@link2xt link2xt deleted the iequidoo/content-type-directives branch October 5, 2023 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants