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

Name part in setFrom is wrapped in double quotes twice #897

Closed
bokibxs opened this issue Oct 22, 2019 · 5 comments · Fixed by #1006
Closed

Name part in setFrom is wrapped in double quotes twice #897

bokibxs opened this issue Oct 22, 2019 · 5 comments · Fixed by #1006
Labels
type: community enhancement feature request not on Twilio's roadmap

Comments

@bokibxs
Copy link

bokibxs commented Oct 22, 2019

Issue Summary

Name part in setFrom is wrapped in double quotes twice, which results in incorrect From header and iOS Mail app not displaying from name properly.

Steps to Reproduce

Copy/paste Hello Email use case from here:
https://github.com/sendgrid/sendgrid-php
and replace this line:
$email->setFrom("[email protected]", "Example User");
with this:
$email->setFrom("[email protected]", "Text, With, Commas");
and output JSON before send call to see request body:
echo json_encode($email, JSON_PRETTY_PRINT);
and you'll get following:

{
    "personalizations": [
        {
            "to": [
                {
                    "name": "Example User",
                    "email": "[email protected]"
                }
            ]
        }
    ],
    "from": {
        "name": "\"Text, With, Commas\"",
        "email": "[email protected]"
    },
    "subject": "Sending with Twilio SendGrid is Fun",
    "content": [
        {
            "type": "text\/plain",
            "value": "and easy to do anywhere, even with PHP"
        },
        {
            "type": "text\/html",
            "value": "<strong>and easy to do anywhere, even with PHP<\/strong>"
        }
    ]
}

If you update emails and API key and send this out, this will result in From header name part like this:
From: "\"Text, With, Commas\""
And iOS Mail app will render that as attached.

IMG_0138

Technical details:

  • sendgrid-php Version: 7.3.0
  • PHP Version: 7.2
@Harumaro
Copy link

See #368
https://github.com/sendgrid/sendgrid-php/blob/master/lib/mail/EmailAddress.php#L116

/*
            Issue #368
            ==========
            If the name is not wrapped in double quotes and contains a comma or
            semicolon, the API fails to parse it correctly.
            When wrapped in double quotes, commas, semicolons and unescaped single
            quotes are supported.
            Escaped double quotes are supported as well but will appear unescaped in
            the mail (e.g. "O\'Keefe").
            Double quotes will be shown in some email clients, so the name should
            only be wrapped when necessary.
        */

@bokibxs
Copy link
Author

bokibxs commented Oct 29, 2019

Yep problem is that it is wrapped twice in double quotes, so instead of name resulting in:
"name": "Text, With, Commas",
It results in:
"name": "\"Text, With, Commas\"",

@childish-sambino
Copy link
Contributor

I took out the changes made in #511 and emails send fine. Checking to see if/when this was fixed on the back-end before committing to submitting a PR to revert the changes.

@childish-sambino childish-sambino added difficulty: easy fix is easy in difficulty status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library labels Mar 20, 2020
@childish-sambino
Copy link
Contributor

After checking with support turns out the original bug does not impact all customers/traffic, so I'm hesitant to take the double-quote wrapping fix out.

Another option is to make this configurable in the client so that you can make adjustments if you know it's going to impact you or not.

I'll circle back here if it ever gets completely fixed on the backend so this code can be ripped out, but open to reviewing a PR if anyone wants to implements the option above.

@childish-sambino childish-sambino added status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap and removed difficulty: easy fix is easy in difficulty status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library labels Mar 20, 2020
@iant-ee
Copy link

iant-ee commented Jun 2, 2020

The original ticket that introduced this bug (#368) acknowledges that this is an API bug that only applies to V3, and @thinkingserious says that it is being fixed at the API level (but hadn't yet been, when 368 was being worked on).

Nowhere on #368, #370 or #511 do I see any mention of these quotes showing up in users email clients. I'd therefore suggest that at some point between those tickets being worked on and this ticket being filed that the underlying API bug was fixed, and this workaround should be reverted.

@childish-sambino Do you have any more information about which customers it does / does not impact? Is there a public bug report for the API-level bug? The response from support sounds like someone a support team might say if they knew that had seen a bug previously but couldn't reproduce it and hadn't been told it had been fixed.

@childish-sambino childish-sambino added status: work in progress Twilio or the community is in the process of implementing and removed status: help wanted requesting help from the community labels Sep 23, 2020
@childish-sambino childish-sambino removed the status: work in progress Twilio or the community is in the process of implementing label Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants