Skip to content

Commas in recipient name cause mail to be silently dropped (worked in v2) #291

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

Closed
medmunds opened this issue Jan 17, 2017 · 16 comments
Closed
Labels
difficulty: unknown or n/a fix is unknown in difficulty status: help wanted requesting help from the community type: non-library issue API issue not solvable via the SDK

Comments

@medmunds
Copy link

Issue Summary

When sending a message, if a recipient name contains a comma, the send appears to succeed but the message ends up with a "dropped" status and is never delivered.

The equivalent code worked correctly (successfully delivered the message -- including the comma in the recipient's display-name) with the v2 send API.

I'm aware the v3 send docs indicate you can't include commas or semicolons (or -- erroneously, periods) in recipient names. But there are many legitimate cases where a display-name would include a comma. And since commas are allowed (and work) in the from_email name, it's confusing that the v3 API wouldn't be properly quoting recipient display-names like the v2 API did.

Steps to Reproduce

  1. Test with v3 API:

    pip install sendgrid==3.6.3 (current version as of this report) and run this code:

    import os
    import sendgrid
    from sendgrid.helpers.mail import Mail, Email, Content
    
    sg = sendgrid.SendGridAPIClient(apikey=os.environ.get('SENDGRID_API_KEY'))
    mail = Mail(
        from_email=Email("[email protected]", "Widgets, Inc."),
        to_email=Email("[email protected]", "Mr. Craig Kaes, SVP Eng."),
        subject="Sent using v3 API",
        content=Content("text/plain", "Body"),
    )
    response = sg.client.mail.send.post(request_body=mail.get())
    print(response.status_code, response.body)  # (202, '')

    Results: (202, '') (send accepted; no apparent error)

  2. Find the message in your SendGrid activity feed.

    Results: The message has been dropped (and displays the confusing email address "mr.craig"):
    screen shot 2017-01-17 at 11 58 29 am
    Expected: The message should have been processed and delivered

  3. Now remove the comma from the to_email -- but leave it in the from_email, and send again.

    Results: Message is processed and delivered, with the comma intact in the from_email display-name. (This demonstrates that the v3 API correctly handles commas in names in some cases. So the code to properly escape and quote names must exist somewhere in the v3 API.)

  4. Now test with the v2 send API:

    pip install sendgrid==2.2.1 (last release using v2 send) and run this code -- the equivalent of the v3 example above:

    import os
    import sendgrid
    
    sg = sendgrid.SendGridClient(os.environ.get('SENDGRID_API_KEY'))
    message = sendgrid.Mail(
        from_email="[email protected]",
        from_name="Widgets, Inc.",
        to="[email protected]",
        to_name="Mr. Craig Kaes, SVP Eng.",
        subject="Sent using v2 API",
        text="Body",
    )
    status, msg = sg.send(message)
    print(status, msg)  # (200, '{"message":"success"}')

    Results: The message is processed and delivered, with the comma in the recipient's display-name intact. The activity feed confirms:
    screen shot 2017-01-17 at 12 11 14 pm

Technical details:

  • sendgrid-python Version: as identified in examples above
  • Python Version: 2.7.12
medmunds added a commit to anymail/django-anymail that referenced this issue Jan 18, 2017
Add workaround for lack of proper recipient name quoting
in SendGrid v3 mail send API. (See sendgrid/sendgrid-python#291.)
Only affects recipient display-names -- not from_email or reply_to.

Also add (undocumented) setting to disable workaround,
if/when SendGrid fixes the bug.
@medmunds
Copy link
Author

(I see this has also been reported against other SendGrid client libs as sendgrid/sendgrid-csharp#268 and sendgrid/sendgrid-ruby#77.)

@thinkingserious
Copy link
Contributor

Hi @medmunds,

Thank you for taking the time to provide these details!

I will pass this along to our product team for possible implementation.

With Best Regards,

Elmer

@thinkingserious thinkingserious added the type: question question directed at the library label Jan 24, 2017
@sbglasius
Copy link

We just got hit by the same error. This is definitely a bug!

@thinkingserious
Copy link
Contributor

@sbglasius,

I have added your vote to the issue.

@sbglasius
Copy link

Just realized I should mention, that we are using the SendGrid Java API client, and not the python one. So the error must be in the SendGrid API not in the client libraries

@thinkingserious
Copy link
Contributor

@sbglasius,

You are correct, this is an API level issue and the appropriate PMs are aware of the issue. I have added your vote to that issue for you. Thanks!

@medmunds
Copy link
Author

FYI, the workaround suggested in the C# issue seems to work: you can quote the recipient names yourself, before handing off to SendGrid. In Python, you'd want to use email.utils.quote. To/cc/bcc names only -- the API handles from and reply-to names correctly.

(Of course, if/when SendGrid fixes the API-side issue, this could result in extra quotes in recipient names until you remove the workaround.)

@thinkingserious
Copy link
Contributor

@medmunds,

Thanks for sharing the work around :)

Here is what I recommend as an interim fix ( hopefully someone awesome will create a PR ;) ): sendgrid/sendgrid-php#368

@sreeram123
Copy link

hi

@prabhakar267
Copy link

I would like to work on this
What I understood from reading both of the issue threads (this and PHP), the client side fix that worked for PHP was cleaning the strings from any double quotes and passing the string in single quotes
I am not sure if that is going to work in Python as well or not. But does this issue wants only a cleaning function or have I missed something? 😄
/cc @thinkingserious

@mbernier mbernier removed difficulty: medium fix is medium in difficulty difficulty: very hard fix is very hard in difficulty labels Oct 27, 2017
@medmunds
Copy link
Author

It's, uh... surprising that SendGrid would want to implement separate workarounds in every client library rather than fix the underlying API bug. (The current API behavior suggests that SendGrid's servers may be assembling messages by concatenating raw API input strings, without properly escaping them for use in email headers -- and there are all sorts of things can go wrong with that approach. Plus, all the client-side workarounds will later need to be removed when the API does get fixed.)

But having said that, if you do want to add a workaround to sendgrid-python, the goal would be to "clean" the email["name"] output from sendgrid.helpers.mail.Email.get() when that Email is being used in a sendgrid.helpers.mail.Personalization (to/cc/bcc), but not alter the name when an Email is used in Mail.from_email or Mail.reply_to (since the API already handles those name fields properly).

The "cleaning" function could be either:

  • Stripping commas and semicolons (and possibly double quotes) from the Email's name. This is what SendGrid's docs recommend, but has the downside of not allowing commas or semicolons in recipients' displayed names.

  • OR use Python's email.utils.quote to properly escape the name for use in an email header. This preserves display names in the resulting email, and has the advantage of using a standard library function to do the string manipulation -- one that's documented and tested specifically for constructing email headers.

(I personally think the second approach is better, but that's just my opinion. FWIW, it's what I used in a library I maintain that calls the SendGrid API directly. But I also included a setting to disable this behavior if/when the API gets fixed.)

@gabrielkrell
Copy link
Contributor

@medmunds, agreed that email.utils.quote seems like the best way to deal with this for now. According to Elmer there's an API fix coming at some point, but might as well fix it here for the time being.

@kirk-quinbar
Copy link

we just switched to V3 api from SMTP and found this out too, that commas in the To address name field result in a dropped email. bad..

@thinkingserious
Copy link
Contributor

Thanks for taking the time to provide feedback @kirk-quinbar, I have passed along your feedback to our product team.

With Best Regards,

Elmer

@eshanholtz
Copy link
Contributor

Closing this as it has been marked a non-library issue.

@medmunds
Copy link
Author

medmunds commented Aug 7, 2024

This seems to have been fixed at the API level, based on testing on 2024-08-07. (No idea how long ago it changed.)

I can't tell if we should count on the fix, because the problem is still mentioned under "Limitations" in SendMail's Mail Send API Overview docs:

The to.name , cc.name , and bcc.name personalizations cannot include either the ; or , characters.

(Also, it's unclear where to report issues with SendGrid's API itself or its documentation, which is why they end up reported as library issues like this one.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: unknown or n/a fix is unknown in difficulty status: help wanted requesting help from the community type: non-library issue API issue not solvable via the SDK
Projects
None yet
Development

No branches or pull requests

10 participants