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 fails: three ways #368

Closed
cbschuld opened this issue Mar 17, 2017 · 9 comments
Closed

Email fails: three ways #368

cbschuld opened this issue Mar 17, 2017 · 9 comments
Labels
difficulty: medium fix is medium in difficulty status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library type: non-library issue API issue not solvable via the SDK

Comments

@cbschuld
Copy link
Contributor

I have found three ways to get the v3 API to essentially fail. The API creates the email and attempts to send it to some random name instead of the email address indicated by the caller. Furthermore the caller does get a 202 return code from the API and immediately receives a "drop" event from the event subsystem/hook. Candidly I feel this is a direct bug with the API but I think we can modify the SDK to deal with the shortcomings of the API IMO. (also, I am horribly impatient because it's impacting our app)

I created six PHPUnit tests to illustrate the failure. I will replace my email(s) with VALID_EMAIL_A and VALID_EMAIL_B for sanity.

If you create the following six tests:

    public function testSimpleEmail()
    {
        $sendGrid = new SendGrid(self::SG_KEY);
        $email = new SendGrid\Mail();

        $from = new SendGrid\Email('TEST 1',VALID_EMAIL_A);
        $email->setFrom($from);

        $personalization = new SendGrid\Personalization();
        $personalization->addTo(new SendGrid\Email('CHRIS SCHULD', VALID_EMAIL_B));
        $email->addPersonalization($personalization);

        $email->setSubject('TEST 1');

        $content = 'this is the content for <strong>test 1</strong>';

        $contentText = new SendGrid\Content('text/plain', $content);
        $contentHtml = new SendGrid\Content('text/html',strip_tags($content));

        $email->addContent($contentText);
        $email->addContent($contentHtml);

        $email->setIpPoolName('transactional');

        $response = $sendGrid->client->mail()->send()->post($email);

        $this->assertEquals(202, $response->statusCode());

    }

    public function testWithApostropheInToName()
    {
        $sendGrid = new SendGrid(self::SG_KEY);
        $email = new SendGrid\Mail();

        $from = new SendGrid\Email('TEST 2',VALID_EMAIL_A);
        $email->setFrom($from);

        $personalization = new SendGrid\Personalization();
        $personalization->addTo(new SendGrid\Email("Chris O'Brian", VALID_EMAIL_B));
        $email->addPersonalization($personalization);

        $email->setSubject('TEST 2');

        $content = 'this is the content for <strong>test 2</strong>';

        $contentText = new SendGrid\Content('text/plain', $content);
        $contentHtml = new SendGrid\Content('text/html',strip_tags($content));

        $email->addContent($contentText);
        $email->addContent($contentHtml);

        $email->setIpPoolName('transactional');

        $response = $sendGrid->client->mail()->send()->post($email);

        $this->assertEquals(202, $response->statusCode());

    }

    public function testWithJsonEncodedName()
    {
        $json = json_decode(json_encode(['name'=>"Chris O'Brian"]));

        $sendGrid = new SendGrid(self::SG_KEY);
        $email = new SendGrid\Mail();

        $from = new SendGrid\Email('TEST 3',VALID_EMAIL_A);
        $email->setFrom($from);

        $personalization = new SendGrid\Personalization();
        $personalization->addTo(new SendGrid\Email($json->name, VALID_EMAIL_B));
        $email->addPersonalization($personalization);

        $email->setSubject('TEST 3');

        $content = 'this is the content for <strong>test 3</strong>';

        $contentText = new SendGrid\Content('text/plain', $content);
        $contentHtml = new SendGrid\Content('text/html',strip_tags($content));

        $email->addContent($contentText);
        $email->addContent($contentHtml);

        $email->setIpPoolName('transactional');

        $response = $sendGrid->client->mail()->send()->post($email);

        $this->assertEquals(202, $response->statusCode());

    }

    public function testWithEncodingsInName()
    {
        $json = json_decode(json_encode(['name'=>"Chris O&#39;Brian"]));

        $sendGrid = new SendGrid(self::SG_KEY);
        $email = new SendGrid\Mail();

        $from = new SendGrid\Email('TEST 4',VALID_EMAIL_A);
        $email->setFrom($from);

        $personalization = new SendGrid\Personalization();
        $personalization->addTo(new SendGrid\Email($json->name, VALID_EMAIL_B));
        $email->addPersonalization($personalization);

        $email->setSubject('TEST 4');

        $content = 'this is the content for <strong>test 4</strong>';

        $contentText = new SendGrid\Content('text/plain', $content);
        $contentHtml = new SendGrid\Content('text/html',strip_tags($content));

        $email->addContent($contentText);
        $email->addContent($contentHtml);

        $email->setIpPoolName('transactional');

        $response = $sendGrid->client->mail()->send()->post($email);

        $this->assertEquals(202, $response->statusCode());

    }

    public function testWithCommaAndPeriodInName()
    {
        $sendGrid = new SendGrid(self::SG_KEY);
        $email = new SendGrid\Mail();

        $from = new SendGrid\Email('TEST 5',VALID_EMAIL_A);
        $email->setFrom($from);

        $personalization = new SendGrid\Personalization();
        $personalization->addTo(new SendGrid\Email('Brian, Chris.', VALID_EMAIL_B));
        $email->addPersonalization($personalization);

        $email->setSubject('TEST 5');

        $content = 'this is the content for <strong>test 5</strong>';

        $contentText = new SendGrid\Content('text/plain', $content);
        $contentHtml = new SendGrid\Content('text/html',strip_tags($content));

        $email->addContent($contentText);
        $email->addContent($contentHtml);

        $email->setIpPoolName('transactional');

        $response = $sendGrid->client->mail()->send()->post($email);

        $this->assertEquals(202, $response->statusCode());

    }

    public function testWithCommaInName()
    {
        $sendGrid = new SendGrid(self::SG_KEY);
        $email = new SendGrid\Mail();

        $from = new SendGrid\Email('TEST 6',VALID_EMAIL_A);
        $email->setFrom($from);

        $personalization = new SendGrid\Personalization();
        $personalization->addTo(new SendGrid\Email('Brian, Chris', VALID_EMAIL_B));
        $email->addPersonalization($personalization);

        $email->setSubject('TEST 6');

        $content = 'this is the content for <strong>test 6</strong>';

        $contentText = new SendGrid\Content('text/plain', $content);
        $contentHtml = new SendGrid\Content('text/html',strip_tags($content));

        $email->addContent($contentText);
        $email->addContent($contentHtml);

        $email->setIpPoolName('transactional');

        $response = $sendGrid->client->mail()->send()->post($email);

        $this->assertEquals(202, $response->statusCode());

    }

You'll see that you VALID_EMAIL_B receives an email from VALID_EMAIL_A in test cases 1, 2 and 3. You will NOT receive an email from test cases 4, 5 or 6. In cases 4, 5, and 6 you'll see receive a hit from the DROPPED webhook.

This is completely reproducible from the test cases above.

I have filed these examples with Evan at priority support as well. It is impacting a production app we use so I am going to work on a work around but I want to see where I am screwing up OR where the API is deficient.

@cbschuld
Copy link
Contributor Author

FYI if I run my "names" through this nasty filter:

$name = str_replace([',', '.'], '', html_entity_decode($recipient['name'], ENT_QUOTES));

it works great... I am really stuck as to where this should be addressed? This seems more like an API bug than an issue with the SDK although I'll probably submit a PR to address it but commas and periods are not exactly bad characters for email names.

@thinkingserious
Copy link
Contributor

Hello @cbschuld,

This is currently being addressed at the API level, but I don't have any timeline. I have added your voice to the chorus to help bump the priority.

My suggestion is to implement a helper method that we point people to via an error message that we throw when an offending character is detected.

A PR would definitely be appreciated :)

@thinkingserious
Copy link
Contributor

Related: sendgrid/sendgrid-python#291

@cbschuld
Copy link
Contributor Author

@thinkingserious - Elmer, understood, thanks for the update.

@cbschuld
Copy link
Contributor Author

cbschuld commented Mar 18, 2017

@thinkingserious - I am not sold that I should toss a PR into this - here is why (assuming in some short time period the API can be "fixed" - BTW, would see this as a massive runtime issue I hope they are stepping on it - I'll send cookies or cake if that helps)

Email "names" are completely okay in at least the US to be listed like this:

Smith, Robert

I am not suggesting that is a nice personal way to have your name in an email but with the API bug that email will look like this:

Smith, Robert <robert@nevergettingthisemail.com>

The email will actually be gobbled up by the API as

smith

Which, IMO is an issue solely with the API and applying

$name = str_replace([',', '.'], '', html_entity_decode($recipient['name'], ENT_QUOTES));

seems lame

@cbschuld
Copy link
Contributor Author

@thinkingserious - this fixes it...
sendgrid/sendgrid-csharp#268 (comment)

I'll submit a PR

@cbschuld
Copy link
Contributor Author

@thinkingserious - PR - #370

@thinkingserious thinkingserious added the type: non-library issue API issue not solvable via the SDK label Apr 28, 2017
@kikosoft
Copy link

Just want to say I also ran into this problem with the email name, and I am now using a filter that is very similar to that of @cbschuld.

What bothers me slightly is that this is such a basic issue, and it has been known about for months. I wasted an hour of my time tracking the problem down and solving it. Why hasn't this been addressed? I don't care how... library or API... either will do.

@thinkingserious
Copy link
Contributor

Thanks for the feedback @kikosoft, I have added your vote to the issue.

This has not been addresses because it has not reached the top of our backlog as an issue. It did reach the top of our backlog as a PR (see #370 ), but that PR stalled. While votes help increase priority, PR's provide the most weight.

With Best Regards,

Elmer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium fix is medium in difficulty status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library type: non-library issue API issue not solvable via the SDK
Projects
None yet
Development

No branches or pull requests

4 participants