Server class not correctly handling unknown SMTP commands

GwendolenLynch commented 6 years ago

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 :boom:

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

GwendolenLynch commented 6 years ago

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

frederikbosch commented 6 years ago

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

frederikbosch commented 6 years ago

@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?

GwendolenLynch commented 6 years ago

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

genkgo / mail