header corruption when composing mail to several recipients

[teythoon]

Steps to reproduce:

hit m, at to> prompt, write fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo, bar, edit mail close editor.

To      fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo,                      
From    Justus Winter <4winter@informatik.uni-hamburg.de>                                                      
Subject                                                                                                        
 bar                                                                                                           
From: Justus Winter <4winter@informatik.uni-hamburg.de>                                                        
Subject:                                                                                                       

[pazz]

sry, cannot reproduce. i get:

To: fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo,
        bar                                                                                                                                          
From    Patrick Totzke <patricktotzke@gmail.com>                                                                                                      
Subject                                                                                                                                               
bodytext

[teythoon]

Maybe it is related to me using emacs as editor, I know Bjoern does too and he also suffers from this problem. Or your editor wraps long lines differently, dunno.

My point is that the header parsing used to be more robust. I remember the header and body being seperated with two newlines. I think the problem appeared when you changed that.

[pazz]

ah: i'm not editing headers in my editor. i set

edit_headers_whitelist =

[pazz]

If i have editable headers or use a template i can reproduce this. i think the problem lies in message.Envelope.parse_template.

[pazz]

the problem seems to be that in commands.globals.compose, the header values are encoded (message.encode_header) before stored to the dict Envelope.headers, which introduces newlines for long values.

One solution, (cf branch issue-135) is to delay this encoding until Envelope.construct_mail is called from commands.envelope.SendCommand upon sendout. This way, you must ensure that in the edited template, all headers are given as one-liner so that the RE in the beginnig of Envelope.parse_template can correctly detect the end of the header part of the template.

I feel that this is OK as the template is not rfc compliant message anyway due to unencoded UTF8 symbols and such. what do you think?

[teythoon]

what do you think?

I think that if you can reproduce the problem and understand its nature enough to produce a fix, you should by all means commit it. I cannot be more specific since I haven't looked at alots source for some time.

[pazz]

it was more a question if you'd be ok with a fix as

This way, you must ensure that in the edited template, all headers are given as one-liner I'm OK with this. will merge this in soon

[teythoon]

an universal tagging solution with some fancy features References: E1RamIG-0003si-K3@thinkbox.jade-hamburg.de 4ee8d62a.596ee30a.1833.ffffc33e@mx.google.com In-Reply-To: 4ee8d62a.596ee30a.1833.ffffc33e@mx.google.com ^^^

It is still happening, see? I just hit 'r' to reply and the long subject is wrapped and corrupts the headers on reparsing.

Quoting Patrick Totzke (2011-12-14 17:59:43)

[...]

Justus

[pazz]

i forgot to correctly reade the envelopes headers when writing the template. but i'm not sure if this is related.

Do you wrap long header values in your editor before saving? because envelope.parse_template assumes that headers are given as one-liner, no matter how long they are.

[teythoon]

Quoting Patrick Totzke (2011-12-19 16:39:55)

i forgot to correctly reade the envelopes headers when writing the template. but i'm not sure if this is related.

Do you wrap long header values in your editor before saving? because envelope.parse_template assumes that headers are given as one-liner, no matter how long they are.

No, even though I'm using the evil emacs editor, it does not wrap lines unless I tell it to. I blame your code :p

Justus

[pazz]

did you try out that patch though?

[teythoon]

Hm, doesn't work even with the patch:

teythoon@thinkbox ~/repos/alot (git)-[issues-135-163-multiheaders] % grep -A1 Subject /tmp/tmpiFjppz
Subject: Re: [afew] announcing afew,
 an universal tagging solution with some fancy features

This is the template written to disk, not modified by me or emacs.

[pazz]

ahhh!! i have seen this msg: for some reason the subject value here contains a newline. of course it gets written to disk and later incorrectly parsed. we should simply replace newlines with spaces before writing templates to disk. will do that soon.

[teythoon]

Well yeah, that fixed it:

Subject: Re: [afew] announcing afew,  an universal tagging solution with some fancy features

I wonder where this newline came from. I surely didn't include it in my mail, and I don't think you did it. And in my search result widget there is no newline:

   15:13pm (8) notmuch Justus, Kazuo, Jani, Patrick [afew] announcing afew, an universal tagging solution with some fancy features

whereas I do see newlines in some subjects

    Dec 10 (1)  http Invent with Python: Pyganim – A Pygame module to make sprite animation?
                        dead simple.                                                                                                           

but that might be my fault, it's a mail generated by my rss to mail solution. But alot should probably sanitize this. I'll open a separate ticket for sanitizing subjects, I've got another related issue on my list, something with top to down unicode scripts. I once tried to open a ticket for this, but github did not allow me to paste chinese characters into tickets...

[teythoon]

But note the extra space you inserted...

[pazz]

better replace \s\n\s with ' '?

[teythoon]

yeah, that sounds neat.

[teythoon]

poke

You fixed this issue in your feature branch but forgot to merge it to master.

[pazz]

i thought this was due to the recent envelope rewrite that isnt merged to master as of today. anyhow, i intend to sync testing and master soon and tag a new release 0.21 (check the milestone to see whats going in before). i plan to do this before xmas. On Dec 21, 2011 9:52 AM, "Justus Winter" < reply@reply.github.com> wrote:

poke

You fixed this issue in your feature branch but forgot to merge it to master.

---

Reply to this email directly or view it on GitHub: https://github.com/pazz/alot/issues/135#issuecomment-3231207

[teythoon]

I don't get it. You set this ticket to closed even though it isn't fixed in master? Well, what's the semantic of 'closed' then?

And what about the testing branch? I thought you were going for release branches instead of going back to the 'master is stable' approach. You know that I'm just going to get cranky again ;)

[pazz]

its not fixed in master because i think it doesnt happen in master. this issue was introduced by a post-master commit. cant check it atm though.

dtk convinced me that the master is stable approach is ok, but im open to being converted :) atm, development is done in testing, master is as stable as it gets, merging in fixes. i tag master for releases. On Dec 21, 2011 11:30 AM, "Justus Winter" < reply@reply.github.com> wrote:

I don't get it. You set this ticket to closed even though it isn't fixed in master? Well, what's the semantic of 'closed' then?

And what about the testing branch? I thought you were going for release branches instead of going back to the 'master is stable' approach. You know that I'm just going to get cranky again ;)

---

Reply to this email directly or view it on GitHub: https://github.com/pazz/alot/issues/135#issuecomment-3232050

[teythoon]

its not fixed in master because i think it doesnt happen in master. this issue was introduced by a post-master commit. cant check it atm though.

Are you kidding me? It's affecting master, it's affecting me, right now, it is annoying, you do have a fix and closed this bug report even though I've written repeatedly that I'm still affected?

[teythoon]

dtk convinced me that the master is stable approach is ok, but im open to being converted :) atm, development is done in testing, master is as stable as it gets, merging in fixes. i tag master for releases.

ymmv. this is how I see it:

1. if I send bug reports, I'll always write them against master unless explicitly stated otherwise
2. if I develop patches, they'll always be based on master unless explicitly stated otherwise

If you let your development branch diverge from master, you're the one who has to rebase or merge any changes written by others.

[pazz]

correction: the fix doesnt apply to master. it uses the structure introduced in the envelope rewrite. in particular envelope.get_all. On Dec 21, 2011 12:24 PM, "Justus Winter" < reply@reply.github.com> wrote:

its not fixed in master because i think it doesnt happen in master. this issue was introduced by a post-master commit. cant check it atm though.

Are you kidding me? It's affecting master, it's affecting me, right now, it is annoying, you do have a fix and closed this bug report even though I've written repeatedly that I'm still affected?

---

Reply to this email directly or view it on GitHub: https://github.com/pazz/alot/issues/135#issuecomment-3232566

[pazz]

On Dec 21, 2011 12:39 PM, "Justus Winter" < reply@reply.github.com> wrote:

dtk convinced me that the master is stable approach is ok, but im open to being converted :) atm, development is done in testing, master is as stable as it gets, merging in fixes. i tag master for releases.

ymmv. this is how I see it:

1. if I send bug reports, I'll always write them against master unless explicitly stated otherwise
2. if I develop patches, they'll always be based on master unless explicitly stated otherwise

If you let your development branch diverge from master, you're the one who has to rebase or merge any changes written by others.

sounds sensible to me. i see that it was not a great idea fixing this only in testing. but i was and am reluctant to fix it in two branches separately when i intend to merge them any time soon. but i get your drift: better fix it on master and rebase testing.

[teythoon]

Well, then please reopen this ticket.

I'm not sure what part of 'There is a bug in a software you wrote, affecting the version that everyone gets who does a git clone, affecting everyone using the default configuration (but not you since you chose not to view mail headers in your editor) that is easily reproducible (and I've described how to do so and shown you what the result looks like) and affecting at least two people (Bjoern and me) on a daily basis.' is so hard to comprehend...

[0x64746b]

Actually, I thinks it's perfectly fine to use 'master' as stable and 'testing' for development. Makes sure that

the version that everyone gets who does a git clone

is stable, and not bleeding edge. Sure sounds reasonable to me. Call the stable branch 'releases' and do your development in 'master' if you desire so, but I cannot exactly see why that is so much better. I'd rather call it a question of perspective?

if I send bug reports, I'll always write them against master unless explicitly stated otherwise

And why would you not? Only thing I fail to see is how you come to expect a patch will immediately be merged into a stable branch without proving itself in testing first? You need an immediate fix? Well, will have to use the bleeding edge, top of development branch? IMHO.

[pazz]

i'll stick with master as stable branch for now. You might have noticed, I merged testing into master yesterday, so this issue is fixed in master now.

[teythoon]

i'll stick with master as stable branch for now.

ok.

You might have noticed, I merged testing into master yesterday, so this issue is fixed in master now.

cool, thanks pazz, and sorry for my harsh comments.