Urlize mode bug fixes

[dverdin]

Fixing bugs in urlize mode :

1- message text was treated as attachment and remained on the server as either "msg.0.bin" or "msg.0.txt" files. Fixed by testing the part type before urlizing it.
2- some special characters in message id led to the attachment to be considered missing by Sympa.

I also introduce a test file for Message.t, testing only urlization. this test is set in travis and the test succeeds. Note that commits 065312d and fcadbac are related to travis. The first one to enable the test, the second one to have Sympa::Constants without running make.

[dverdin]

All remarks made in PR #830 are included in the current PR.

[ikedas]

I think simply recursive urlization of subparts is not always appropriate.

We would be better to consider more precise cases where the subparts are treated as "attachment":

• multipart/mixed

  This type on the top level may be special. Subparts may be treated as either ---

  • The first text/* part should be treated as "text", and the others should be "attachment" even if it is a text.
  • Or, text/* part that is decodable (i.e. with proper charset parameter) and does not have "content-disposition: attachment" should be treated as "text", the others should be "attachment".

  --- and only "attachment" can be urlized.

• multipart/alternative, multipart/related
  Urlizing any subparts is not useful, because MUA is expected to show only most suitable subpart, or to render message using all subparts. Entire part (multipart) should be urlized if necessary.

• multipart/digest, multipart/report etc.
  Urlizing any subparts is not useful, because a multipart itself is an integrated structure. Entire part (multipart) should be urlized if necessary.

• message/rfc822, message/global
  Urlizing any parts this part contains is not useful, because this type represents embedded e-mail message and its content should not be broken. Entire part (message) should be urlized if necessary.

Thus, IMO only multipart/mixed may be modified recursively (with restrictions described in above) to "urlize" messages, and all others can be "urlized" by its own (if it is not treated as "text") or be intact (if it is treated as "text" or it is smaller than urlize_min_size).

[dverdin]

Great review @ikedas !
I'll confront it to the problems I met in the sample files provided in the PR to check whether your suggestions work with them. These were originally messages for which urlization did not work correctly.
More news in a day or two!
David

[dverdin]

Okie dokie... I had a look and I still have a problem with nested multiparts.

Apple Mail sometimes sends emails with attachments within multipart/alternative messages.
In those messages, the first part is text/plain, the second is multipart/mixed, and contains the text/html version of the first part and any attachments.

I think this is wrong because it makes no sense to put the attachments within an alternative part, however it is a fact. I'd like to simply ignore such misbehaviours but end users will probably consider it is Sympa's fault is urlize mode does not work.

I'd the like to consider multipart/related and multipart/alternative as potential attachments bearers.

That would lead to the following (changes to your version are in bold font):

• multipart/mixed

  This type on the top level may be special. Subparts may be treated as either ---

  • The first text/* part should be treated as "text", and the others should be "attachment" even if it is a text. (I prefer the scond option, simpler to implement)
  • text part that is decodable (i.e. with proper charset parameter) and does not have "content-disposition: attachment" should be treated as "text", the others should be "attachment".

  --- and only "attachment" can be urlized.

• multipart/alternative, multipart/related
  Don't urlize any subpart except if it is a multipart/mixed.
  Urlizing any subparts is not useful, because MUA is expected to show only most suitable subpart, or to render message using all subparts. Entire part (multipart) should be urlized if necessary.

• multipart/digest, multipart/report etc.
  Urlizing any subparts is not useful, because a multipart itself is an integrated structure. Entire part (multipart) should be urlized if necessary.

• message/rfc822, message/global
  Urlizing any parts this part contains is not useful, because this type represents embedded e-mail message and its content should not be broken. Entire part (message) should be urlized if necessary.

[dverdin]

OK, I have implemented what we discussed. A slight change : I urlize sub-parts of multipart/alternative, whichever they are, bu only one level below the message top level part. That means that if they contains anything other than valid text, it will be urlized. That leaves a chance to catch a multipart/mixed inside, but we won't go further.

This is not satisfactory yet, though it works well with the test files. It still needs working.

I have reorganized the test files to make their usage more explicit.

[dverdin]

OK, now it does exactly what I wanted. Only going to urlize sub parts of a multipart/related if it is a multipart/mixed.
I'll wait to see whether all the tests complete, then maybe squash some commits to make the whole PR more understandable.

[dverdin]

OK, I squashed the commits to keep only four commits, combining the code modifications we compromised over. It should be easier to understand fr future developers.

[ikedas]

Nice. I don't have comments any more.

[dverdin]

Actually, I still found some cases in production, just today, that don't come out of urlization as expected.
So I'll have to dig in them a little to understand why my code don't process them correctly.
I'll post an update here soon.

Regards,

[dverdin]

OK, now that seems right. I had to go one level further in the message architecture but I added an explicit rule in _urlize_one_part that will prevent urlization to occur when the parent part is not a multipart/mixed.
As usual, I put this code in production right now.

[ikedas]

I merged. Thanks @dverdin !

[dverdin]

Thanks @ikedas !
Glad to see this behind me...
Now, I'm heading towards the next bunch of PR...
Regards,
David