GNOME Bugzilla – Bug 235681
Preserve header's folding
Last modified: 2017-06-23 00:51:10 UTC
(See bug#235664) When adding an X-Evolution header to mail (in an MH folder), Evolution strips whitespace and newlines from all headers in the mail, rather than only adding a single line as it should.
this doesn't destroy any info about the mail, so isn't critical
See http://bugzilla.gnome.org/show_bug.cgi?id=260055 for an example of how this header mangling becomes a nuisance.
its still cosmetic, folding is just a line-level/whitespace syntax, the headers are not corrupted. it has nothing to do with the x-evolution* headers either.
*** Bug 256916 has been marked as a duplicate of this bug. ***
david, does this still happen with a current evolution version?
I haven't tried Evolution with local mailboxes for a long time, but yes, the corruption still happens deep within Camel -- even when you select 'Show email source' you get the munging demonstrated at http://david.woodhou.se/evo-ate-my-spam-report.jpeg Bug #266916 and #260055 were about this broken _display_, although I originally filed this because I made the mistake of pointing Evolution at a few GB of MH mailboxen, and it went through them _all_, adding an X-Evolution-Status: header and mangling the whitespace in the process. The root cause of each is the same (although the fact that it _did_ add that header to every mail in the store is a separate bug).
Sorry, that should have been bug 256916 and bug 260055. Not 266916.
hmm, ok. at least updating the version number.
It's actually an e-d-s bug, because it's done in camel.
I verified that the problem still exists in evolution-data-server-1.9.4. Updating the version to 1.9.x. For reference, this was also reported downstream: http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=83257
Created attachment 98008 [details] [review] Fix for PRESERVE_HEADERS This patch enables the PRESERVE_HEADERS option in camel/camel-mime-parser.c, and fixes a bug in the end of headers detection that otherwise prevented this option from working properly. This tidies up the display of headers such as X-Spam-Report which are otherwise difficult to read without the PRESERVE_HEADERS option enabled.
Matt/Sankar, can you guys review this?
Matt/Sankar: ping
It works. But have a little issue I observed, it does preserve all headers format, which looks weird for Subject header, when it's folded and last two words of the subject are on the second line. It should be improved a bit, I think.
I think a better solution to this problem is to have custom header-folder functions for special headers like the spam header in question here. This way users don't get annoyed by folded headers like subject, but are happy with the folding of their spam headers (who so few users actually read).
Better still, mangle only the headers you _know_ you can mangle (like Subject), and leave all others alone.
the problem is that will get ugly in the parser, or I'd have recommended that instead :)
Bumping version to a stable release.
Hi Again, I noticed recently that with this patch, GPG signed messages in my Sent items folder were blank. Yesterday I tried to get to the bottom of what was happening, and have had to make a change to this patch. I can't understand why I have only recently been seeing this problem. The only thing I have changed recently is the type of IMAP server, was using dovecot, now using cyrus. Please check if I am not the only one seeing this issue. The version of evolution tested is from Debian etch, libcamel1.2-8 1.6.3-5etch1 The patch V2 should be in the attachments. Regards
Created attachment 107102 [details] [review] Fix for PRESERVE_HEADERS V2 This patch removes the header wrapping using a slightly different approach. Originally the PRESERVE_HEADERS option changes the code path for normal header processing to include the end of line, and also removes the whitespace deleting code for a header continuation. By default this option didn't work properly because it prevented the end of headers detection from working correctly. The first version of this patch appeared to cure that problem, but for some reason was exhibiting similar behaviour with GPG signed messages. Patch V2 causes the PRESERVE_HEADERS option not to modify the code on the normal header processing path, but makes changes only to the case where a continuation is detected, i.e. a line beginning with a white-space. I think this is a less intrusive approach, and keeps the header parsing clean and easier to understand. So without the PRESERVE_HEADERS option, when a continuation is detected, all white space (tabs & spaces) is reduced to a single space, and the line is appended to the current header. With the PRESERVE_HEADERS option, when a continuation is detected, the end of line character gets added to the end of the current header, then the continuation line is appended with no processing.
Fejj/Milan, Can you guys give a look at the V2 patch?
hey srag, I would prefer fejj for this, he's better in camel reviews.
the patch looks ok as far as the parser goes. how does this patch affect other areas of the mailer though? e.g. what does the message-view show, if for example, an address is folded somewhere in the middle of the name part? (folding in the middle of the domain should already be handled properly by the addrspec parser assuming it is folded legally... the lwsp should be stripped out afaik) similar goes for the other standard headers (like Subject) displayed in the message view pane... ideally this would be reflowed to the width of the view pane. the message-list view is probably the most important to make sure it works ok... the thing is that you really don't want embedded \n's making it to the UI layer unless you specifically requested a raw header. I highly suspect that other areas will need to be "fixed" to maintain the old behavior for the majority of the headers. I suspect that the only headers people care about maintaining formatting are the X-* headers (is this correct David? Craig?). That said, I think Craig's patch should go in, but I'd re-disable PRESERVE_HEADERS for the time being until either: 1. we discover that the non-X header values reflow the same as they used to in the message-list and in the view pane. 2. we have a patch to fix the non-X headers to reflow the same as before. Oh, actually... in addition to the X-* headers, the Received: headers might also be a good candidate for PRESERVE_HEADERS. Maybe I can take a look at this over the weekend a bit... but just in case I forget, here are my thoughts: 1. fix camel-mime-message.c:process_header() to unfold the values it is passed when appropriate (subject/address headers) 2. review the code that constructs the CamelMessageInfo's and see if that might get raw headers from the parser... if so, unfold them. I'm pretty sure that the above 2 fixes will make sure that the message-list renders the strings the same as before... now we just have to worry about the message-view pane: 3. review the evolution/mail/ code that renders headers in the normal views and make sure that it unfolds the headers where appropriate (obviously not for X-* headers and possibly not for Received: headers either) 4. source view should always use raw headers, no unfolding should take place How does that sound? btw, I'm going to commit your PRESERVE_HEADERS patch as it looks correct.
ok, think I've fixed all the display issues I raised. of course... I don't have a working build setup for evolution anymore so hopefully I didn't break anything ;)
*** Bug 563022 has been marked as a duplicate of this bug. ***
Did this die on the vine? Back on 2008-04-04 there was a comment saying it would be committed but in the lastest (2.24.2) evolution this is still a problem.
Hrm. Here I am again. :-/ I was about to open a new bug report about SpamAssassin's headers being mangled and BZ found me this trail of many duplicates. So 8 years ago I asked about the status of this and 8 years later, still no answer. :-( Is there any hope of this getting fixed?
Jeff committed the change [1] as he said, then he did some other folding changes in the nearby code [2]. I see one change in [1], the PRESERVE_HEADERS is not enabled [3], as Craig have it done in his patch above. That is, the code is there, only not used. Brian, do you have a simple test message with a description what evolution does and what it should do on a concrete data, please? I'd like to test it first, before enabling the Craig's change. [1] https://git.gnome.org/browse/evolution-data-server/commit/?id=8fd04a6d61 [2] https://git.gnome.org/browse/evolution-data-server/commit/?id=75070ceec6 [3] https://git.gnome.org/browse/evolution-data-server/tree/camel/camel-mime-parser.c#n44
I will attach 1. the output of a message from evolution's Save as mbox as that has the headers folded exactly as I see them when viewing the message content (ctrl-u). 2. a screenshot of the folded "X-Spam-Ham-Report:" header as I see it in an evolution window wrapped at screen width 3. the message from the system mailbox which has the headers as they should be, unfolded
Created attachment 326230 [details] comment #29, attachment #1 [details]
Created attachment 326231 [details] comment #29, attachment #2 [details]
Created attachment 326232 [details] comment #29, attachment #3 [details]
Thanks for the test message. I enabled the define provided by Craig and it works as expected (you still will not see the values aligned as in the message source, because the first value is shown beside the header name, but it's still better than before). Let's see how many things this change will break. Created commit 676056d in eds master (3.21.1+) Created commit b9c01bd in eds gnome-3-20 (3.20.2+)
A little regression reported at bug #767780.
A slightly more serious regression reported at bug #784116