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

Message-Id improperly formatted #41

Closed
leanne63 opened this issue Aug 18, 2017 · 8 comments · Fixed by #42
Closed

Message-Id improperly formatted #41

leanne63 opened this issue Aug 18, 2017 · 8 comments · Fixed by #42
Assignees

Comments

@leanne63
Copy link

Problem:
Based on the current MESSAGE-ID format (as of v1.0.8), messages sent via Swift-SMTP are being marked as spam.

For example, Google changes the MESSAGE-ID value from
C3740F09-A2C0-4A84-A382-85FE08C87556.Swift-SMTP
for Gmail recipients to
<59971d49.8153650a.c8814.4b18SMTPIN_ADDED_BROKEN@mx.google.com>
and sends the message to the spam folder.

The correct format for the MESSAGE-ID field, as specified in RFC 5322, is:

message-id = "Message-ID:" msg-id CRLF
...
msg-id = [CFWS] "<" id-left "@" id-right ">" [CFWS]

id-left = dot-atom-text / obs-id-left

id-right = dot-atom-text / no-fold-literal / obs-id-right

"dot-atom-text" represents text, followed by a ".", followed by more text.

Solution:
To prevent Swift-SMTP messages from being marked as spam due to incorrect formatting, I propose coding the MESSAGE-ID field to contain a value like the following:
<B5B87EC6-683B-4A7D-AC8C-31F85BA01570.Swift-SMTP@senderdomain.com>

@quanvo87 quanvo87 self-assigned this Aug 21, 2017
@quanvo87 quanvo87 mentioned this issue Aug 21, 2017
3 tasks
@quanvo87 quanvo87 reopened this Aug 21, 2017
@quanvo87
Copy link
Collaborator

@leanne63 Thanks for pointing this out! I tried implementing a fix and have tagged a release. Would you like to give it a try? https://github.com/IBM-Swift/Swift-SMTP/releases/tag/1.1.0
Note the breaking API changes.

@quanvo87
Copy link
Collaborator

Also didn't mean to close the issue--accident.

@leanne63
Copy link
Author

So, it does work, @quanvo87 . However, I was thinking of a solution more similar to the following:

private var hostname: String {
	let fullEmail = from.email
	let atIndex = fullEmail.characters.index(of: "@")
	let hostStart = fullEmail.index(after: atIndex!)    // TODO: validate email, or leave to library user?
	let hostnameVal = fullEmail.substring(from: hostStart)
        
	return hostnameVal
}

That doesn't break the API. It also saves the library user from having to determine/code a separate hostname in the email initialization, when the sender's email domain will work just fine.

@quanvo87
Copy link
Collaborator

@leanne63 I like that this doesn't break the API.

There are some things I am looking into:

  • Does smtp.gmail.com vs gmail.com matter?
    • Currently, to send an email from my account, I provide smtp.gmail.com as the hostname, and xxx@gmail.com as the from email.
    • What if a hostname that is different from the one pulled from the email address is required?

@leanne63
Copy link
Author

To answer your questions in short:

What if a hostname that is different from the one pulled from the email address is required?

The spec doesn't say exactly what we need to put in the right-hand side of the "@" in a message-id.
It does recommend that this text represent a domain.

Does smtp.gmail.com vs gmail.com matter?

The spec says the right-hand side of the "@" needs to be a "dot-atom-text" - which means [sometext-without-dots].[sometext-without-dots].

Here are some quotes from the spec to explain in more detail:

Note: As with addr-spec, a liberal syntax is given for the right-
hand side of the "@" in a msg-id. However, later in this section,
the use of a domain for the right-hand side of the "@" is
RECOMMENDED.
Again, the syntax of domain constructs is specified
by and used in other protocols (e.g., [RFC1034], [RFC1035],
[RFC1123], [RFC5321]). It is therefore incumbent upon
implementations to conform to the syntax of addresses for the
context in which they are used.
.

This one means that we can put anything there, within the context of our implementation. It doesn't even have to be a domain name, but it's recommended to be. In our context, I only really see two possible domains: the logged in SMTP user and the sender.

We have easy access to the sender via the "From:" field; however, it would take more code manipulation to use the logged in SMTP user's domain. Also, I haven't seen any spam filters freak out based on which field is used here during my testing so far.


  • From the RFC, section 3.2.3, with my bolding and italics. Also formatted slightly different from original for easier reading:

atext = Printable US-ASCII characters not including specials. Used for atoms.
ALPHA / DIGIT /
"!" / "#" /
"$" / "%" /
"&" / "'" /
"*" / "+" /
"-" / "/" /
"=" / "?" /
"^" / "_" /
"`" / "{" /
"|" / "}" /
"~"

atom = [CFWS] 1*atext [CFWS]

dot-atom-text = 1*atext ("." 1atext)

dot-atom = [CFWS] dot-atom-text [CFWS]

specials = Special characters that do not appear in atext
"(" / ")" /
"<" / ">" /
"[" / "]" /
":" / ";" /
"@" / "" /
"," / "." /
DQUOTE

This is the definition of a dot-atom-text. Note that it consists of atext, followed by a dot, followed by another atext.

Note also that the spec says that atext can't contain "specials". And, the "specials" include a dot (on the line above DQUOTE, alongside the comma).

Therefore, the atext can't include a dot - thus smtp.gmail.com would violate the spec, but gmail.com would not.

@quanvo87
Copy link
Collaborator

Awesome, thanks for the breakdown! I see no reason not to go with your implementation then. Were you working on a PR? Otherwise, I'll revert my changes, and add what you have suggested.

@leanne63
Copy link
Author

I can do a PR. I'll get it out there sometime today.

@quanvo87
Copy link
Collaborator

https://github.com/IBM-Swift/Swift-SMTP/releases/tag/v1.1.2

@leanne63 merged and tagged. Thank you!

@quanvo87 quanvo87 closed this as completed Sep 7, 2017
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 a pull request may close this issue.

2 participants