Skip to content

SmtpMailer: add support for AUTH PLAIN#31

Merged
dg merged 1 commit into
nette:masterfrom
grongor:smtp-add-auth-plain
Jul 31, 2016
Merged

SmtpMailer: add support for AUTH PLAIN#31
dg merged 1 commit into
nette:masterfrom
grongor:smtp-add-auth-plain

Conversation

@grongor

@grongor grongor commented Jun 27, 2016

Copy link
Copy Markdown
Contributor

It's strange that there are some SMTP servers not supporting the LOGIN mechanism but I stumbled upon one ... so here is the fix :)

@JanTvrdik

JanTvrdik commented Jun 27, 2016

Copy link
Copy Markdown
Contributor

I wonder how did you manage accidentally set the executable flag on the file 😄

@JanTvrdik

Copy link
Copy Markdown
Contributor

https://tools.ietf.org/html/rfc2595#section-2.3 may be related.

Comment thread src/Mail/SmtpMailer.php Outdated
$this->write(base64_encode($this->password), 235, 'password');
$authMechanisms = [];
foreach (preg_split("~\r?\n~", $ehloResponse) as $line) {
if (strpos($line, '250-AUTH ') === 0) {

@JanTvrdik JanTvrdik Jun 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this be 250 AUTH? EDIT: https://tools.ietf.org/html/rfc4954#section-4.1 uses both so I don't really know

@grongor

grongor commented Jun 27, 2016

Copy link
Copy Markdown
Contributor Author

... some Windows magic I guess, I don't usually push code on this computer so it got a bit clumsy :D I will try to fix it ...

How should the STARTTLS be related? If it is required and for some reason it fails to activate then the execution stops - the credentials are never sent.

The space after return code is used to indicate the last line of the response. The link you posted is a good example of that.

@grongor

grongor commented Jun 27, 2016

Copy link
Copy Markdown
Contributor Author

Thanks for pointing out the space vs dash issue - I updated the implementation to count with both of them (because the AUTH can also be the last line of the response).

And btw the +x flag was set by PhpStorm on Windows ... I don't know why but it happened again. Weird.

Comment thread src/Mail/SmtpMailer.php Outdated
$authMechanisms = explode(' ', trim($matches[1]));
}

if (array_search('PLAIN', $authMechanisms, true) !== false) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can be simplified with in_array


Also now it uses PLAIN auth whenever possible, shouldn't we prefer LOGIN auth when both plain and login auth are allowed?

@grongor grongor Jun 27, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, I will change it to in_array.

I also though about it and I don't think that there is a reason to prefer LOGIN mechanism over PLAIN. They both offer same security. The PLAIN mechanism will be just a bit faster ... correct me if I'm wrong.

@dg

dg commented Jul 31, 2016

Copy link
Copy Markdown
Member

Thanks

@dg dg merged commit f2bf695 into nette:master Jul 31, 2016
dg pushed a commit that referenced this pull request Jul 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants