119532 - Apparent Leak and stuff caused by bug 73250

Status: RESOLVED INVALID

(MailNews Core :: Backend, defect)

Description

[timeless]

Bug 73250 Infinite loop when displaying messages with image attachments 

1. this code appears to leak if there is a part and no messageid
2. this code makes no sense
3. i'm rewriting this code because it's a consumer of extractAttributeValue which i'm folding into nsIIOService [bug 119051].
4. you don't store part anywhere :-(
5. did i mention this code makes no sense? I can't find any consumer who actually does anything to part.

Anyways please excuse the fact that i'm whomping through this code without understanding all of it, all I know is that it isn't correct.

Note to self: these lines didn't turn up via my lxr'ings but do use part, so i'll probably want to massage them later.
/mailnews/mime/src/mimefilt.cpp, line 406 -- (PL_strstr(url, "?part=") 
||
/mailnews/mime/src/mimefilt.cpp, line 407 -- PL_strstr(url, "&part=")))
/mailnews/mime/src/mimei.cpp, line 1065 -- if (!nsCRT::strncasecmp(s, 
"?part=", 6))
/mailnews/mime/src/mimei.cpp, line 1068 -- else if (got_q && *s == '&' 
&& !nsCRT::strncasecmp(s, "&part=", 6))
/mailnews/mime/src/mimei.cpp, line 1101 -- PL_strcat(result, "&part=");
/mailnews/mime/src/mimei.cpp, line 1103 -- PL_strcat(result, "?part=");
/mailnews/mime/src/mimei.cpp, line 1116 -- !nsCRT::strcmp("part=0", 
result + L - 6))
/mailnews/mime/src/mimei.cpp, line 1144 -- PL_strcat(result, "&part=");
/mailnews/mime/src/mimei.cpp, line 1384 -- /* Compatibility with the 
"?part=" syntax used in the old (Mozilla 2.0)
/mailnews/mime/src/mimei.cpp, line 1408 -- what "?part=" references 
would do in 3.0b4 and earlier; you'll see that
/mailnews/mime/src/nsStreamConverter.cpp, line 339 -- char *part = 
PL_strcasestr(url, "?part=");
/mailnews/mime/src/nsStreamConverter.cpp, line 343 -- if (!part) part = 
PL_strcasestr(url, "&part=");

I'll attach a patch somewhere, but I would appreciate at least corrections to any incorrect conclusions that i may have drawn.

Comment 1

[timeless]

Attachment: draft

Comment 2

[timeless]

Attachment: a spoonful of glue helps the medicine go down

Comment 3

[Darin Fisher]

Comment on attachment 64915 [details] [diff] [review]
a spoonful of glue helps the medicine go down

>Index: nsMailboxUrl.cpp

>+static NS_DEFINE_CID(kIOServiceCID, NS_IOSERVICE_CID);

it's better to #include "nsNetUtil.h" and then call do_GetIOService, thus
avoiding yet another static CID instance.

Comment 4

[timeless]

fixed as part of bug 119051

Comment 5

[(not reading, please use [e-mail] instead)]

119051 was backed out.

Comment 6

[Wayne Mery (:wsmwk)]

this leak isn't too bad?  
I can't see that it's quantified anywhere - in bug 119051 or elsewhere

Comment 7

[:Irving Reid (No longer working on Firefox)]

The leak was fixed in CVS rev 1.89 by sspitzer, in 2003 (with no bug): http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsMailboxUrl.cpp&branch=&root=/cvsroot&subdir=mozilla/mailnews/local/src&command=DIFF_FRAMESET&rev1=1.88&rev2=1.89