Implement legacy RFC821 connections

[GwendolenLynch]

As per everything with me, feel free to request changes or flat out reject. :+1:

Changes:

• Add Protocol\Smtp\Negotiation\TryRfc821UpgradeNegotiation
• Add Protocol\Smtp\Request\HeloCommand
• Add Protocol\Smtp\Reply#isCommandNotImplemented() method
• Add RFC821 "connection allowed" parameter & setter method in ClientFactory
  • Enabled via 'rfc821=true' query parameter in factory string
  • TryRfc821UpgradeNegotiation adds to $negotiators[]
• Unit tests
  • Add HELO response & legacy flag paramter to FakeSmtpConnection
  • Add Protocol\Smtp\Negotiation\TryRfc821UpgradeNegotiationTest
  • Add Protocol\Smtp\Request\HeloCommandTest
  • Test Protocol\Smtp\Reply#isCommandNotImplemented()
  • Test Protocol\Smtp\ClientFactory withRfc821ConnectionAllowed() & newClient() additions

Use-cases

• Testing, e.g. python -m smtpd -n -c DebuggingServer localhost:25
• Legacy lock-in :disappointed:

Using

$transport = new SmtpTransport(
    ClientFactory::fromString('smtp://localhost:25?rfc821=true')->newClient(),
    EnvelopeFactory::useExtractedHeader()
);

[frederikbosch]

Looks good at first hand. One remark, I am against setters/mutability on classes like FakeSmtpConnection. I'd rather see a named constructor FameSmtpConnection::newLegacyRfc821() which creates a new client and sets the boolean to true.

[frederikbosch]

Maybe we should also add this to the server end, with an example in the example folder. Then there is no need to run Python to test it.

[GwendolenLynch]

I'd rather see a named constructor FameSmtpConnection::newLegacyRfc821() which creates a new client and sets the boolean to true.

:+1:

Maybe we should also add this to the server end, with an example in the example folder. Then there is no need to run Python to test it.

I'll look into it.

[frederikbosch]

Also, add a link to the RFC for the README.

[GwendolenLynch]

Also, add a link to the RFC for the README.

~I just added a "dump" of the SMTP RFCs to the README, specifically:~

[GwendolenLynch]

I just added a "dump" of the SMTP RFCs to the README,

That moment you scroll down … just a little further … and your head meets the desk.

[frederikbosch]

Why did you choose to add a new negotiation and an additional configuration directive in dsn factory? Why not add this to TryTlsUpgradeNegotiation directly? Then it will work out of the box for everyone.

[GwendolenLynch]

The short version was that I was unsure as to your architectural direction in that regard. If you're happy with that approach, it would be my preferred one too. I'll update.

[GwendolenLynch]

Done. AppVeyor seems to have had a tantrum over Windows MSI installs, unrelated to these changes as best I can tell.

[frederikbosch]

Yeah, it seems that the upgrade to PHP 7.1.13 breaks AppVeyor. No idea why. I am not a specialist on AppVeyor/Windows.

[frederikbosch]

Thanks again!