231141 - NS_GetSpecialDirectory rv isn't checked in nsDownloadsDataSource::LoadDataSource

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect)

Description

[timeless]

1542             nsresult
1543             nsDownloadsDataSource::LoadDataSource()
1544             {
1545               nsCOMPtr<nsIFile> downloadsFile;
1546               NS_GetSpecialDirectory(NS_APP_DOWNLOADS_50_FILE,
getter_AddRefs(downloadsFile));
1547                 
1548               nsCAutoString downloadsDB;
1549               NS_GetURLSpecFromFile(downloadsFile, downloadsDB);

NS_GetSpecialDirectory can fail, when it fails then downloadsFile will be null,
and 1549 will crash.

The crash looks like this:
 	necko.dll!net_GetURLSpecFromFile(nsIFile * aFile=0x00000000, nsACString &
result={...})  Line 54 + 0x7	C++
 	necko.dll!nsFileProtocolHandler::GetURLSpecFromFile(nsIFile * file=0x00000000,
nsACString & result={...})  Line 165 + 0xd	C++
 	tkitcmps.dll!NS_GetURLSpecFromFile(nsIFile * aFile=0x00000000, nsACString &
aUrl={...}, nsIIOService * ioService=0x00000000)  Line 616 + 0x1f	C++
>	tkitcmps.dll!nsDownloadsDataSource::LoadDataSource()  Line 1549 + 0x14	C++
 	tkitcmps.dll!nsDownloadManager::Init()  Line 193 + 0x15	C++
 	tkitcmps.dll!nsDownloadManagerConstructor(nsISupports * aOuter=0x00000000,
const nsID & aIID={...}, void * * aResult=0x0012e19c)  Line 62 + 0x7d	C++
 	xpcom.dll!nsGenericFactory::CreateInstance(nsISupports * aOuter=0x00000000,
const nsID & aIID={...}, void * * aResult=0x0012e19c)  Line 86 + 0x15	C++

stack from Mozilla1.6/Firebird0.8 branch (code matches trunk) checkout from
~wednesday

Comment 1

[Ali Ebrahim]

<mconnor> NEW
<mconnor> he's saying that we're not ensuring that the function returns a value,
and as a result we can crash since we pass null to the function

--> NEW
--> critical (crasher)

Comment 2

[Phil Ringnalda (:philor)]

Attachment: Do the obvious thing

Untested, since I don't know how to make NS_GetSpecialDirectory fail, but doing what all its other callers, including the analogous code in SeaMonkey's version, do must be the right thing.

Comment 3

[Phil Ringnalda (:philor)]

Attachment: In the style of the file

Oops, apparently we prefer single-line returns here.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp 	1.61
mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp 	1.53.2.6