Skip to content

Commit

Permalink
Fix issues where emails would have multipart/mixed when they should not.
Browse files Browse the repository at this point in the history 
When sending multi-part emails with no attachments we shouldn't include
the outer multipart/mixed header as it confuses Outlook and causes it to
show the email as having attachments even though there are none.

A bunch of tests need to be adjusted as the empty multipart/mixed
container has been removed.

Fixes #3474
  • Loading branch information
@markstory
markstory committed May 14, 2014
1 parent 2259309 commit b8fa7ce
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 8 deletions.
13 changes: 8 additions & 5 deletions lib/Cake/Network/Email/CakeEmail.php
View file
Expand Up @@ -756,8 +756,10 @@ public function getHeaders($include = array()) {
}

$headers['MIME-Version'] = '1.0';
if (!empty($this->_attachments) || $this->_emailFormat === 'both') {
if (!empty($this->_attachments)) {
$headers['Content-Type'] = 'multipart/mixed; boundary="' . $this->_boundary . '"';
} elseif ($this->_emailFormat === 'both') {
$headers['Content-Type'] = 'multipart/alternative; boundary="' . $this->_boundary . '"';
} elseif ($this->_emailFormat === 'text') {
$headers['Content-Type'] = 'text/plain; charset=' . $this->_getContentTypeCharset();
} elseif ($this->_emailFormat === 'html') {
Expand Down Expand Up @@ -1521,6 +1523,7 @@ protected function _render($content) {
$hasInlineAttachments = count($contentIds) > 0;
$hasAttachments = !empty($this->_attachments);
$hasMultipleTypes = count($rendered) > 1;
$multiPart = ($hasAttachments || $hasMultipleTypes);

$boundary = $relBoundary = $textBoundary = $this->_boundary;

Expand All @@ -1531,15 +1534,15 @@ protected function _render($content) {
$relBoundary = $textBoundary = 'rel-' . $boundary;
}

if ($hasMultipleTypes) {
if ($hasMultipleTypes && $hasAttachments) {
$msg[] = '--' . $relBoundary;
$msg[] = 'Content-Type: multipart/alternative; boundary="alt-' . $boundary . '"';
$msg[] = '';
$textBoundary = 'alt-' . $boundary;
}

if (isset($rendered['text'])) {
if ($textBoundary !== $boundary || $hasAttachments) {
if ($multiPart) {
$msg[] = '--' . $textBoundary;
$msg[] = 'Content-Type: text/plain; charset=' . $this->_getContentTypeCharset();
$msg[] = 'Content-Transfer-Encoding: ' . $this->_getContentTransferEncoding();
Expand All @@ -1552,7 +1555,7 @@ protected function _render($content) {
}

if (isset($rendered['html'])) {
if ($textBoundary !== $boundary || $hasAttachments) {
if ($multiPart) {
$msg[] = '--' . $textBoundary;
$msg[] = 'Content-Type: text/html; charset=' . $this->_getContentTypeCharset();
$msg[] = 'Content-Transfer-Encoding: ' . $this->_getContentTransferEncoding();
Expand All @@ -1564,7 +1567,7 @@ protected function _render($content) {
$msg[] = '';
}

if ($hasMultipleTypes) {
if ($textBoundary !== $boundary) {
$msg[] = '--' . $textBoundary . '--';
$msg[] = '';
}
Expand Down
50 changes: 47 additions & 3 deletions lib/Cake/Test/Case/Network/Email/CakeEmailTest.php
View file
Expand Up @@ -1301,6 +1301,52 @@ public function testSendRenderNoLayout() {
$this->assertNotContains('This email was sent using the CakePHP Framework', $result['message']);
}

/**
* testSendRender both method
*
* @return void
*/
public function testSendRenderBoth() {
$this->CakeEmail->reset();
$this->CakeEmail->transport('debug');

$this->CakeEmail->from('cake@cakephp.org');
$this->CakeEmail->to(array('you@cakephp.org' => 'You'));
$this->CakeEmail->subject('My title');
$this->CakeEmail->config(array('empty'));
$this->CakeEmail->template('default', 'default');
$this->CakeEmail->emailFormat('both');
$result = $this->CakeEmail->send();

$this->assertContains('Message-ID: ', $result['headers']);
$this->assertContains('To: ', $result['headers']);

$boundary = $this->CakeEmail->getBoundary();
$this->assertContains('Content-Type: multipart/alternative; boundary="' . $boundary . '"', $result['headers']);

$expected = "--$boundary\r\n" .
"Content-Type: text/plain; charset=UTF-8\r\n" .
"Content-Transfer-Encoding: 8bit\r\n" .
"\r\n" .
"\r\n" .
"\r\n" .
"This email was sent using the CakePHP Framework, http://cakephp.org." .
"\r\n" .
"\r\n" .
"--$boundary\r\n" .
"Content-Type: text/html; charset=UTF-8\r\n" .
"Content-Transfer-Encoding: 8bit\r\n" .
"\r\n" .
"<!DOCTYPE html";
$this->assertStringStartsWith($expected, $result['message']);

$expected = "</html>\r\n" .
"\r\n" .
"\r\n" .
"--$boundary--\r\n";
$this->assertStringEndsWith($expected, $result['message']);
}

/**
* testSendRender method for ISO-2022-JP
*
Expand Down Expand Up @@ -1542,8 +1588,6 @@ public function testSendMultipleMIME() {
$this->assertFalse(empty($boundary));
$this->assertContains('--' . $boundary, $message);
$this->assertContains('--' . $boundary . '--', $message);
$this->assertContains('--alt-' . $boundary, $message);
$this->assertContains('--alt-' . $boundary . '--', $message);

$this->CakeEmail->attachments(array('fake.php' => __FILE__));
$this->CakeEmail->send();
Expand Down Expand Up @@ -2005,7 +2049,7 @@ public function testBodyEncodingIso2022JpMs() {
}

protected function _checkContentTransferEncoding($message, $charset) {
$boundary = '--alt-' . $this->CakeEmail->getBoundary();
$boundary = '--' . $this->CakeEmail->getBoundary();
$result['text'] = false;
$result['html'] = false;
$length = count($message);
Expand Down

2 comments on commit b8fa7ce

@dereuromark
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit breaks existing emails with HTML templates and inline images.
I tried to narrow it down via diff tool.
Looks like the beginning of the inline attachments is off now:

</html>

--rel-6da67945d648c399dbd9c0de0890f42a
Content-Type: image/jpeg; charset=binary
Content-Transfer-Encoding: base64
Content-ID: <53734f45303c4dee86f0354c4e2f905e@domain.de>
Content-Disposition: inline; filename="logo.jpg"

/9j/4QAYRXhpZgAASUkqAAgAAAAAAAAAAAAAAP/sABFEdWNreQABAAQAAABkAAD/4QNvaHR0cDov

Used to work just fine.

Now it's:

</html>

--rel-6da67945d648c399dbd9c0de0890f42a--

--rel-6da67945d648c399dbd9c0de0890f42a
Content-Type: image/jpeg; charset=binary
Content-Transfer-Encoding: base64
Content-ID: <537352da37804a5fa7a936884e2f905e@domain.de>
Content-Disposition: inline; filename="logo.jpg"

/9j/4QAYRXhpZgAASUkqAAgAAAAAAAAAAAAAAP/sABFEdWNreQABAAQAAABkAAD/4QNvaHR0cDov

And this seems to not render the first image (in this case logo.jpg).

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll get this fixed today.

Please sign in to comment.