WebExtensions: chrome.downloads.download will not download blob created in background script

RESOLVED FIXED in Firefox 49

Status

P2
normal
RESOLVED FIXED
3 years ago
4 months ago

People

(Reporter: dw-dev, Assigned: aswan)

Tracking

45 Branch
mozilla49

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [downloads] triaged)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160407164938

Steps to reproduce:

I am in the final stage of migrating my Print Edit add-on to use the new WebExtensions APIs with the objective of having the same source code for Firefox and Chrome versions.

I have managed to work around most of the incompabibilities between Firefox and Chrome, but there is one show-stopper problem remaining: chrome.downloads.download will not download a blob created in the background script.

Here is a simplified version of the source code:

chrome.runtime.onMessage.addListener(
function(message,sender,sendResponse)
{
    chrome.tabs.query({ lastFocusedWindow: true, active: true },
    function(tabs)
    {
        var blob,objectURL;
        
        blob = new Blob( [ (new Uint8Array([ 0xEF,0xBB,0xBF ])), message.textstring ], { type : "text/html" });
        
        objectURL = window.URL.createObjectURL(blob);
        
        chrome.downloads.download({ url: objectURL, filename: tabs[0].title + ".html", saveAs: true },
        function(downloadItemId)
        {
        });
    });
});

Note: message.textstring is a long text string that is constructed in a content script.


Actual results:

When this code is executed in Firefox, it fails with the following error message:

Type error for parameter options (Error processing url: Error: Access denied for URL blob:moz-extension://52ad408b-a345-4339-b1b7-f90eec9aed20/1b2921be-704c-49fe-9fc4-82f11b303bb5) for downloads.download.

When this code is executed in Chrome, it works correctly and downloads the blob into a file.




Expected results:

The blob should be downloaded into a file.

There should not be any cross-origin issues since the blob is both created and downloaded in the background script.

Updated

3 years ago
Component: Untriaged → WebExtensions
Product: Firefox → Toolkit

Updated

2 years ago
Duplicate of this bug: 1272556

Comment 2

2 years ago
Have the same issue. Currently this is blocking my extension.

Simple example:

var blob = new Blob(['text'], {
    type: 'text/plain'
});
var url = URL.createObjectURL(blob);
browser.downloads.download({
    url: url
});

Here is my sample extension to test this bug: https://github.com/egoroof/sample-chrome-extension/tree/firefox

Updated

2 years ago
Assignee: nobody → aswan
Priority: -- → P2
Whiteboard: [downloads] triaged

Comment 3

2 years ago
I have the same problem

(In reply to mail from comment #2)
> Have the same issue. Currently this is blocking my extension.
> 
> Simple example:
> 
> var blob = new Blob(['text'], {
>     type: 'text/plain'
> });
> var url = URL.createObjectURL(blob);
> browser.downloads.download({
>     url: url
> });
> 
> Here is my sample extension to test this bug:
> https://github.com/egoroof/sample-chrome-extension/tree/firefox

Comment 4

2 years ago
I have this problem in one from my extension.
(Assignee)

Comment 5

2 years ago
Created attachment 8758772 [details]
MozReview Request: Bug 1271345 Fix brower.download.download() on blob: urls r?kmag

Calling download() on a blob URL was failing in schema validation
since we weren't propagating the extension principal all the way
to the call to scriptSecurityManager.checkLoadURI...

Review commit: https://reviewboard.mozilla.org/r/56932/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56932/
Attachment #8758772 - Flags: review?(kmaglione+bmo)
(Assignee)

Updated

2 years ago
Status: UNCONFIRMED → ASSIGNED
Iteration: --- → 49.3 - Jun 6
Ever confirmed: true
Comment on attachment 8758772 [details]
MozReview Request: Bug 1271345 Fix brower.download.download() on blob: urls r?kmag

https://reviewboard.mozilla.org/r/56932/#review53722

::: toolkit/components/extensions/ext-downloads.js:429
(Diff revision 1)
> +            try {
> -            let uri = NetUtil.newURI(options.url).QueryInterface(Ci.nsIURL);
> +              let uri = NetUtil.newURI(options.url).QueryInterface(Ci.nsIURL);
> -            target = OS.Path.join(downloadsDir, uri.fileName);
> +              filename = uri.fileName;
> +            } catch (ex) {
> +              // A blob: url doesn't support nsIURL
> +              if (ex.result != Cr.NS_NOINTERFACE) {

let uri = NetUtil.newURI(options.url);
    if (uri instanceof Ci.nsIURL) {
      filename = uri.fileName;
    }

::: toolkit/components/extensions/ext-downloads.js:433
(Diff revision 1)
> +              // A blob: url doesn't support nsIURL
> +              if (ex.result != Cr.NS_NOINTERFACE) {
> +                throw ex;
> +              }
> +            }
> +            if (!filename || filename.length == 0) {

`filename.length == 0` implies `!filename`.

This could be written as:

    target = OS.Path.join(downloadsDir, filename || "download");

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_download.html:54
(Diff revision 1)
>      Services.prefs.clearUserPref("browser.download.dir");
>    });
>  }
>  
>  function backgroundScript() {
> -  browser.test.onMessage.addListener(function(msg) {
> +  browser.test.onMessage.addListener(function(msg, ...args) {

Since you're not using `arguments` anymore, can you make this an arrow function?

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_download.html:61
(Diff revision 1)
> +      let options = args[0];
> +
> +      if (options.blobme) {
> +        let blob = new Blob(options.blobme);
> +        delete options.blobme;
> +        options.url = window.URL.createObjectURL(blob);

We should call `revokeObjectURL` after the download completes.
Attachment #8758772 - Flags: review?(kmaglione+bmo) → review+
(Assignee)

Comment 7

2 years ago
Comment on attachment 8758772 [details]
MozReview Request: Bug 1271345 Fix brower.download.download() on blob: urls r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56932/diff/1-2/
Comment on attachment 8758772 [details]
MozReview Request: Bug 1271345 Fix brower.download.download() on blob: urls r?kmag

https://reviewboard.mozilla.org/r/56932/#review53838

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_download.html:69
(Diff revisions 1 - 2)
>        // download() throws on bad arguments, we can remove the extra
>        // promise when bug 1250223 is fixed.
>        return Promise.resolve().then(() => browser.downloads.download(options))
>               .then((id) => browser.test.sendMessage("download.done", {status: "success", id}))
> -             .catch(error => browser.test.sendMessage("download.done", {status: "error", errmsg: error.message}));
> +             .catch(error => browser.test.sendMessage("download.done", {status: "error", errmsg: error.message}))
> +             .finally(() => {

Promises don't have a `finally` method...
Attachment #8758772 - Flags: review+
(Assignee)

Comment 9

2 years ago
https://reviewboard.mozilla.org/r/56932/#review53838

> Promises don't have a `finally` method...

whoops, i was having a bluebird flashback
(Assignee)

Comment 10

2 years ago
Comment on attachment 8758772 [details]
MozReview Request: Bug 1271345 Fix brower.download.download() on blob: urls r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56932/diff/2-3/
Attachment #8758772 - Flags: review?(kmaglione+bmo)
Comment on attachment 8758772 [details]
MozReview Request: Bug 1271345 Fix brower.download.download() on blob: urls r?kmag

https://reviewboard.mozilla.org/r/56932/#review53864

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_download.html:74
(Diff revisions 2 - 3)
> -             .catch(error => browser.test.sendMessage("download.done", {status: "error", errmsg: error.message}))
> -             .finally(() => {
> -               window.URL.revokeObjectURL(options.url);
> -             });
> +                      browser.test.sendMessage("download.done", {status: "success", id});
> +                    })
> +                    .catch(error => {
> +                      browser.test.sendMessage("download.done", {status: "error", errmsg: error.message});
> +                    });
> +    } else if (msg == "killTheBlob") {

Is this necessary? I'd expect this to work as long as the download has started by the time we revoke the blob URL.
Attachment #8758772 - Flags: review?(kmaglione+bmo) → review+
(Assignee)

Comment 12

2 years ago
https://reviewboard.mozilla.org/r/56932/#review53864

> Is this necessary? I'd expect this to work as long as the download has started by the time we revoke the blob URL.

Yeah, when we call `download()` there is some work involving asynchronous I/O to figure out the target path before we get around to actually starting the download (callers follow the progress and detect errors via onChanged).  If we revoke the blob right away when `download()` returns there is a race and DownloadCore can reference the revoked URL and throw.

Comment 13

2 years ago
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/332438c70582
Fix brower.download.download() on blob: urls r=kmag
https://reviewboard.mozilla.org/r/56932/#review53864

> Yeah, when we call `download()` there is some work involving asynchronous I/O to figure out the target path before we get around to actually starting the download (callers follow the progress and detect errors via onChanged).  If we revoke the blob right away when `download()` returns there is a race and DownloadCore can reference the revoked URL and throw.

OK, we should probably fix that, then. I think it's reasonable to expect add-ons to be able to revoke the blob after `download()` resolves.
(Assignee)

Comment 15

2 years ago
(In reply to Kris Maglione [:kmag] from comment #14)
> > Yeah, when we call `download()` there is some work involving asynchronous I/O to figure out the target path before we get around to actually starting the download (callers follow the progress and detect errors via onChanged).  If we revoke the blob right away when `download()` returns there is a race and DownloadCore can reference the revoked URL and throw.
> 
> OK, we should probably fix that, then. I think it's reasonable to expect
> add-ons to be able to revoke the blob after `download()` resolves.

Any thoughts on how to do that?

Comment 16

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/332438c70582
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(In reply to Andrew Swan [:aswan] from comment #15)
> (In reply to Kris Maglione [:kmag] from comment #14)
> > OK, we should probably fix that, then. I think it's reasonable to expect
> > add-ons to be able to revoke the blob after `download()` resolves.
> 
> Any thoughts on how to do that?

That's a good question. Can you check if it works in Chrome? If not, we can probably leave it as-is for now. If it does, we may have to add an extra callback so we can tell when the channel has been successfully opened.

Comment 18

2 years ago
For more input: in my chrome extension I catch downloads.onChanged and when downloads.State is "interrupted" or "complete" only then I call URL.revokeObjectURL.

The issue is now working, thanks.

Also I've got an error that "saveAs" isn't supported by Firefox. Why not this mentioned in docs https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/Downloads/download ?
(Assignee)

Comment 19

2 years ago
Implementing saveAs is tracked in bug 1247791, I've also updated the docs.  I don't think anybody here has short-term plans to implement it, but I'd be happy to help you out with it if you want to contribute...

Comment 20

2 years ago
Thanks for suggestion. No :)
In short-term you can just ignore saveAs option and continue downloading with warning in console (not breaking it). This will improve compatibility with existing Chrome extensions.

Comment 21

2 years ago
Hello again. Some input after testing.

When I call:

var blob = new Blob(['text'], {
    type: 'text/plain'
});
var url = URL.createObjectURL(blob);
browser.downloads.download({
    url: url
});

On Firefox 49 (dev) and 50 (nightly) download complete successful but in console I get:

[Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIAnnotationService.setPageAnnotation]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://app/modules/DownloadsCommon.jsm :: onDownloadChanged :: line 781"  data: no]

Stack:

onDownloadChanged() DownloadsCommon.jsm:781
this.DownloadList.prototype._notifyAllViews() DownloadList.jsm:213
DL_change() DownloadList.jsm:134
bound DL_change() self-hosted
D_notifyChange() DownloadCore.jsm:314
task_D_start() DownloadCore.jsm:555
InterpretGeneratorResume() self-hosted
next() self-hosted
TaskImpl_run() Task.jsm:319
bound TaskImpl_run() self-hosted
Handler.prototype.process() Promise-backend.js:937
this.PromiseWalker.walkerLoop() Promise-backend.js:816
bound walkerLoop() self-hosted
bound bound walkerLoop() self-hosted

It prevent some async tasks which I call in downloads.onChanged (but sync code in downloads.onChanged works). Should I open new issue for it?



The second problem - downloads.download failed with subdirectories in filename:

var blob = new Blob(['text'], {
    type: 'text/plain'
});
var url = URL.createObjectURL(blob);
browser.downloads.download({
    url: url,
    filename: 'one/file.txt'
});

This works fine in chrome and creates subdirectory "one" and file in it file.txt. It described here: https://developer.chrome.com/extensions/downloads#method-download. Is it bug or Firefox won't support it?
Can you please file a follow-up bug for the setPageAnnotation error? It looks like it shouldn't cause any real issues, since it's the last statement in a try-catch block, but it would still be good to fix.

A separate follow-up for file names in subdirectories would also be good, but I'm not sure we'll support it. And we'll need to decide how to handle platform-specific path separators.

Comment 23

2 years ago
About setPageAnnotation error - you are right, it's my fail. Not real problem.
My problem was that downloads.DownloadItem has "byExtensionName" and "byExtensionId" equal to undefined. I was using byExtensionName to check downloads and process only those who are from my ext. This is an issue but I don't use byExtensionName anymore so it isn't important for me.

Subdirectories issue is here: https://bugzilla.mozilla.org/show_bug.cgi?id=1280044

Updated

2 years ago
Duplicate of this bug: 1287347

Comment 25

a year ago
Hi,

While downloading the blob content like below code,


var pdfLink ="<html><body onload=myFunction()>	<script>function myFunction(){  window.location.assign('https://local.xyz.com/record/)}</script></body></html>"

var blob = new Blob([pdfLink], {
				    type: 'data:text/html;charset=utf-8'
				});
				var finalUrl= URL.createObjectURL(blob);


In Background.Js downloading the blob like below,

browser.downloads.download({url: finalUrl,filename:"abc.html"},function(download) {
			
			browser.downloads.onChanged.addListener(function (download) {
				console.log( download.state);
			});
			
		});

I am getting  below error as,


Security Error: Content at moz-extension://27e3d23f-12a6-4893-9849-97a7991c3b1a/_generated_background_page.html may not load data from blob:https://local.xyz.com/05fc9018-291d-47c3-ad21-291765aad500.

and after that Below message 

Unchecked lastError value: Error: Type error for parameter options (Error processing url: Error: Access denied for URL blob:https://local.xyz.com/05fc9018-291d-47c3-ad21-291765aad500) for downloads.download.

"Couple of month back it was working fine."

Please Advise,

Thanks In Advance,

Thank you,
Yogesh

Comment 26

a year ago
Anyone get a chance to look at the posted issue?
Flags: needinfo?(aswan)
(Assignee)

Comment 27

a year ago
Please open a new bug and attach a complete working example.
Flags: needinfo?(aswan) → needinfo?(dw-dev)
(Reporter)

Comment 28

a year ago
Not sure exactly what information is wanted from me.

After reported this bug, I implemented a workaround in the content script, which created a link with a 'download' attribute and simulated a click on the link.

I have not been back to test chrome.downloads.download with Print Edit WE since this bug was fixed.  The workaround works very well and is actually more efficient because there is no need to send a very long message to the background page.

I could test chrome.downloads.download with Print Edit WE if that would help.
Flags: needinfo?(dw-dev)
(Assignee)

Comment 29

a year ago
(In reply to dw-dev from comment #28)
> Not sure exactly what information is wanted from me.

Oops, I put in the wrong address for the needinfo, it was meant for the author of comment 25, but it looks like he subsequently opened bug 1369146.  Sorry for the confusion.

Updated

4 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.