Roundup Tracker - Issues

Message3679

Author yyz
Recipients ajaksu2, schlatterbeck, yyz
Date 2009-04-02.23:46:35
Message-id <20090402234554.GB11935@invocation.cs.utexas.edu>
In-reply-to <1238711595.31.0.788377411768.issue2550534@psf.upfronthosting.co.za>
thus spake Daniel Diniz <issues@roundup-tracker.org>: 
> The change to multipart to allow attachments looks good to me too, but
> I'm not sure if it's the only/best way to do it (nothing against it,
> just lack of knowledge on my part).

Since the code appeared to me to want to attach the original message
to the new bounce message that already contained the error (created as
a MIMEText), then it seemed you either go Multipart and attach them,
or concatenate them as text.  The code appeared to support the first
option more strongly so I went with that.

> This bit below I don't understand:
>          else:
> -            body.write(bounced_message.fp.read())
> -        part = MIMEText(bounced_message.fp.read())
> -        part['Content-Disposition'] = 'attachment'
> -        for header in bounced_message.headers:
> -            part.write(header)
> +            part = MIMEText(bounced_message.fp.read())
>          message.attach(part)
> 
> Again, not that I know it's wrong, I just can't say it's correct because
> it's not obvious. I can't even understand it before the patch :/

I have to admit this is me guessing based on the subversion history of
mailer.py (thank you svn blame).

As it had been written it wouldn't work at all, but I found this
in svn -r1866:

        body.write('\n')
        try:
            bounced_message.rewindbody()
        except IOError, message:
            body.write("*** couldn't include message body: %s ***"
                       % bounced_message)
        else:
            body.write(bounced_message.fp.read())

Oh, darn it. You know what, I don't think this part of my patch is entirely correct.
It shouldn't crash, so in that way it's better than before, but it
doesn't preserve the original desired behavior, which I believe is to
send along the headers as well...

New patch attached. Sorry for all this churn.
Files
File name Uploaded
roundupmailer.patch yyz, 2009-04-02.23:46:34
History
Date User Action Args
2009-04-02 23:46:36yyzsetrecipients: + yyz, schlatterbeck, ajaksu2
2009-04-02 23:46:36yyzlinkissue2550534 messages
2009-04-02 23:46:35yyzcreate