genkgo/mail

Server class not correctly handling unknown SMTP commands

GwendolenLynch opened this issue · 4 comments

Test case

  • Note the missing DATA capability
$backend = new DevNullBackend();
$spam = new ForbiddenWordSpamScore([], 0);
$cap = [
    new MailFromCapability(),
    new RcptToCapability($backend),
];

$server = new Server(new PlainTcpConnectionListener('localhost', 2525), $cap, 'localhost');
$server->start();

Result

PHP Fatal error:  Uncaught TypeError: Return value of Genkgo\Mail\Protocol\AbstractConnection::receive() must be of the type string, boolean returned in [..]/src/Protocol/AbstractConnection.php:115
Stack trace:
#0 [..]/src/Protocol/AppendCrlfConnection.php(65): Genkgo\Mail\Protocol\AbstractConnection->receive()
#1 [..]/src/Protocol/TrimCrlfConnection.php(62): Genkgo\Mail\Protocol\AppendCrlfConnection->receive()
#2 [..]/src/Protocol/Smtp/Server.php(63): Genkgo\Mail\Protocol\TrimCrlfConnection->receive()
#3 [..]/test-server.php(27): Genkgo\Mail\Protocol\Smtp\Server->start()
#4 {main}
  thrown in [..]/src/Protocol/AbstractConnection.php on line 115

Additional Details

The return value of fgets in \Genkgo\Mail\Protocol\AbstractConnection::receive() is not checked for a boolean response, and an exception thrown. As the function is type hinted as string, things go 💥

A similar exception to the one thrown in send(), e.g. CannotReadFromStreamException, might be appropriate but still won't cover the handling in the server class itself

This also causes the same fatal when a message gets marked as spam.

Will fix this week. Then I am going to create a new tag, probably 2.3.0.

@GawainLynch I see that receive might return boolean, so that should be fixed. However, I cannot replicate your behaviour. The server I started was the same as in the example with the exception of authentication.

➜  mail git:(master) ✗ telnet localhost 8025
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
220 Welcome to Genkgo Mail Server
EHLO test
250 mail.local Hello test
MAIL FROM <test@genkgo.nl>
250 OK
RCPT TO <mailbox@domain.com>
250 OK
DATA
354 Enter message, ending with "." on a line by itself
Subject: test
Hello: test

viagra viagra viagra viagra
.
550 Message discarded as high-probability spam

If I disable DATA my response is as expected.

DATA
500 unrecognized command

So I guess you are doing something different. Do you have an example session that actually causes trouble? Can you elaborate?

Server script:

#!/usr/bin/env php
<?php

require __DIR__.'/../vendor/autoload.php';

use Genkgo\Mail\Protocol\PlainTcpConnectionListener;
use Genkgo\Mail\Protocol\Smtp\Backend\ConsoleBackend;
use Genkgo\Mail\Protocol\Smtp\Capability\DataCapability;
use Genkgo\Mail\Protocol\Smtp\Capability\MailFromCapability;
use Genkgo\Mail\Protocol\Smtp\Capability\RcptToCapability;
use Genkgo\Mail\Protocol\Smtp\GreyList\ArrayGreyList;
use Genkgo\Mail\Protocol\Smtp\Server;
use Genkgo\Mail\Protocol\Smtp\SpamDecideScore;
use Genkgo\Mail\Protocol\Smtp\SpamScore\FixedSpamScore;

$backend = new ConsoleBackend();
$spam = new FixedSpamScore(0);
$cap = [
    new MailFromCapability(),
    new RcptToCapability($backend),
    new DataCapability($backend, $spam, new ArrayGreyList(), new SpamDecideScore(1, 100)),
];

$server = new Server(new PlainTcpConnectionListener('localhost', 2525), $cap, 'localhost');
$server->start();

Message send script:

#!/usr/bin/env php
<?php

require __DIR__.'/../vendor/autoload.php';

use Genkgo\Mail\FormattedMessageFactory;
use Genkgo\Mail\Header\Cc;
use Genkgo\Mail\Header\From;
use Genkgo\Mail\Header\Subject;
use Genkgo\Mail\Header\To;
use Genkgo\Mail\Protocol\Smtp\ClientFactory;
use Genkgo\Mail\Transport\EnvelopeFactory;
use Genkgo\Mail\Transport\SmtpTransport;

$message = (new FormattedMessageFactory())
    ->withHtml('<html><body><p>Hello World</p></body></html>')
    ->createMessage()
    ->withHeader(new Subject('Hello World'))
    ->withHeader(From::fromEmailAddress('from@example.com'))
    ->withHeader(To::fromSingleRecipient('to@example.com', 'name'))
    ->withHeader(Cc::fromSingleRecipient('cc@example.com', 'name'))
;

$client = ClientFactory::fromString('smtp://localhost:2525')
    ->withInsecureConnectionAllowed()
    ->newClient()
;

$transport = new SmtpTransport($client, EnvelopeFactory::useExtractedHeader());
$transport->send($message);

Result:

Subject: Hello World
From: from@example.com
To: name <to@example.com>
Cc: name <cc@example.com>
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary=GenkgoMailV2Partf680b585a53f

This is a multipart message in MIME format.

--GenkgoMailV2Partf680b585a53f
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hello World

--GenkgoMailV2Partf680b585a53f
Content-Type: text/html; charset=us-ascii
Content-Transfer-Encoding: 7bit

<html><body><p>Hello World</p></body></html>
--GenkgoMailV2Partf680b585a53f--

Subject: Hello World
From: from@example.com
To: name <to@example.com>
Cc: name <cc@example.com>
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary=GenkgoMailV2Partf680b585a53f

This is a multipart message in MIME format.

--GenkgoMailV2Partf680b585a53f
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hello World

--GenkgoMailV2Partf680b585a53f
Content-Type: text/html; charset=us-ascii
Content-Transfer-Encoding: 7bit

<html><body><p>Hello World</p></body></html>
--GenkgoMailV2Partf680b585a53f--

PHP Fatal error:  Uncaught TypeError: Return value of Genkgo\Mail\Protocol\AbstractConnection::receive() must be of the type string, boolean returned in [..]/src/Protocol/AbstractConnection.php:115
Stack trace:
#0 [..]/src/Protocol/AppendCrlfConnection.php(65): Genkgo\Mail\Protocol\AbstractConnection->receive()
#1 [..]/src/Protocol/TrimCrlfConnection.php(62): Genkgo\Mail\Protocol\AppendCrlfConnection->receive()
#2 [..]/src/Protocol/Smtp/Server.php(63): Genkgo\Mail\Protocol\TrimCrlfConnection->receive()
#3 [..]/test-server.php(27): Genkgo\Mail\Protocol\Smtp\Server->start()
#4 {main}
  thrown in [..]/src/Protocol/AbstractConnection.php on line 115

Running both off of genkgo/mail current master branch.

Note that the result is the same fatal if you terminate a telnet session via a ^]\nclose