From 3666c3469acd042d75e97044c0f1ccaf719fd6be Mon Sep 17 00:00:00 2001 From: Synchro Date: Wed, 15 May 2013 12:36:42 +0200 Subject: [PATCH] Fix cid generation in MsgHTML (Thanks to @digitalthought), fixes #60 Fix handling of multiple SMTP servers (Thanks to @NanoCaiordo), see #58, #32 --- changelog.md | 2 + class.phpmailer.php | 87 +++++++++++++++++++++++------------------- test/phpmailerTest.php | 20 ++++++++-- 3 files changed, 66 insertions(+), 43 deletions(-) diff --git a/changelog.md b/changelog.md index 6790ae71..2ed7cc76 100644 --- a/changelog.md +++ b/changelog.md @@ -19,6 +19,8 @@ * Don't set unnecessary reply-to addresses * Make fakesendmail.sh a bit cleaner and safer * Set a content-transfer-encoding on multiparts (fixes msglint error) +* Fix cid generation in MsgHTML (Thanks to @digitalthought) +* Fix handling of multiple SMTP servers (Thanks to @NanoCaiordo) ## Version 5.2.6 (April 11th 2013) * Reflect move to PHPMailer GitHub organisation at https://github.com/PHPMailer/PHPMailer diff --git a/class.phpmailer.php b/class.phpmailer.php index 1cc1e960..c2626b07 100644 --- a/class.phpmailer.php +++ b/class.phpmailer.php @@ -1069,62 +1069,65 @@ class PHPMailer { $this->smtp = new SMTP; } + //Already connected? + if ($this->smtp->Connected()) { + return true; + } + $this->smtp->Timeout = $this->Timeout; $this->smtp->do_debug = $this->SMTPDebug; $this->smtp->Debugoutput = $this->Debugoutput; $this->smtp->do_verp = $this->do_verp; - $hosts = explode(';', $this->Host); $index = 0; - $connection = $this->smtp->Connected(); + $tls = ($this->SMTPSecure == 'tls'); + $ssl = ($this->SMTPSecure == 'ssl'); + $hosts = explode(';', $this->Host); + $lastexception = null; - // Retry while there is no connection - try { - while($index < count($hosts) && !$connection) { - $hostinfo = array(); - if (preg_match('/^(.+):([0-9]+)$/', $hosts[$index], $hostinfo)) { - $host = $hostinfo[1]; - $port = $hostinfo[2]; - } else { - $host = $hosts[$index]; - $port = $this->Port; - } - - $tls = ($this->SMTPSecure == 'tls'); - $ssl = ($this->SMTPSecure == 'ssl'); - - if ($this->smtp->Connect(($ssl ? 'ssl://':'').$host, $port, $this->Timeout)) { - - $hello = ($this->Helo != '' ? $this->Helo : $this->ServerHostname()); + foreach ($hosts as $hostentry) { + $hostinfo = array(); + $host = $hostentry; + $port = $this->Port; + if (preg_match('/^(.+):([0-9]+)$/', $hostentry, $hostinfo)) { //If $hostentry contains 'address:port', override default + $host = $hostinfo[1]; + $port = $hostinfo[2]; + } + if ($this->smtp->Connect(($ssl ? 'ssl://':'').$host, $port, $this->Timeout)) { + try { + if ($this->Helo) { + $hello = $this->Helo; + } else { + $hello = $this->ServerHostname(); + } $this->smtp->Hello($hello); if ($tls) { if (!$this->smtp->StartTLS()) { throw new phpmailerException($this->Lang('connect_host')); } - //We must resend HELO after tls negotiation $this->smtp->Hello($hello); } - - $connection = true; if ($this->SMTPAuth) { if (!$this->smtp->Authenticate($this->Username, $this->Password, $this->AuthType, $this->Realm, $this->Workstation)) { throw new phpmailerException($this->Lang('authenticate')); } } + return true; + } catch (phpmailerException $e) { + $lastexception = $e; + //We must have connected, but then failed TLS or Auth, so close connection nicely + $this->smtp->Quit(); } - $index++; - if (!$connection) { - throw new phpmailerException($this->Lang('connect_host')); - } - } - } catch (phpmailerException $e) { - $this->smtp->Reset(); - if ($this->exceptions) { - throw $e; } } - return true; + //If we get here, all connection attempts have failed, so close connection hard + $this->smtp->Close(); + //As we've caught all exceptions, just report whatever the last one was + if ($this->exceptions and !is_null($lastexception)) { + throw $lastexception; + } + return false; } /** @@ -2478,8 +2481,8 @@ class PHPMailer { */ public function MsgHTML($message, $basedir = '', $advanced = false) { preg_match_all("/(src|background)=[\"'](.*)[\"']/Ui", $message, $images); - if(isset($images[2])) { - foreach($images[2] as $i => $url) { + if (isset($images[2])) { + foreach ($images[2] as $i => $url) { // do not change urls for absolute images (thanks to corvuscorax) if (!preg_match('#^[A-z]+://#', $url)) { $filename = basename($url); @@ -2487,11 +2490,15 @@ class PHPMailer { if ($directory == '.') { $directory = ''; } - $cid = 'cid:' . md5($url).'@phpmailer.0'; //RFC2392 S 2 - if ( strlen($basedir) > 1 && substr($basedir, -1) != '/') { $basedir .= '/'; } - if ( strlen($directory) > 1 && substr($directory, -1) != '/') { $directory .= '/'; } - if ( $this->AddEmbeddedImage($basedir.$directory.$filename, $cid, $filename, 'base64', self::_mime_types(self::mb_pathinfo($filename, PATHINFO_EXTENSION)))) { - $message = preg_replace("/".$images[1][$i]."=[\"']".preg_quote($url, '/')."[\"']/Ui", $images[1][$i]."=\"".$cid."\"", $message); + $cid = md5($url).'@phpmailer.0'; //RFC2392 S 2 + if (strlen($basedir) > 1 && substr($basedir, -1) != '/') { + $basedir .= '/'; + } + if (strlen($directory) > 1 && substr($directory, -1) != '/') { + $directory .= '/'; + } + if ($this->AddEmbeddedImage($basedir.$directory.$filename, $cid, $filename, 'base64', self::_mime_types(self::mb_pathinfo($filename, PATHINFO_EXTENSION)))) { + $message = preg_replace("/".$images[1][$i]."=[\"']".preg_quote($url, '/')."[\"']/Ui", $images[1][$i]."=\"cid:".$cid."\"", $message); } } } diff --git a/test/phpmailerTest.php b/test/phpmailerTest.php index ee9803de..18dc5dd1 100644 --- a/test/phpmailerTest.php +++ b/test/phpmailerTest.php @@ -738,7 +738,7 @@ EOT; $this->Mail->CharSet = 'utf-8'; $this->Mail->Body = ''; $this->Mail->AltBody = ''; - $this->Mail->MsgHTML($message, '..'); + $this->Mail->MsgHTML($message, '../examples'); $this->Mail->Subject .= ': MsgHTML'; $this->assertNotEmpty($this->Mail->Body, 'Body not set by MsgHTML'); @@ -747,7 +747,7 @@ EOT; //Again, using the advanced HTML to text converter $this->Mail->AltBody = ''; - $this->Mail->MsgHTML($message, '..', true); + $this->Mail->MsgHTML($message, '../examples', true); $this->Mail->Subject .= ' + html2text advanced'; $this->assertNotEmpty($this->Mail->AltBody, 'Advanced AltBody not set by MsgHTML'); @@ -954,7 +954,21 @@ EOT; $this->Mail->SmtpClose(); } - /** + /** + * Test SMTP host connections + */ + function test_SmtpConnect() + { + $this->assertTrue($this->Mail->SmtpConnect(), 'SMTP single connect failed'); + $this->Mail->SmtpClose(); + $this->Mail->Host = "localhost:12345;10.10.10.10:54321"; + $this->assertFalse($this->Mail->SmtpConnect(), 'SMTP bad multi-connect succeeded'); + $this->Mail->SmtpClose(); + $this->Mail->Host = "localhost:12345;10.10.10.10:54321;".$_REQUEST['mail_host']; + $this->assertTrue($this->Mail->SmtpConnect(), 'SMTP multi-connect failed'); + } + + /** * Tests this denial of service attack: * http://www.cybsec.com/vuln/PHPMailer-DOS.pdf */