Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Protocol\Smtp: toggle QUIT>221 command at disconnect #117

Closed
wants to merge 2 commits into from

Conversation

Slamdunk
Copy link
Contributor

@Slamdunk Slamdunk commented Oct 12, 2016

Original issue: #27

When a SMTP server implements a timeout, a script like this is a pain in the ass:

$transport = new Zend\Mail\Transport\Smtp();

$message = new Zend\Mail\Message();
$message->setFrom('test@test.com');
$message->addTo('test@test.com');
$message->setSubject('Test ' . date('H:i:s'));
$message->setBody('Test ' . date('H:i:s'));

$transport->send($message);
var_dump('E-mail sent');

sleep(305);

var_dump('Soon to exit...');
exit;

// E-mail sent
// Soon to exit...
// Notice: fwrite(): send of 6 bytes failed with errno=32 Broken pipe in ./zend-mail/src/Protocol/AbstractProtocol.php on line 255
// Fatal error: Uncaught Zend\Mail\Protocol\Exception\RuntimeException: Could not read from 127.0.0.1 in ./zend-mail/src/Protocol/AbstractProtocol.php:301

While the other features mentioned in #27 can be implemented in a custom way using OOP, this one cannot.

If a user wants to use Protocol\Smtp and Protocol\Smtp\Auth\Login, the fact that:

  1. quit() is hardcoded into Protocol\Smtp::_disconnect()
  2. Protocol\Smtp::_disconnect() is hardcoded into Protocol\AbstractProtocol::__destruct()

Makes impossible to customize zendframework/zend-mail component, even with subclassing, without rewriting the core code lines of Zend\Protocol\*

With this PR a user can decide wether or not send the QUIT at __destruct / exit:

$transport = new Zend\Mail\Transport\Smtp(new Zend\Mail\Transport\SmtpOptions(array(
    'connection_class'  => 'smtp', // or plain, login, crammd5
    'connection_config' => [
        'use_complete_quit' => false,
    ],
)));

As soon as the PR is accepted, I'll PR to the doc.

@Slamdunk
Copy link
Contributor Author

@Maks3w does the change sound applicable this way?

@Slamdunk
Copy link
Contributor Author

Ping anyone?

@weierophinney
Copy link
Member

This looks good. I'll merge to develop for release with 2.8.0 (which will likely be a couple weeks, as there are a few other pending items that I'm waiting for author feedback on). I'll take care of the conflict during merge.

weierophinney added a commit that referenced this pull request Feb 14, 2017
Protocol\Smtp: toggle QUIT>221 command at disconnect

Conflicts:
	test/Protocol/SmtpTest.php
weierophinney added a commit that referenced this pull request Feb 14, 2017
weierophinney added a commit that referenced this pull request Feb 14, 2017
@weierophinney
Copy link
Member

Merged to develop for release with 2.8.0.

@Slamdunk Slamdunk deleted the set-complete-quit branch February 15, 2017 07:49
Slamdunk added a commit to Slamdunk/zend-mail that referenced this pull request Feb 15, 2017
Slamdunk added a commit to Slamdunk/zend-mail that referenced this pull request Feb 15, 2017
@Slamdunk
Copy link
Contributor Author

Here the doc against develop: #131

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

Successfully merging this pull request may close these issues.

3 participants