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

MailSender doesn't correctly set JavaMail properties for SMTPS connections #111

Closed
cbarcenas opened this issue Nov 2, 2017 · 6 comments
Closed

Comments

@cbarcenas
Copy link
Contributor

cbarcenas commented Nov 2, 2017

Several of the configuration methods in MailSender incorrectly set JavaMail properties under the mail.smtp prefix when smtps mail transport (TransportStrategy.SMTP_SSL) is used. I'm pretty sure the mail.smtps prefix should be used instead.

Examples: [1] [2] [3] [4]

The cleanest fix, I think, is to implement additional abstract property name getters under the TransportStrategy enum.

@cbarcenas cbarcenas changed the title MailSender properties aren't correctly set for SMTPS connections MailSender doesn't correctly set JavaMail properties for SMTPS connections Nov 2, 2017
@bbottema
Copy link
Owner

bbottema commented Nov 3, 2017

Indeed, quoting javaee:

Note that if you're using the "smtps" protocol to access SMTP over SSL, all the properties would be named "mail.smtps.*".

I agree, these properties will need to come from the TransportStrategy enum. Strange that I overlooked this. Let's fix this asap.

@cbarcenas
Copy link
Contributor Author

Now that I think about it, why don't we scrap the per-property interface and instead add a single propertyNamePrefix() method to each enum type?

@bbottema
Copy link
Owner

bbottema commented Nov 3, 2017

I like the flexibility it gives me in case the property is completely different per transport strategy. Of course this will never happen with this decades old specification, so it is kind of moot.

However, I like this design, it's clear enough (and I've already updated it).

@cbarcenas
Copy link
Contributor Author

Cool, I'm pretty indifferent either way

@bbottema
Copy link
Owner

bbottema commented Nov 3, 2017

Actually, this was rather tricky, since the transport strategy is an optional parameter in favor of a preconfigured provided Session object.

In one type of scenario an exception was in order, in a second skipping defaults and in a third skipping properties logging they were assumed to be preconfigured.

bbottema added a commit that referenced this issue Nov 3, 2017
…rt Strategy while also handle various scenario's where Transport Strategy was not provided
@bbottema bbottema closed this as completed Nov 3, 2017
@bbottema bbottema added this to the 4.5.0 milestone Nov 3, 2017
@bbottema
Copy link
Owner

Released as 5.0.0.rc1-SNAPSHOT. Add OSS' snapshots repo to find it in Maven.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants