Skip to content
This repository has been archived by the owner on Nov 17, 2021. It is now read-only.

Fix sorting MIME children when their types are equal #907

Closed
wants to merge 4 commits into from
Closed

Fix sorting MIME children when their types are equal #907

wants to merge 4 commits into from

Conversation

Grendel7
Copy link
Contributor

Currently, if a message has two MIME children with the same content type, they will be added in their inverted order. In most cases, this won't matter, but for some e-mails (like X-ARF, which has two text/plain parts, one with human readable and one with YAML content), this is problematic.

This small fix ensures the original ordering is preserved when the typePrefs are identical.

@fabpot
Copy link
Member

fabpot commented Apr 29, 2017

Thanks for the fix. Can you add a test for that behavior?

@Grendel7
Copy link
Contributor Author

Sure, I will add a test for it ASAP.

@Grendel7
Copy link
Contributor Author

After adding my test, it turns out that this fix causes a regression for PHP 5.

The reason for this is the ordering of the first and second argument of usort being different in PHP 5 and PHP 7.

Given a simple proof of content:

<?php
$array = ['one', 'two'];

usort($array, function($a, $b) {
	echo 'A: '.$a.' - B: '.$b;
	return 0;
});

The output for PHP 5.6 is:

A: two - B: one

But for PHP 7.0, the ouput is:

A: one - B: two

In other words, you can't make any assumptions about $a and $b regarding their position in the source array.

Because of that, I rewrote the entire sorting function to not rely on usort anymore. This implementation groups the mime parts first, sorts those and then flattens the sorted groups. This is the only method I could think of which reliably works on all PHP versions.

@fabpot
Copy link
Member

fabpot commented Apr 30, 2017

Can you rebase? I've fixed Travis conf. Thanks

@Grendel7
Copy link
Contributor Author

Thanks, I've rebased the branch and all tests are green now.

@fabpot
Copy link
Member

fabpot commented May 1, 2017

Thank you @HansAdema.

@fabpot fabpot closed this in a2caa64 May 1, 2017
voku added a commit to voku/swiftmailer that referenced this pull request May 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants