Problem/Motivation

Add options for the SMTP transport type.

See https://symfony.com/doc/current/mailer.html#tls-peer-verification and https://symfony.com/doc/current/mailer.html#other-options

Issue fork symfony_mailer-3256740

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

JeroenT created an issue. See original summary.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

As a workaround, you can create a mailer transport of the type "DSN" and fill in something like smtp://localhost:25?verify_peer=0

adamps’s picture

adamps commented
Category: Task » Feature request

Thanks for the issue, good idea. This could be added as an option in SmtpTransport.

Mschudders made their first commit to this issue’s fork.

Mschudders opened merge request !23

mschudders’s picture

mschudders commented

added the option for verify_peer.

Will report back after testing, but local testing seems good.

adamps’s picture

adamps commented
Status: Active » Needs work

Many thanks for the patch. I have some comments (some in the MR, some here that are additions)

  1. Needs an update to symfony_mailer.schema.yml
  2. Needs an update hook in symfony_mailer.install
  3. There is a @todo comment in SmtpTransport at line 29. Please delete the line that you have done.
  4. It will be a bit tedious to have a separate issue and hook to fix each of these 5 - any chance you might add them all? (Could even skip the form, and just do the schema update.)

nrogers made their first commit to this issue’s fork.

nrogers’s picture

nrogers commented

I have made the requested changes and also changed what I think was unintended behavior in the TransportBase getDsn function since it was adding an '@' when username and pass are empty.

nrogers’s picture

nrogers commented

Also, I'm not entirely sure it's necessary to update the schema if the new options are in the defaultConfiguration() function since setConfiguration is called upon plugin initialization and the default configuration is merged with the current configuration. But I could be wrong.

nrogers’s picture

nrogers commented
Status: Needs work » Needs review
solideogloria’s picture

solideogloria commented
Version: 1.0.0-alpha3 » 1.x-dev
adamps’s picture

adamps commented
Title: Add an option to disable peer verification for SMTP » Add options to SMTP transport
Issue summary: View changes
Status: Needs review » Needs work

Looks very good thanks. I added some more comments, however they are now all minor and should be very quick.

Also, I'm not entirely sure it's necessary to update the schema

The schema is needed thanks - it is separate from the idea of default values. It provides metadata for config information, see docs. The config_inspector module is useful to test the schema is correct.

nrogers’s picture

nrogers commented
Status: Needs work » Needs review

Ok, requested changes made as well as some other minor form changes for consistency and to follow the form elements UI standards.

nrogers’s picture

nrogers commented

Also, I meant the schema update hook to resave all the smtp transports, not the schema modification itself, sorry I wasn't very clear there. It doesn't hurt to just ensure all the values are present either way, I just wasn't sure if it mattered if existing smtp transport configs were updated with the new default values or not. Thanks Adam!

  • AdamPS committed a3a939b on 1.x authored by Mschudders 
    Issue #3256740: Add an option to disable peer verification for SMTP
adamps’s picture

adamps commented
Status: Needs review » Fixed

Great many thanks

I just wasn't sure if it mattered if existing smtp transport configs were updated with the new default values or not

I see you might be right, although I have a vague memory of warnings in the past if it's skipped. Anyway it's good as it is, definitely safe.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.