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

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

Closed
angelo-fuchs opened this issue May 12, 2020 · 8 comments · Fixed by #1476
Assignees
Labels
bug Something is not working

Comments

@angelo-fuchs
Copy link

When I write a message to a contact that has the form

Test <example@example.com>

It gets send by DC (tried Desktop and Android) and treated as fine, but it is neither actually sent, nor displayed in the other DC because we refuse to accept mails like this. (Which is maybe also a Bug, because a Groupmail that contains one of such adresses is dropped completely)

Steps to reproduce:

  1. Open DC
  2. Start a new Chat
  3. Enter example <test@example.com> or any other message with that form
  4. Click "New Contact 'example test@example.com'"
  5. Type any message
  6. Hit send.

When send with DC-Desktop, DC-Android says in the log (but no alert shown; Should this be fixed as well?):

src/imap/mod.rs:766: dc_receive_imf failed for imap-message DeltaChat/3051: Message("could not parse \"<Example <test@example.com>>\": \"Unexpected char found after bracketed address\"")

I noticed the issue because I somehow have a contact in my list with this form (I still try to figure out where it came from).

@angelo-fuchs
Copy link
Author

wrong_To_form.eml.txt

This is an anonymized copy of the message where I first noticed the issue. This mail was SEND by DC, but can not be RECEIVED by it.

@Hocuri
Copy link
Collaborator

Hocuri commented May 12, 2020

Are you working on this or shall I try my luck?

@r10s
Copy link
Member

r10s commented May 12, 2020

@Hocuri please also have a look, might be related to #1438 EDIT: probably unrelated, @angelo-fuchs do you know if the issue is really new? i would assume, it is pretty old. or is it a regression?

for fixing: email addresses in the database should never have the format name <addr@domain> but only the raw thing addr@domain. so we should probably be more careful when receiving addresses - i think, "from the wire" this is okayish as we get things parses from the mimeparser, however, this goes over dc_create_contact() which seems to be too accepting.

just had a rough look:

  • Contact::add_or_lookup() us called from dc_create_contact() as well as for addresses coming from the wire, i think, this function should just not accept non-email addresses. in theory, there is a check, may_be_valid_addr() that calls addr.parse::() - but this seems not to work.
    it is hard to check for email-addresses via regex, i would maybe not go that way, it is so easy to introduce new here :) maybe just check that it contains one @ and one . and no whitespace and no < or >

  • Contact::create() is called from dc_create_contact() and gets user-input - i think, instead of just returning and error, here we can also adapt/split the user input before handing it finally to Contact::add_or_lookup().

@angelo-fuchs
Copy link
Author

I don't know if it is new. I could not find an open issue addressing it and it is in the current DC.

I found the source of the Address: When I hit "copy" in my regular Mail program on an email that contains a name, it copies the mail with it.
When I then just hit paste in DC -> new message/contact it copies the whole segment in and accepts it.

@Hocuri I am not working on this currently.

@r10s
Copy link
Member

r10s commented May 12, 2020

When I then just hit paste in DC -> new message/contact it copies the whole segment in and accepts it.

yip, this is what i mean with adapt/split in dc_create_contact() - we should either (a) not accept the whole thing, (b) parse the email address out of the "name " thing - or (c) even split to name + addr if no name is given to dc_create_contact() explicitly.

@Hocuri i assigned you to the issue.

@Hocuri
Copy link
Collaborator

Hocuri commented May 12, 2020

There is also dc_add_address_book() which does not call through to Contact::create() but directly to Contact::add_or_lookup(). So, I think that this should be done in Contact::add_or_lookup().

@Hocuri
Copy link
Collaborator

Hocuri commented May 12, 2020

even split to name + addr if no name is given to dc_create_contact() explicitly.

This does sound like a job for a regex. And we do not want to reject any bad email address, just the obvious ones.

@r10s
Copy link
Member

r10s commented May 12, 2020

So, I think that this should be done in Contact::add_or_lookup()

checking for validity - yes. Contact::add_or_lookup() already calls may_be_valid_addr() - so for a fix, adapting may_be_valid_addr() might already be fine.

but if it comes to "smart" things as splitting NAME <ADDR> i would do this in dc_create_contact() only - add_or_lookup() and add_address_book() should not try to be smart imho, just fail gracefully. (cc @hpk42) at least this is what i would do now, to make things stable and to fix. changing the behavior of add_or_lookup() might introduce new problems.
the issue is actually only on dc_create_contact().

but we can put this into a sanitize_input() or so function, that takes a string (email and maybe more) and returns sanitized email+name. so we can easily decide to use that also on different places.

but the very first, and maybe sufficient things is to improve may_be_valid_addr().

Hocuri added a commit that referenced this issue May 29, 2020
…reated as "Sent" but is not received" (#1476)

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>.
@r10s r10s added the bug Something is not working label Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working
Projects
None yet
3 participants