Potential issue on Mail::Body#sort_parts! being trigger by internal api

[mikel]

Potential issue on Mail::Body#sort_parts! being trigger by internal api

[mikel]

Sounds good, any chance you can write a patch with a spec to cover?

[nel]

Here's a failing spec:

http://gist.github.com/320229

Mail::Body#sort_parts! only makes sense for email generation, and Mail::Message#html_part Mail::Message#text_part Mail::Message#part Mail::Message#all_part mostly makes senses and need consistency in email parsing.

I triggered the bug through the to_s (that trigger) body.encoded that trigger Body#sort_parts!. There is no real reason to use to_s in real case, so in most case this won't be a problem.

If you want me to fix it, what I'll do is get rid of Body#sort_parts! and replace it by a dynamic Body#sorted_parts (private?) and Body#encoded would use it directly. I can imagine the use of sorting the parts comes from a bad support for some mail client when attachment come first before text part or html part.

It's your lib, you decide :)

[mikel]

Please leave sort_parts! but add a public method sorted_parts and switch encoding to use this.

Mikel

[mikel]

Actually, I can't really see the problem here, at least with the spec you provided, because in that case we ARE generating the mail message and so the parts should get sorted.

Do you have a case where we are not?

Mikel

[nel]

I agree for real work use this is not an issue as you are unlikely to generate the encoded email and then iterate over its parts. But while playing around with the api this is kinda unexpected that the order of your parts is changing when you execute some methods.

By the way I had a look at this time and this created tons of change just to add a sorted_parts which does change the internal state of the parts as this is deeply nest to a low level Array Extension and tweeked version of sort! with extra arguments if I remind well.

As a conclusion this is far too complicated for the minimal level of inconvenience which by the way is not a problem in real world case so you can close this ticket in my opinion.

[mikel]

OK, awesome :)

[mikel]

Having said that and closed, I do agree that it could be neater... maybe for Mail 3 :)