zendframework / zend-mail

Mail component from Zend Framework
BSD 3-Clause "New" or "Revised" License
96 stars 111 forks source link

addCc() not delivering. #48

Open SBC-Fujian opened 8 years ago

SBC-Fujian commented 8 years ago

The recipient listed in addCc() doesn't get the email, although the person who is in addTo() can get the email and can even see the addCc recipient's name is listed in Cc field.

We are using SMTP on IBM Domino mail server.

Tested "Cc" using pure php mail() function, it worked.

This is most liked caused by the "\r\n" in the header.

asgrim commented 8 years ago

It's caused by using LF \n instead of the expected CRLF \r\n. This can be made to work by forcing the result of \Zend\Mail\Transport\Sendmail::isWindowsOs() to return true.

My suggestion as a workaround is simply extend the Zend\Mail Sendmail transport and override that method, something like this:

<?php

namespace App\Mail\Transport;

use Zend\Mail\Transport\Sendmail as ZendSendmail;

class Sendmail extends ZendSendmail
{
    protected function isWindowsOs()
    {
        return true;
    }
}

As for fixing permanently in Zend\Mail, I'd propose:

    protected function isWindowsOs()
    {
        if (!$this->operatingSystem) {
            $this->operatingSystem = strtoupper(substr(PHP_OS, 0, 3));
        }
        //return ($this->operatingSystem == 'WIN');
        return ($this->operatingSystem === 'WIN') || ($this->operatingSystem === 'AIX');
    }

but not sure if this affects other mail sending on IBM i...

akrabat commented 8 years ago

I'm slightly confused. RFC 2822, section 2.2 states:

Header fields are lines composed of a field name, followed by a colon (":"), followed by a field body, and terminated by CRLF.

Surely Zend\Mail should be using \r\n in all cases?

asgrim commented 8 years ago

@akrabat see comment in the Sendmail class:

// On *nix platforms, we need to replace \r\n with \n
// sendmail is not an SMTP server, it is a unix command - it expects LF
if (!$this->isWindowsOs()) {
    $to      = str_replace("\r\n", "\n", $to);
    $subject = str_replace("\r\n", "\n", $subject);
    $body    = str_replace("\r\n", "\n", $body);
    $headers = str_replace("\r\n", "\n", $headers);
}
akrabat commented 8 years ago

Interesting as the manual page for mail() says:

additional_headers (optional) String to be inserted at the end of the email header.

This is typically used to add extra headers (From, Cc, and Bcc). Multiple extra headers should be separated with a CRLF (\r\n). If outside data are used to compose this header, the data should be sanitized so that no unwanted headers could be injected.

Though there is a note:

If messages are not received, try using a LF (\n) only. Some Unix mail transfer agents (most notably » qmail) replace LF by CRLF automatically (which leads to doubling CR if CRLF is used). This should be a last resort, as it does not comply with » RFC 2822.

It feels like we're doing it wrong by sending \n to mail() under any OS…

asgrim commented 8 years ago

Using the Smtp transport solves this problem... I think that's the safer alternative for this platform :)

michalbundyra commented 4 years ago

This repository has been closed and moved to laminas/laminas-mail; a new issue has been opened at https://github.com/laminas/laminas-mail/issues/68.