[+]: "bug swiftmailer#907 Fix sorting MIME children when their types …

…are equal (Hans Adema)" -> swiftmailer@a2caa64
voku · May 4, 2017 · 158fb79 · 158fb79
1 parent 360133f
commit 158fb79
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 28 deletions.
diff --git a/lib/classes/Swift/Mime/SimpleMimeEntity.php b/lib/classes/Swift/Mime/SimpleMimeEntity.php
@@ -647,9 +647,9 @@ protected function _getHeaderFieldModel($field)
     {
         if ($this->_headers->has($field)) {
             return $this->_headers->get($field)->getFieldBodyModel();
-        } else {
-            return null;
         }
+
+        return null;
     }
 
     /**
@@ -682,9 +682,9 @@ protected function _getHeaderParameter($field, $parameter)
     {
         if ($this->_headers->has($field)) {
             return $this->_headers->get($field)->getParameter($parameter);
-        } else {
-            return false;
         }
+
+        return false;
     }
 
     /**
@@ -898,34 +898,25 @@ private function _sortChildren()
 
         // Sort in order of preference, if there is one
         if ($shouldSort) {
-            usort($this->_immediateChildren, array($this, '_childSortAlgorithm'));
-        }
-    }
-
-    /**
-     * @param Swift_Mime_MimeEntity $a
-     * @param Swift_Mime_MimeEntity $b
-     *
-     * @return int
-     */
-    private function _childSortAlgorithm($a, $b)
-    {
-        $typePrefs = array();
+            // Group the messages by order of preference
+            $sorted = array();
+            foreach ($this->_immediateChildren as $child) {
+                $type = $child->getContentType();
+                $level = array_key_exists($type, $this->_alternativePartOrder) ?
+                    $this->_alternativePartOrder[$type] :
+                    max($this->_alternativePartOrder) + 1;
 
-        $types = array(
-            Swift::strtolowerWithStaticCache($a->getContentType()),
-            Swift::strtolowerWithStaticCache($b->getContentType())
-        );
+                if (empty($sorted[$level])) {
+                    $sorted[$level] = array();
+                }
 
-        foreach ($types as $type) {
-            if (isset($this->_alternativePartOrder[$type])) {
-                $typePrefs[] = $this->_alternativePartOrder[$type];
-            } else {
-                $typePrefs[] = max($this->_alternativePartOrder) + 1;
+                $sorted[$level][] = $child;
             }
-        }
 
-        return $typePrefs[0] >= $typePrefs[1] ? 1 : -1;
+            ksort($sorted);
+
+            $this->_immediateChildren = array_reduce($sorted, 'array_merge', array());
+        }
     }
 
     /**

diff --git a/tests/unit/Swift/Mime/AbstractMimeEntityTest.php b/tests/unit/Swift/Mime/AbstractMimeEntityTest.php
@@ -1076,6 +1076,44 @@ public function testOrderingTextBeforeHtml()
     );
   }
 
+  public function testOrderingEqualContentTypesMaintainsOriginalOrdering()
+  {
+    $firstChild = new MimeEntityFixture(Swift_Mime_MimeEntity::LEVEL_ALTERNATIVE,
+                                        "Content-Type: text/plain\r\n".
+                                        "\r\n".
+                                        'PART 1',
+                                        'text/plain'
+    );
+    $secondChild = new MimeEntityFixture(Swift_Mime_MimeEntity::LEVEL_ALTERNATIVE,
+                                         "Content-Type: text/plain\r\n".
+                                         "\r\n".
+                                         'PART 2',
+                                         'text/plain'
+    );
+    $headers = $this->_createHeaderSet(array(), false);
+    $headers->shouldReceive('toString')
+            ->zeroOrMoreTimes()
+            ->andReturn("Content-Type: multipart/alternative; boundary=\"xxx\"\r\n");
+    $entity = $this->_createEntity($headers, $this->_createEncoder(),
+                                   $this->_createCache()
+    );
+    $entity->setBoundary('xxx');
+    $entity->setChildren(array($firstChild, $secondChild));
+    $this->assertEquals(
+        "Content-Type: multipart/alternative; boundary=\"xxx\"\r\n".
+        "\r\n\r\n--xxx\r\n".
+        "Content-Type: text/plain\r\n".
+        "\r\n".
+        'PART 1'.
+        "\r\n\r\n--xxx\r\n".
+        "Content-Type: text/plain\r\n".
+        "\r\n".
+        'PART 2'.
+        "\r\n\r\n--xxx--\r\n",
+        $entity->toString()
+    );
+  }
+
   public function testUnsettingChildrenRestoresContentType()
   {
     $cType = $this->_createHeader('Content-Type', 'text/plain', array(), false);