Re: [evolution-patches] fix for bug #44457

[Jeffrey Stedfast <fejj ximian com>]

On Wed, 2003-06-11 at 21:12, Not Zed wrote:
> On Thu, 2003-06-12 at 07:37, Jeffrey Stedfast wrote:
> > since the mail-format.c code didn't know how to handle video/* types, it
> > had gnome-vfs sniff it. gnome-vfs told us it was text/x-troff-man and so
> > we tried to display it as text. since the content->rawtext bit was not
> > set, we assumed it was already in UTF-8 and so didn't bother to convert
> > - crash.
> > 
> > the camel portion of the patch changes rawtext to just raw and makes it
> > default to TRUE so that application/octet-stream, etc parts have this
> > bit set always. the change from rawtext to raw could be undone, I mostly
> > just did it because the name "rawtext" isn't really appropriate for
> > non-textual parts.
> 
> But it only has meaning for textual parts.  All other parts, by
> definition, are and can only be raw.  Textual parts can be un-raw if
> they're converted to utf8 - something you can't do for non-textual
> parts.  So all of that should be reverted, otherwise it confuses the
> meaning of the flag.

true. perhaps a is_utf8 flag? if we continue having the logic in camel,
we need to default a flag to say that the mime-part is likely *not* in
UTF-8 format so that if we decide in the mailer to display a non-textual
part as text, the mailer can know that it needs to convert to UTF-8
before displaying.

whether the flag stays as rawtext or becomes is_utf8, or whatever is
mostly irrelevant.

> 
> (and maybe another reason this logic should be moved up)

*nod*

> 
> > the mailer patch fixes it so that we don't sniff a mime part unless it
> > was declared as application/octet-stream (any other mime type doesn't
> > make sense to sniff, since presumably the declared mime-type is
> > correct).
> 
> I dont understand what the format/flowed text/plain check is for.  If
> we're already in that bit of code, we've already decided that the text
> is plain text, or should be treated as such.

right, we've decided that a part that was not originally marked as
text/plain is to be handled as text. BUT, if the part was originally:

Content-Type: application/foo-bar; format=<blah>

the format param has a completely separate meaning from the one defined
for text/plain parts, so we should ignore it.

> 
> The check for application/octet-stream seems ok.
> 
> > another way (smaller patch) to fix this bug would be to make it so that
> > handle_text_plain() checked the actualy mime-type of the part and if it
> > was not text/*, then set content->rawtext to TRUE, but that seemed like
> > an ugly hack so I opted for the slightly larger fix, but which seemed
> > like the more correct fix.
> 
> This doesn't make sense anyway.
> 
> The part is already supposed to have been identified by then.
> 
> 
> The important bug is a crash in the display code anyway.  A "DOS"
> attack, e.g. if someone passes such a mail with text/plain as the
> content-type, it'll still crash/abort.  Displaying rubbish is a bit
> untidy, but crashing is serious.

no, it wouldn't. it would NOT crash because camel would have
appropriately set the rawtext flag and/or converted the content to UTF-8
in the first place, thus preventing the crash.

(I tested this theory, and it displays fine without a crash if we tell
the mailer that the part is rawtext)

Jeff

> 
> 
-- 
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
fejj ximian com  - www.ximian.com