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

Email.To.Name containing comma causes problem #268

Closed
APM3 opened this issue Jul 9, 2016 · 14 comments
Closed

Email.To.Name containing comma causes problem #268

APM3 opened this issue Jul 9, 2016 · 14 comments
Labels
type: non-library issue API issue not solvable via the SDK

Comments

@APM3
Copy link

APM3 commented Jul 9, 2016

Issue Summary

When I create Email with Name that is Last, First, it gets thru the SendGrid ok, but then SendGrid drops it. The Json that is created by Mail.Get() looks correct, so I think the problem is in the webApi handler on the server side.

Steps to Reproduce

Send message that has a recipient with name that has a comma in it.
SendGrid will drop it for having an invalid address

Technical details:

var msg = new Mail(new Email("[email protected]"), "Test", new Email("[email protected]", "Last, First"), new Content("text/plain", "Testing 1-2-3"));
var request = new SendGrid.SendGridAPIClient(ApiKey);
request.client.mail.send.post(requestBody: msg.Get());

It sends ok, but now if you go check in SendGrid then you will see that SendGrid drops the email for having a bad address, and it appears to be using part of the name as the address...instead of the actual address.

  • sendgrid-csharp Version: 7.0.5
  • .NET Version: 4.5.2
@thinkingserious thinkingserious added type: bug bug in the library status: help wanted requesting help from the community labels Jul 10, 2016
@thinkingserious
Copy link
Contributor

@APM3,

This is an issue server side. This may be a temporary fix for you: http://stackoverflow.com/questions/30267595/smtpclient-send-with-valid-email-address-that-has-a-comma-in-it

Thanks!

@jsgoupil
Copy link

jsgoupil commented Oct 5, 2017

This is still an issue as of October 5th 2017.
Surrounding the name with double quotes is a terrible workaround that will display double quotes on the email client.
The "FROM" can have a comma, but the "TO" cannot.

More than a year and this bug is not fixed and marked as closed.

@thinkingserious thinkingserious added type: non-library issue API issue not solvable via the SDK and removed status: help wanted requesting help from the community type: bug bug in the library labels Oct 5, 2017
@thinkingserious
Copy link
Contributor

Hi @jsgoupil,

I've passed along your vote to our product team on this one. I'm not sure why I closed this issue, but a similar issue exists in some of our other repos and the product team does have a fix scheduled on the backlog. Your vote helps bump the priority, so thanks for taking the time to provide feedback!

With Best Regards,

Elmer

@jgyllen
Copy link

jgyllen commented Apr 10, 2018

Hey @thinkingserious, any update on this? We just spent a whole day investigating why a recipient name containing a comma was processed successfully but dropped because it parsed it incorrectly and used part of it as the email. Thanks!

@thinkingserious
Copy link
Contributor

Hello @jgyllen,

I know this issue is on the product teams radar and a fix should happen at some point.

Would you mind reporting your issue using the feedback blue button at SendGrid.com? Queries from customers directly help increase the priority.

Thanks!

With Best Regards,

Elmer

@velja
Copy link

velja commented Jul 12, 2018

This still exists and caused problems to all our customers having comma in company name. Is there any time-frame for resolving this? This is a deal breaker for us since users don't want to change company names.

@thinkingserious
Copy link
Contributor

Hello @velja,

I don't have a firm time frame, but I've added your vote to the issue to help increase priority.

In the meantime, someone implemented this patch in PHP, you may be able to create a fix based on the same concept in Node.js.

With Best Regards,

Elmer

@thinkingserious thinkingserious added the difficulty: unknown or n/a fix is unknown in difficulty label Sep 29, 2018
@blushingpenguin
Copy link

Sendgrid also doesn't quote @ characters in mailboxes properly. This leads to

msg.AddTo(new EmailAddress("[email protected]", "[email protected]"))

generating an invalid to header:

To: [email protected] [email protected]

the name portion ([email protected]) is invalid according to RFC2822 -- it may not contain @.

While this may appear to be cosmetic it causes deliverability issues, e.g. in exim (with a largely default configuration), header validation is enabled:

2018-10-31 15:43:09 1g0000-000000-LG H=o1.f.az.sendgrid.net [208.117.55.132] X=TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256 CV=no F=<bounces+7000000-0000-....> rejected after DATA: malformed address: [email protected]\n may not follow [email protected] : failing address in "To:" header is: [email protected] [email protected]"

The workaround here is the same -- manual quoting of the name is required.

@mikeball
Copy link

Unfortunately I too just wasted a whole afternoon tracking down with this precise issue, and it seems crazy it's not yet been dealt with after 3 years.

My suggestion is to simply check for names that have a comma in them, and if they are not quoted, then quote them. Simple fix would have saved me, and others so very many hours of work, and the thousands of people wouldn't have their emails silently dropped for such a silly reason.

@vanillajonathan
Copy link
Contributor

vanillajonathan commented Apr 11, 2019

Would adding double quotes around the value of the Name attribute in the EmailAddress class solve this?

public EmailAddress(string email, string name = null)
{
this.Email = email;
this.Name = name;
}

-this.Name = name;
+this.Name = '"' + name+ '"';

.NET normalizes the string.
https://github.com/dotnet/corefx/blob/master/src/System.Net.Mail/src/System/Net/Mail/MailAddress.cs#L79
https://github.com/dotnet/corefx/blob/master/src/Common/src/System/Net/Mail/MailAddressParser.cs#L314

It would probably be best to accept the MailAddress class from .NET as a parameter instead of an own EmailAddress class, then read the DisplayName property from the .NET MailAddress class to internally create the EmailAddress class used for JSON serialization.

@mxa0079
Copy link

mxa0079 commented Jun 19, 2020

We also wasted a full evening tracking down this issue. It has been 4 years since this was reported. Sengrid team, could you please either solve this issue or make it more visible so other people won't wast time on this?

@eshanholtz
Copy link
Contributor

Closing as this was marked as a non-library issue.

@eshanholtz
Copy link
Contributor

After further investigation, I'm reopening this issue and marking it as a bug. This issue has been added to our internal backlog to be prioritized. +1 on the original issue description and PRs will help move it up the backlog.

@eshanholtz eshanholtz reopened this Sep 3, 2020
@eshanholtz eshanholtz removed difficulty: unknown or n/a fix is unknown in difficulty type: non-library issue API issue not solvable via the SDK labels Sep 3, 2020
@eshanholtz eshanholtz added status: help wanted requesting help from the community type: bug bug in the library labels Sep 3, 2020
@childish-sambino
Copy link
Contributor

This issue has been resolved in the backend. Closing out.

@childish-sambino childish-sambino added type: non-library issue API issue not solvable via the SDK and removed status: help wanted requesting help from the community type: bug bug in the library labels Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: non-library issue API issue not solvable via the SDK
Projects
None yet
Development

No branches or pull requests