Fix not allowed encoding of content-transfer-encoding and content-type headers in single part encoded mails #5140

Closed
wants to merge 10 commits into
from

Projects

None yet

4 participants

@2DivisionsByZero

for single-part mime mails.

See #5128

Requires #5303
Needs to be adapted if #5291 is merged.

2DivisionsByZero Fix spacing 7d5a92e
2DivisionsBy... added some commits Sep 19, 2013
2DivisionsByZero Fix spaces 56fd41f
2DivisionsByZero Fix spaces eba42c9
2DivisionsBy... added some commits Sep 19, 2013
2DivisionsByZero Fix spacing fb9eae0
2DivisionsByZero Spaces fixed? 3de4916
@samsonasik samsonasik commented on an outdated diff Sep 20, 2013
library/Zend/Mail/Header/ContentTransferEncoding.php
+ public function getEncoding()
+ {
+ return 'ASCII';
+ }
+
+ public function toString()
+ {
+ return 'Content-Transfer-Encoding: ' . $this->getFieldValue();
+ }
+
+ /**
+ * Set the content transfer encoding
+ *
+ * @param string $transferEncoding
+ * @throws Exception\InvalidArgumentException
+ * @return ContentTransferEncoding
@samsonasik samsonasik commented on an outdated diff Sep 20, 2013
library/Zend/Mail/Header/HeaderLoader.php
- 'mime-version' => 'Zend\Mail\Header\MimeVersion',
- 'received' => 'Zend\Mail\Header\Received',
- 'replyto' => 'Zend\Mail\Header\ReplyTo',
- 'reply_to' => 'Zend\Mail\Header\ReplyTo',
- 'reply-to' => 'Zend\Mail\Header\ReplyTo',
- 'sender' => 'Zend\Mail\Header\Sender',
- 'subject' => 'Zend\Mail\Header\Subject',
- 'to' => 'Zend\Mail\Header\To',
+ 'bcc' => 'Zend\Mail\Header\Bcc',
+ 'cc' => 'Zend\Mail\Header\Cc',
+ 'contenttype' => 'Zend\Mail\Header\ContentType',
+ 'content_type' => 'Zend\Mail\Header\ContentType',
+ 'content-type' => 'Zend\Mail\Header\ContentType',
+ 'contenttransferencoding' => 'Zend\Mail\Header\ContentTransferEncoding',
+ 'content_transfer_encoding' => 'Zend\Mail\Header\ContentTransferEncoding',
+ 'content-transfer-encoding' => 'Zend\Mail\Header\ContentTransferEncoding',
@samsonasik samsonasik commented on an outdated diff Sep 20, 2013
.../ZendTest/Mail/Header/ContentTransferEncodingTest.php
+ * @subpackage UnitTests
+ * @group Zend_Mail
+ */
+class ContentTransferEncodingTest extends \PHPUnit_Framework_TestCase
+{
+
+ public function testContentTransferEncodingFromStringCreatesValidContentTransferEncodingHeader()
+ {
+ $contentTransferEncodingHeader = ContentTransferEncoding::fromString('Content-Transfer-Encoding: 7bit');
+ $this->assertInstanceOf('Zend\Mail\Header\HeaderInterface', $contentTransferEncodingHeader);
+ $this->assertInstanceOf('Zend\Mail\Header\ContentTransferEncoding', $contentTransferEncodingHeader);
+ }
+
+ public function testContentTransferEncodingFromStringCreateExcaption()
+ {
+ $this->setExpectedException('Zend\Mail\Header\Exception\InvalidArgumentException');
@samsonasik samsonasik commented on an outdated diff Sep 20, 2013
tests/ZendTest/Mail/MessageTest.php
+ $mime = new Mime('foo-bar');
+ $part = new MimePart('UTF-8 TestString: AaÜüÄäÖöß');
+ $part->type = Mime::TYPE_TEXT;
+ $part->encoding = Mime::ENCODING_QUOTEDPRINTABLE;
+ $part->charset = 'utf-8';
+ $body = new MimeMessage();
+ $body->setMime($mime);
+ $body->addPart($part);
+
+ $this->message->setEncoding('UTF-8');
+ $this->message->setBody($body);
+
+ $this->assertContains(
+ 'Content-Type: text/plain;' . Headers::FOLDING . 'charset="utf-8"' . Headers::EOL
+ . 'Content-Transfer-Encoding: quoted-printable' . Headers::EOL,
+ $this->message->getHeaders()->toString()
@samsonasik
samsonasik Sep 20, 2013

indentation 4 spaces.

@Maks3w Maks3w commented on an outdated diff Oct 2, 2013
.../ZendTest/Mail/Header/ContentTransferEncodingTest.php
@@ -0,0 +1,66 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Mail
@Maks3w
Maks3w Oct 2, 2013

@samsonasik You are missing capabalities :) Where are you comment about @package ...

@2DivisionsByZero Please remove package, subpackage and category

@samsonasik samsonasik commented on an outdated diff Oct 2, 2013
.../ZendTest/Mail/Header/ContentTransferEncodingTest.php
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Mail
+ */
+
+namespace ZendTest\Mail\Header;
+
+use Zend\Mail\Header\ContentTransferEncoding;
+
+/**
+ * @category Zend
+ * @package Zend_Mail
+ * @subpackage UnitTests
2DivisionsByZero remove package, subpackage and category
as requested by @samsonasik
2a6d095
@2DivisionsByZero

@Maks3w @samsonasik Is the request OK now?

@samsonasik

👍

@DASPRiD DASPRiD and 1 other commented on an outdated diff Oct 6, 2013
library/Zend/Mail/Header/ContentTransferEncoding.php
+ return $header;
+ }
+
+ public function getFieldName()
+ {
+ return 'Content-Transfer-Encoding';
+ }
+
+ public function getFieldValue($format = HeaderInterface::FORMAT_RAW)
+ {
+ return $this->transferEncoding;
+ }
+
+ public function setEncoding($encoding)
+ {
+ // This header must be always in US-ASCII
@DASPRiD
DASPRiD Oct 6, 2013

I'd rather throw an exception here, instead of silently ignoring the setting (if possible).

@2DivisionsByZero
2DivisionsByZero Oct 6, 2013

Not changed because it should not be changed regardless what encoding the rest of the message is changed to.
Similar to ContentType.

@DASPRiD DASPRiD commented on an outdated diff Oct 6, 2013
library/Zend/Mail/Header/ContentTransferEncoding.php
+
+ public function toString()
+ {
+ return 'Content-Transfer-Encoding: ' . $this->getFieldValue();
+ }
+
+ /**
+ * Set the content transfer encoding
+ *
+ * @param string $transferEncoding
+ * @throws Exception\InvalidArgumentException
+ * @return self
+ */
+ public function setTransferEncoding($transferEncoding)
+ {
+ if (!preg_match('/^(7bit|8bit|quoted\-printable|base64)+$/i', $transferEncoding)) {
@DASPRiD
DASPRiD Oct 6, 2013

Why not a simple in_array() comparison?

@DASPRiD DASPRiD commented on an outdated diff Oct 6, 2013
library/Zend/Mail/Header/ContentTransferEncoding.php
+ /**
+ * @var array
+ */
+ protected $parameters = array();
+
+ public static function fromString($headerLine)
+ {
+ $headerLine = iconv_mime_decode($headerLine, ICONV_MIME_DECODE_CONTINUE_ON_ERROR, 'UTF-8');
+ list($name, $value) = explode(': ', $headerLine, 2);
+
+ // check to ensure proper header type for this factory
+ if (strtolower($name) !== 'content-transfer-encoding') {
+ throw new Exception\InvalidArgumentException('Invalid header line for Content-Transfer-Encoding string');
+ }
+
+ $transferEncoding = $value;
@DASPRiD
DASPRiD Oct 6, 2013

Remove too many spaces before equal sign.

@Maks3w Maks3w commented on an outdated diff Oct 6, 2013
library/Zend/Mail/Message.php
@@ -402,7 +402,27 @@ public function setBody($body)
$parts = $this->body->getParts();
if (!empty($parts)) {
$part = array_shift($parts);
- $headers->addHeaders($part->getHeadersArray());
+ $part_headers = $part->getHeadersArray();
@Maks3w
Maks3w Oct 6, 2013

use camelCase var names

Test Fixes problems highlighted by @DASPRiD and @Maks3w
- do simple in_array comparison for transfer encoding
- remove to many spaces befor equal sign
- use camelCase var names
27be8a2
@Maks3w Maks3w was assigned Oct 18, 2013
@Maks3w Maks3w commented on the diff Oct 20, 2013
library/Zend/Mail/Header/ContentTransferEncoding.php
@@ -0,0 +1,117 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace Zend\Mail\Header;
+
+use Zend\Mail\Headers;
@Maks3w Maks3w commented on the diff Oct 20, 2013
library/Zend/Mail/Header/ContentTransferEncoding.php
+ /**
+ * @var array
+ */
+ protected $parameters = array();
+
+ public static function fromString($headerLine)
+ {
+ $headerLine = iconv_mime_decode($headerLine, ICONV_MIME_DECODE_CONTINUE_ON_ERROR, 'UTF-8');
+ list($name, $value) = explode(': ', $headerLine, 2);
+
+ // check to ensure proper header type for this factory
+ if (strtolower($name) !== 'content-transfer-encoding') {
+ throw new Exception\InvalidArgumentException('Invalid header line for Content-Transfer-Encoding string');
+ }
+
+ $transferEncoding = $value;
@Maks3w
Maks3w Oct 20, 2013

This var name change is unnecessary

@Maks3w Maks3w commented on the diff Oct 20, 2013
library/Zend/Mail/Message.php
@@ -402,7 +402,27 @@ public function setBody($body)
$parts = $this->body->getParts();
if (!empty($parts)) {
$part = array_shift($parts);
- $headers->addHeaders($part->getHeadersArray());
@Maks3w
Maks3w Oct 20, 2013

Please undo all this set of changes. This is not the expected way of manage specific headers.

@Maks3w
Zend Framework member

I've open #5303 for fix the main issue related with the cast to string. That PR makes unnecessary the changes of this PR in Zend\Mail\Message.

Obviously #5303 needs to be merged before this PR.

@Maks3w
Zend Framework member

Finally header classes needs to be adapted for the future #5291 since introduce the same bug about headerline split

@Maks3w Maks3w added a commit that referenced this pull request Oct 20, 2013
@Maks3w Maks3w [#5140] Adapt changes to actual master changes.
Adapt code to design changes added by #5291 and #5303
81a4ae2
@Maks3w Maks3w added a commit that referenced this pull request Oct 20, 2013
@Maks3w Maks3w Forward port #5140 0d5c7fe
@Maks3w Maks3w closed this Oct 20, 2013
@weierophinney weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
@Maks3w Maks3w [zendframework/zendframework#5140] Adapt changes to actual master cha…
…nges.

Adapt code to design changes added by zendframework/zendframework#5291 and zendframework/zendframework#5303
c38a124
@weierophinney weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
@Maks3w Maks3w Merge pull request zendframework/zendframework#5140 branch 'hotfix/5140' 57d925a
@weierophinney weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
@Maks3w Maks3w Forward port zendframework/zendframework#5140 a1e32c7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment