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

Fix #1474 "Sending message to contact with < or > in Recipient gets treated as "Sent" but is not received" #1476

Merged
merged 5 commits into from
May 29, 2020

Conversation

Hocuri
Copy link
Collaborator

@Hocuri Hocuri commented May 12, 2020

Fix #1474 "Sending message to contact with < or > in Recipient gets treated as "Sent" but is not received".

As I was at it, I also extracted the correct name and address from addresses like Mueller, Dave <dave@domain.com>.

src/dc_tools.rs Show resolved Hide resolved
Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

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

looks good to me logic-wise, but i would prefer if someone with a better idea of rust than me can also have a look at that

src/contact.rs Show resolved Hide resolved
lazy_static! {
static ref ADDR_WITH_NAME_REGEX: Regex = Regex::new("(.*)<(.*)>").unwrap();
}
if let Some(captures) = ADDR_WITH_NAME_REGEX.captures(addr.as_ref()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use slice pattern to avoid indexing captures and ensuring that there are exactly 2 groups captured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The shortest way I found to do this was:

    if let Some(captures) = ADDR_WITH_NAME_REGEX.captures(addr.as_ref()) {
        if let [n, a] = &captures.iter().collect::<Vec<_>>()[..] {
...

because

  • it is not that easy to build a slice from a Captures struct
  • a slice is not owned, so I can't use map but need the two ifs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So, I do not think this is worth it because it introduces quite some complexity (unless there is a way to express this shorter I did not find)

src/contact.rs Show resolved Hide resolved
@r10s
Copy link
Member

r10s commented May 13, 2020

as this not fixes a critical bug, compared to the other fixes, but touches pretty critical things on incoming mails, i think we should merge this to master after we released android 1.8.1 resp. core1.33

@Hocuri Hocuri force-pushed the add-combined-mail-and-addr branch from 95eafa9 to f5cd8f2 Compare May 16, 2020 16:41
@Hocuri
Copy link
Collaborator Author

Hocuri commented May 16, 2020

The CI is failing with a timeout. From the logs:

87.25 [inbox-ac1] DC_EVENT_WARNING data1=0 data2=ThreadId(125) src/contact.rs:349: Bad address "mailer-daemon@hq5.merlinux.eu (mail delivery system)" for contact "<unset>".
87.25 [inbox-ac1] DC_EVENT_INFO data1=0 data2=ThreadId(125) src/imap/mod.rs:642: Read error for message 20200516164742.112564051B@hq5.merlinux.eu from "INBOX", trying over later: IMAP other error: dc_receive_imf error: Bad address supplied: "mailer-daemon@hq5.merlinux.eu (mail delivery system)".
87.25 [inbox-ac1] DC_EVENT_WARNING data1=0 data2=ThreadId(125) src/imap/mod.rs:663: 2 mails read from "INBOX" with 2 errors.

Seems that DC receives a mail with
From: mailer-daemon@hq5.merlinux.eu (mail delivery system) (WTF????)
which is now rejected because it contains spaces.

Should we change merlinux's mail server configuration? Or fix this so that the mailer deamon does not have to send us messages (they probably say that the message could not be sent)?

Or be more relaxed when receiving messages (I do not think that we should accept mails where FROM is bad but we could add them to the database to avoid downloading them again; leaving from_id in dc_receive_imf 0 should be sufficient)?

@link2xt
Copy link
Collaborator

link2xt commented May 16, 2020

Seems that DC receives a mail with
From: mailer-daemon@hq5.merlinux.eu (mail delivery system) (WTF????)
which is now rejected because it contains spaces.

It is rejected because mailparse crate can't parse comments. According to https://tools.ietf.org/html/rfc5322#page-18 there is an optional [CFWS] after domain-literal, which stands for Comment or Folding White Space. (mail delivery system) is a comment and mailparse can't parse it.

deltachat-ffi/deltachat.h Outdated Show resolved Hide resolved
@Hocuri
Copy link
Collaborator Author

Hocuri commented May 17, 2020

So, what do we do? Use the unstable version from master? Wait for the mailparse update? Or be more strict later and only merge the sanitize_name_and_addr() thing now?

@hpk42
Copy link
Contributor

hpk42 commented May 17, 2020 via email

@Hocuri Hocuri force-pushed the add-combined-mail-and-addr branch from f5cd8f2 to 6fdca87 Compare May 18, 2020 13:40
@Hocuri
Copy link
Collaborator Author

Hocuri commented May 20, 2020

Yay, it works! Shall I merge this PR?

@Hocuri Hocuri force-pushed the add-combined-mail-and-addr branch from dd6ac94 to b9c21e7 Compare May 29, 2020 13:53
@Hocuri Hocuri merged commit b6161c4 into master May 29, 2020
@Hocuri Hocuri deleted the add-combined-mail-and-addr branch May 29, 2020 16:14
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.

Sending message to contact with < or > in Recipient gets treated as "Sent" but is not received
4 participants