Fix failing tests and missing boundary markers.

When sending html + text emails, there were duplicate multipart/alternative sections and the trailing top level boundary was missing.
cakephp · Dec 28, 2011 · 83b28c4 · 83b28c4
1 parent f366a9f
commit 83b28c4
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 18 deletions.
diff --git a/lib/Cake/Network/Email/CakeEmail.php b/lib/Cake/Network/Email/CakeEmail.php
@@ -662,14 +662,12 @@ public function getHeaders($include = array()) {
 		}
 
 		$headers['MIME-Version'] = '1.0';
-		if (!empty($this->_attachments)) {
+		if (!empty($this->_attachments) || $this->_emailFormat === 'both') {
 			$headers['Content-Type'] = 'multipart/mixed; boundary="' . $this->_boundary . '"';
 		} elseif ($this->_emailFormat === 'text') {
 			$headers['Content-Type'] = 'text/plain; charset=' . $this->charset;
 		} elseif ($this->_emailFormat === 'html') {
 			$headers['Content-Type'] = 'text/html; charset=' . $this->charset;
-		} elseif ($this->_emailFormat === 'both') {
-			$headers['Content-Type'] = 'multipart/alternative; boundary="alt-' . $this->_boundary . '"';
 		}
 		$headers['Content-Transfer-Encoding'] = $this->_getContentTransferEncoding();
 
@@ -1377,6 +1375,8 @@ protected function _render($content) {
 		if ($hasAttachments) {
 			$attachments = $this->_attachFiles();
 			$msg = array_merge($msg, $attachments);
+		}
+		if ($hasAttachments || $hasMultipleTypes) {
 			$msg[] = '';
 			$msg[] = '--' . $boundary . '--';
 			$msg[] = '';

diff --git a/lib/Cake/Test/Case/Controller/Component/EmailComponentTest.php b/lib/Cake/Test/Case/Controller/Component/EmailComponentTest.php
@@ -228,12 +228,6 @@ public function testSendFormats() {
 		$expect = str_replace('{CONTENTTYPE}', 'text/html; charset=UTF-8', $message);
 		$this->assertTrue($this->Controller->EmailTest->send('This is the body of the message'));
 		$this->assertEquals(DebugCompTransport::$lastEmail, $this->__osFix($expect));
-
-		// TODO: better test for format of message sent?
-		$this->Controller->EmailTest->sendAs = 'both';
-		$expect = str_replace('{CONTENTTYPE}', 'multipart/alternative; boundary="alt-"', $message);
-		$this->assertTrue($this->Controller->EmailTest->send('This is the body of the message'));
-		$this->assertEquals(preg_replace('/alt-[a-z0-9]{32}/i', 'alt-', DebugCompTransport::$lastEmail), $this->__osFix($expect));
 	}
 
 /**
@@ -307,13 +301,29 @@ public function testTemplates() {
 		$this->assertEquals(DebugCompTransport::$lastEmail, $this->__osFix($expect));
 
 		$this->Controller->EmailTest->sendAs = 'both';
-		$expect = str_replace('{CONTENTTYPE}', 'multipart/alternative; boundary="alt-"', $header);
-		$expect .= '--alt-' . "\n" . 'Content-Type: text/plain; charset=UTF-8' . "\n" . 'Content-Transfer-Encoding: 8bit' . "\n\n" . $text . "\n\n";
-		$expect .= '--alt-' . "\n" . 'Content-Type: text/html; charset=UTF-8' . "\n" . 'Content-Transfer-Encoding: 8bit' . "\n\n" . $html . "\n\n";
-		$expect = '<pre>' . $expect . '--alt---' . "\n\n" . '</pre>';
+		$expect = str_replace('{CONTENTTYPE}', 'multipart/mixed; boundary="{boundary}"', $header);
+		$expect .= "--{boundary}\n" .
+			'Content-Type: multipart/alternative; boundary="alt-{boundary}"' . "\n\n" .
+			'--alt-{boundary}' . "\n" .
+			'Content-Type: text/plain; charset=UTF-8' . "\n" .
+			'Content-Transfer-Encoding: 8bit' . "\n\n" .
+			$text .
+			"\n\n" .
+			'--alt-{boundary}' . "\n" .
+			'Content-Type: text/html; charset=UTF-8' . "\n" .
+			'Content-Transfer-Encoding: 8bit' . "\n\n" . 
+			$html .
+			"\n\n" . 
+			'--alt-{boundary}--' . "\n\n\n" . 
+			'--{boundary}--' . "\n";
+
+		$expect = '<pre>' . $expect . '</pre>';
 
 		$this->assertTrue($this->Controller->EmailTest->send('This is the body of the message'));
-		$this->assertEquals(preg_replace('/alt-[a-z0-9]{32}/i', 'alt-', DebugCompTransport::$lastEmail), $this->__osFix($expect));
+		$this->assertEquals(
+			$this->__osFix($expect),
+			preg_replace('/[a-z0-9]{32}/i', '{boundary}', DebugCompTransport::$lastEmail)
+		);
 
 		$html = <<<HTMLBLOC
 <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN">
@@ -441,7 +451,7 @@ public function testMessageRetrievalWithoutTemplate() {
 
 		$this->Controller->EmailTest->delivery = 'DebugComp';
 
-		$text = $html = 'This is the body of the message';
+		$text = $html = "This is the body of the message\n";
 
 		$this->Controller->EmailTest->sendAs = 'both';
 		$this->assertTrue($this->Controller->EmailTest->send('This is the body of the message'));
@@ -740,8 +750,8 @@ public function testSendAsIsNotIgnoredIfAttachmentsPresent() {
 		$this->assertTrue($this->Controller->EmailTest->send($body));
 		$msg = DebugCompTransport::$lastEmail;
 
-		$this->assertNotRegExp('/text\/plain/', $msg);
-		$this->assertNotRegExp('/text\/html/', $msg);
+		$this->assertRegExp('/text\/plain/', $msg);
+		$this->assertRegExp('/text\/html/', $msg);
 		$this->assertRegExp('/multipart\/alternative/', $msg);
 	}
 

diff --git a/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php b/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php
@@ -1080,7 +1080,7 @@ public function testSendMultipleMIME() {
 		$boundary = $this->CakeEmail->getBoundary();
 		$this->assertFalse(empty($boundary));
 		$this->assertContains('--' . $boundary, $message);
-		$this->assertNotContains('--' . $boundary . '--', $message);
+		$this->assertContains('--' . $boundary . '--', $message);
 		$this->assertContains('--alt-' . $boundary, $message);
 		$this->assertContains('--alt-' . $boundary . '--', $message);