Skip to content

[Mailer] Add a transport that uses php.ini settings for configuration #36131

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

Merged
merged 1 commit into from
Aug 13, 2020

Conversation

l-vo
Copy link
Contributor

@l-vo l-vo commented Mar 18, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR symfony/symfony-docs#14083

This PR aims to allow to use the mailer when sendmail is not on the /usr/sbin directory or when the -bs option is not supported.

@fabpot
Copy link
Member

fabpot commented May 3, 2020

I don't think this should be part of the DSN. That's an internal configuration setting, not related to the transport DSN itself.

@l-vo l-vo force-pushed the allow_custom_sendmail_command branch from 741ed9e to 7d3ba72 Compare May 4, 2020 16:20
@l-vo
Copy link
Contributor Author

l-vo commented May 4, 2020

Usage of a container parameter instead of a dsn option.

@l-vo l-vo force-pushed the allow_custom_sendmail_command branch 12 times, most recently from bfffb9e to f81c0a6 Compare May 7, 2020 14:32
@Nyholm Nyholm mentioned this pull request May 28, 2020
3 tasks
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be rebased as well as config is now done in PHP :)


protected function getSupportedSchemes(): array
{
$supportedShemes = ['native', 'native+sendmail'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we really need the 2 possibiliies? Can't we only support native and do the switch ourselves in the code depending on the platform?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really needed indeed; I added the two options to be consistent with SendmailTransportFactory. But I can only keep native if you prefer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove the native-sendmail scheme, which is not needed IMHO.

@l-vo l-vo force-pushed the allow_custom_sendmail_command branch 4 times, most recently from 8b504d9 to 65cc5d7 Compare June 30, 2020 14:50
@nicolas-grekas
Copy link
Member

Can't this be solved from the outside, aka by providing a sendmail script wrapper instead?

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a note in the CHANGELOG file?

@l-vo l-vo force-pushed the allow_custom_sendmail_command branch from 49b39fa to 04de561 Compare August 13, 2020 13:56
@l-vo
Copy link
Contributor Author

l-vo commented Aug 13, 2020

Updated and CHANGELOG modified

@fabpot
Copy link
Member

fabpot commented Aug 13, 2020

@l-vo Great work as always, can you see if you can add some docs about this feature?

@fabpot
Copy link
Member

fabpot commented Aug 13, 2020

Thank you @l-vo.

@l-vo
Copy link
Contributor Author

l-vo commented Aug 13, 2020

@l-vo Great work as always, can you see if you can add some docs about this feature?

Thank you 🙂 Yes I created my TODO list with the docs for the PR recently merged 😁

@l-vo l-vo deleted the allow_custom_sendmail_command branch August 13, 2020 14:27
fabpot added a commit to symfony/swiftmailer-bundle that referenced this pull request Sep 27, 2020
This PR was merged into the 3.4-dev branch.

Discussion
----------

Use sendmail_path as default value

Quite some time ago, Swiftmailer dropped support for the PHP `mail` function for security reasons and thus forcing you to use either `sendmail` or `smtp`. However, when using `sendmail`, The Swiftmailer Bundle only uses a hard coded command by default, which might not work on all hosting environments out of the box. The PHP `mail` function on the other hand uses the configured `sendmail_path` of the PHP configuration, which most likely will work in the respective hosting environment.

This PR simply uses the configured `sendmail_path` as the default value for the `command` parameter of the bundle configuration. Therefore emails should work again out of the box, just like it was before when Swiftmailer still supported PHP `mail`.

Kind of like the poor-man's version of symfony/symfony#36131 ;)

Commits
-------

31a4fed use sendmail_path as default value
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020
@fabpot fabpot mentioned this pull request Oct 5, 2020
wouterj added a commit to symfony/symfony-docs that referenced this pull request Nov 21, 2020
This PR was submitted for the 5.x branch but it was squashed and merged into the 5.2 branch instead.

Discussion
----------

Complete documentation about mailer integration

- Complete framework config reference with `Mailer` (based on #14087 plus `message_bus` and `headers` that was added after 4.4)
- Add documentation about `native` DSN protocol (symfony/symfony#36131)

Close #14066

Commits
-------

a389dfb Complete documentation about mailer integration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants