8000 [Mailer] Postmark Transport doesn't escape commas in name of Address · Issue #39416 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Mailer] Postmark Transport doesn't escape commas in name of Address #39416

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.

Sign up for GitHub

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

Jump to bottom
Closed
scvandenbraak opened this issue Dec 9, 2020 · 4 comments
Closed

[Mailer] Postmark Transport doesn't escape commas in name of Address #39416

scvandenbraak opened this issue Dec 9, 2020 · 4 comments
Labels
Bug Mailer Mime Status: Needs Review

Comments

@scvandenbraak
Copy link
scvandenbraak commented Dec 9, 2020 •
edited
Loading

This is my first bug report / contribution to Symfony (or any other open source project), so I hope I did it correctly. Otherwise, just let me know.

Symfony version(s) affected: 4.4 - 5.2

Description
The Postmark API expects multiple addresses to be split by a comma. When the name of an Address contains a comma, this comma is not escaped. So the Postmark API thinks it's getting multiple addresses, while it's actually one address with a comma in the name.

How to reproduce
See https://github.com/scvandenbraak/symfony-postmark-mailer-comma-bug for example code and README.

When you run the simple example without a Postmark Server token, you'll see the output below, showing the comma has not been escaped.

string(24) "My, Name <me@domain.tld>"

You will get the error if you run a real example with Postmark:

In PostmarkApiTransport.php line 59:
Unable to send an email: Error parsing 'To': Illegal email address 'My'. It must contain the '@' symbol. (code 300).

Possible Solution
Use quotes around the name. This can be fixed in several places:

  1. Method getPayload in Transport/PostmarkApiTransport.php of symfony/postmark-mailer.
  2. Method stringifyAddresses in Transport/AbstractTransport.php of symfony/mailer.
  3. Method toString in Address.php of symfony/mime.

I would guess option 3 is the best, but that depends on which other components rely on this method and it the extra quotes could brake anything. I can also make a PR when I know where it should be fixed.

Additional context
See API description "Multiple addresses are comma separated." at "To" on https://postmarkapp.com/developer/api/email-api#send-a-single-email.
@stof
Copy link
Member
stof commented Dec 10, 2020

I think the AddressEncoder should escape , in the name, if possible. that would probably be the simplest fix

@stof stof added the Mime label Dec 10, 2020
@scvandenbraak
Copy link
Author

I'm not really familiar with the AddressEncoder, but I looked at the code. Would you suggest adding a method getEncodedName in Address.php (Mime) where a NameAddressEncoder is used? Just like getEncodedAddress uses the IdnAddressEncoder in Address.php. And the getEncodedAddress is used in the method toString.

@fbourigault
Copy link
Contributor

I use the following code to escape such cases.

I'm not 100% sure of it's correctness, but it probably could be used as a starting point.

        $name = preg_replace('/"/u', '\"', $name);
        if (preg_match('/,/u', $name)) {
            $name = sprintf('"%s"', $name);
        }

Note: we can also drop the " in $name instead of escaping. This is what Gmail does regardless of the presence or not of a comma to escape.

@OskarStark
Copy link
Contributor

@fbourigault are you open for a PR including a testcase?

@YaFou YaFou mentioned this issue Jan 17, 2021
Merged
@fabpot fabpot closed this as completed Mar 12, 2021
fabpot added a commit that referenced this issue Mar 12, 2021
@fabpot
bug #39866 [Mime] Escape commas in address names (YaFou)
93467c5 
This PR was merged into the 4.4 branch.

Discussion
----------

[Mime] Escape commas in address names

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #39416
| License       | MIT
| Doc PR        | --
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Never break backward compatibility (see https://symfony.com/bc).
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too.)
 - Features and deprecations must be submitted against branch 5.x.
-->

Before:
```php
$address = new Address('fabien@symfony.com', 'Fabien, Potencier');
$address->toString(); // Fabien, Potencier <fabien@symfony.com> -> Interpreted like two emails
```

After:
```php
$address = new Address('fabien@symfony.com', 'Fabien, Potencier');
$address->toString(); // "Fabien, Potencier" <fabien@symfony.com>
```

Commits
-------

39e9158 [Mime] Escape commas in address names
@ukutaht ukutaht mentioned this issue Jun 6, 2022
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Mailer Mime Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @stof @OskarStark @fbourigault @scvandenbraak @carsonbot
0