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

Switch mailparsing to mailparse #904

Merged
merged 20 commits into from
Dec 3, 2019
Merged

Switch mailparsing to mailparse #904

merged 20 commits into from
Dec 3, 2019

Conversation

dignifiedquire
Copy link
Member

@dignifiedquire dignifiedquire commented Nov 28, 2019

This uses mailparse for parsing and `lettre_email for mime generation.

@link2xt
Copy link
Collaborator

link2xt commented Nov 29, 2019

@dignifiedquire Remove .rsynclist

This file alone adds 3k lines

@r10s
Copy link
Member

r10s commented Nov 30, 2019

just played around a bit, i could sent out unencrypted mails!

whats came over my way is that the To: field repeats which may lead into group-chats instead of one-to-one-chats. although this could probably fixed on group-creation, i think it is better not to add ourself to the To: list - even not if bcc_self is enabled and ourself is added as a recipient for the smtp-server.

@dignifiedquire dignifiedquire changed the title [WIP] [experiment] switch mailparsing to mailparse [WIP] switch mailparsing to mailparse Nov 30, 2019
@dignifiedquire dignifiedquire marked this pull request as ready for review December 2, 2019 07:53
@dignifiedquire dignifiedquire changed the title [WIP] switch mailparsing to mailparse Switch mailparsing to mailparse Dec 2, 2019
#[fail(display = "Building invalid Email: {:?}", _0)]
LettreError(#[cause] lettre_email::error::Error),
#[fail(display = "FromStr error: {:?}", _0)]
FromStr(#[cause] mime::FromStrError),
Copy link
Contributor

Choose a reason for hiding this comment

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

according to latest discussions and PRs from @link2xt @flub and me we put errors into each module (see sql/smtp and imap.rs). but that doesn't need to be part of this PR now.

unsafe {
dc_receive_imf(context, &body, folder.as_ref(), server_uid, flags as u32);
}
dc_receive_imf(context, &body, folder.as_ref(), server_uid, flags as u32);
Copy link
Contributor

Choose a reason for hiding this comment

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

w.o.n.d.e.r.full :)

Copy link
Member

Choose a reason for hiding this comment

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

incredible. awesome. just great.

.body(encrypted)
.build(),
)
.header(("Subject".to_string(), "...".to_string()));
Copy link
Contributor

Choose a reason for hiding this comment

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

beautiful 💯


if let Some(raw) = raw {
mail_raw = raw;
info!(context, "decrypted: {:?}", std::str::from_utf8(&mail_raw));
Copy link
Contributor

Choose a reason for hiding this comment

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

can this not crash if the mail_raw is invalid utf8?

Copy link
Member Author

Choose a reason for hiding this comment

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

it can, and it should not be there, I need to go and cleanup all my debugging logs

Copy link
Contributor

Choose a reason for hiding this comment

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

are there many debugging logs? I think what's more important is going through all from_utf8 if any.

// Handle gossip headers
let gossip_headers = mail.headers.get_all_values("Autocrypt-Gossip")?;
parser.gossipped_addr =
update_gossip_peerstates(context, message_time, &mail, gossip_headers)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems wrong. We can only use and process gossip headers if they are in the encrypted part.

Copy link
Contributor

@hpk42 hpk42 left a comment

Choose a reason for hiding this comment

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

Overall this looks great to me. Remaining from my side:

  • check all utf8 handling (maybe also in mailparse).
  • move the gossip header updates into the decryption part

@r10s r10s merged commit fe2a4b7 into master Dec 3, 2019
@r10s r10s deleted the mailparse branch December 3, 2019 13:16
@dignifiedquire
Copy link
Member Author

giphy

@hpk42 hpk42 restored the mailparse branch January 29, 2020 15:27
@link2xt link2xt deleted the mailparse branch June 16, 2021 21:21
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.

None yet

4 participants