Closed Bug 794606 Opened 7 years ago Closed 7 years ago

nsDownload::Resume creates a channel out of thin air

Categories

(Toolkit :: Downloads API, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18
Tracking Status
firefox18 + fixed

People

(Reporter: jdm, Assigned: jdm)

References

Details

Attachments

(1 file, 1 obsolete file)

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/nsDownloadManager.cpp#2963

This means that a previously private download can be paused and resumed, and it won't be treated as private any more (ie. disk cache, etc.) This is bad. This will be easiest to fix after bug 722859 lands, when we'll have the per-download privacy information available.
Does that mean that bug 722859 needs to get fixed for 18 as well?
Assignee: nobody → josh
I was under the impression that was already the case, since I believe mobile is using the download manager.
Depends on: 795065
(In reply to comment #2)
> I was under the impression that was already the case, since I believe mobile is
> using the download manager.

Not sure what mobile has to do with this...  I'm talking about the impact of this as far as the cache is concerned.
Depends on: 794602, 722850
No longer depends on: 722859
The fix here is really simple. The test, on the other hand, only passes if the three dependent bugs are fixed. It's delightful.
Attachment #665784 - Flags: review?(mak77)
Also in terms of the cache, I believe we're fine. We refuse to cache any HTTP channel that is an explicit resume action (ie. has had ResumeAt called on it). Hence, I had to resort to playing with cookies to get results out of the test.
Comment on attachment 665784 [details] [diff] [review]
Make resuming a download correctly share the privacy status of the original download.

Review of attachment 665784 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/downloads/test/unit/test_private_resume.js
@@ +5,5 @@
> +this.__defineGetter__("dm", function() {
> +  delete this.dm;
> +  return this.dm = Cc["@mozilla.org/download-manager;1"].
> +                   getService(Ci.nsIDownloadManager);
> +});

just use downloadUtils.downloadManager? it's defined in head_download_manager.js

@@ +33,5 @@
> +    }
> +    let full = "";
> +    let body = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; //60
> +    for (var i = 0; i < 1000; i++)
> +      full += body;

nit: please always brace loops
Attachment #665784 - Flags: review?(mak77) → review+
Blocks: PBnGen
Blocks: mobilepb
Attachment #665784 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/0e4560cb5a35
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.