internalSave function : incorrect logic: using target file path as source path

RESOLVED INVALID

Status

()

Toolkit
Downloads API
RESOLVED INVALID
10 years ago
9 years ago

People

(Reporter: Nate Weiner, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

I do not have 100% understanding of what's happening in this function, but from my work with it I think the logic is flawed on 2 lines.  Looking at:

365     tr.init((aChosenData ? aChosenData.uri : source),
366             persistArgs.target, "", null, null, null, persist);
367     persist.progressListener = new DownloadListener(window, tr);
368     persist.saveURI((aChosenData ? aChosenData.uri : source),
369                     null, aReferrer, persistArgs.postData, null,
370                     persistArgs.target);

Noting the segment: aChosenData ? aChosenData.uri : source

I can't think of a scenario where aChosenData.uri would apply here.  From what I understand aChosenData.uri (if it exists) contains the URI of the file to save TO.  However, the argument here is suppose to be for the file to save FROM.  Therefore the other option (source) makes sense.

Basically it's saying:

PATHTOSAVETO ? PATHTOSAVETO : PATHOFSOURCEFILE

The first argument of saveURI is defined as:
@param aURI       URI to save to file. Some implementations of this interface may also support <CODE>nsnull</CODE> to imply the currently loaded URI.

Which is the file source, not the target file, that's suppose to be the argument aFile.

See: http://mxr.mozilla.org/firefox/source/embedding/components/webbrowserpersist/public/nsIWebBrowserPersist.idl#144







Reproducible: Always

Steps to Reproduce:
1.
2.
3.
(Reporter)

Comment 1

10 years ago
Here is the url to the actual part of the function:
http://mxr.mozilla.org/firefox/source/toolkit/content/contentAreaUtils.js#365
Summary: Possible that logic is wrong in internalSave function → internalSave function : incorrect logic: using target file path as source path
Component: General → Download Manager
Product: Firefox → Toolkit
QA Contact: general → download.manager
IMO there's no problem here. aChosenData.uri specifies where to get the download FROM; it is aChosenData.file (a nsILocalFile object) that specifies where to save it TO.
Yeah, looks to me like Christopher is right. Thanks for taking the time to file the bug anyways, even though it took a while before someone else took a look at it!

Comment 4

9 years ago
I agree that the code in "contentAreaUtils.js" is correct, however it is not
as clear as it could! I'm already working on making this code more straightful
in bug 471875.

Do you think this bug can be closed, since bug 471875 covers that?
Closing as INVALID per comments #2 and #3.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.