Closed Bug 1046240 Opened 6 years ago Closed 6 years ago

Create API to derive origin URL from blob URL

Categories

(Toolkit :: Downloads API, defect, minor)

30 Branch
defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: Sander.Karas, Assigned: baku)

References

(Blocks 2 open bugs, )

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140605174243

Steps to reproduce:

Trigger a file open dialog on a link where the href attribute points to a blob.

For example:
1) Open index.html in the demo of FileSaver.js (https://github.com/eligrey/FileSaver.js)
2) Click on any of the buttons labeled save
3) A dialog box titled Opening <filename> will display


Actual results:

The open dialog box will show "from: blob:"


Expected results:

The open dialog should show "from: <domain>"

E.g. if I generated a blob from data on www.mozilla.org the dialog should show:
"from: www.mozilla.org"

Ideally it would be good to be able to set the from: field in javascript independently from the href attribute. This would allow for setting a more meaningful/user-friendly string such as "from: Company Name". I understand this could present security concerns but "from: blob:" is less secure. At least the dialog should say "from: <site>" where <site> is the site hosting the code which generated the blob/link when the file is a blob.
Severity: normal → minor
Component: Untriaged → Download Manager
OS: Windows 7 → All
Product: Firefox → Toolkit
Hardware: x86_64 → All
Baku, you came up when I asked about on IRC on how to get the URL/origin of a Blob URL, which we'd need to implement this... could you clarify how the dialog could get this information if it's handed just the Blob nsIURI?
Flags: needinfo?(amarchesini)
If your code runs with chrome privilege in C++, you can use:

nsIPrincipal* nsHostObjectProtocolHandler::GetDataEntryPrincipal(const nsACString& aUri);

to retrieve the principal. From this object, you can get the domain.
It seems that at the moment GetdataEntryPrincipal is not exposed to JS code.
Flags: needinfo?(amarchesini)
(In reply to Andrea Marchesini (:baku) from comment #2)
> If your code runs with chrome privilege in C++, you can use:
> 
> nsIPrincipal* nsHostObjectProtocolHandler::GetDataEntryPrincipal(const
> nsACString& aUri);
> 
> to retrieve the principal. From this object, you can get the domain.
> It seems that at the moment GetdataEntryPrincipal is not exposed to JS code.

To the best of my knowledge all the dialog code is in JS. Are there plans to expose this through XPCOM or other means? Ideally, what bug could be used to block this one to indicate we can fix this one?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(amarchesini)
Attached patch URL.patch (obsolete) — Splinter Review
Smaug, this patch implements URL.getPrincipalForURL for chrome-only and just for the main-thread. In workers I don't think we need it atm.
Attachment #8472277 - Flags: review?(bugs)
Flags: needinfo?(amarchesini)
Comment on attachment 8472277 [details] [diff] [review]
URL.patch

 
>+nsIPrincipal*
>+URL::GetPrincipalFromURL(const GlobalObject& aGlobal, const nsAString& aURL,
>+                         ErrorResult& aRv)
>+{
>+  if (!nsContentUtils::IsCallerChrome()) {
>+    aRv.Throw(NS_ERROR_FAILURE);
>+    return nullptr;
>+  }
Why we need this? The method is [ChromeOnly]

>+
>+  NS_LossyConvertUTF16toASCII asciiurl(aURL);
(Not sure why we use this, but URL seems to use this elsewhere too.)



>+    is(p.URI.spec, "http://mochi.test:8888/tests/dom/base/test/test_url.html", "Principal.URI is correct");
Couldn't is(p.URI.spec, "http://mochi.test:8888/tests/dom/base/test/test_url.html", "Principal.URI is correct");
be 
is(p.URI.spec, window.location.href, "Principal.URI is correct");
Attachment #8472277 - Flags: review?(bugs) → review+
Pushed to try with your comments: https://tbpl.mozilla.org/?tree=Try&rev=a715646fffa3
Attached patch URL.patchSplinter Review
Attachment #8472277 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/2691765d149c
Assignee: nobody → amarchesini
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Blocks: 1053327
Summary: Open dialog displays "from: blob:" → Create API to derive origin URL from blob URL
https://hg.mozilla.org/mozilla-central/rev/2691765d149c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla34
Ideally we would have fixed this by putting the origin of blob URLs in the blob URL as per the latest specification. That makes origin extraction way easier and would also make url.origin and such work.
(In reply to Anne (:annevk) from comment #10)
> Ideally we would have fixed this by putting the origin of blob URLs in the
> blob URL as per the latest specification. That makes origin extraction way
> easier and would also make url.origin and such work.

http://dev.w3.org/2006/webapi/FileAPI/#originOfBlobURL this is nice. can you file a bug?
Currently the API implemented in this bug is meant to be used only in chrome code.
Filed bug 1058470.
I'm still seeing this issue on Firefox 34 Beta 10 (20141117202603) using Windows 7 x64 and Mac OSX 10.9.5.

I followed the STR from Comment 0 as follows: I opened the https://github.com/eligrey/FileSaver.js page → I clicked to open the "FileSaver.js demo" link → "from: blob" appeared on every dialog box that was shown up on "Save" click.

At this point, the description of this bug is a bit confusing. Andrea, could you please elaborate a bit on what this patch should actually fix? Am I missing something?
Flags: needinfo?(amarchesini)
(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #13)
> I'm still seeing this issue on Firefox 34 Beta 10 (20141117202603) using
> Windows 7 x64 and Mac OSX 10.9.5.
> 
> I followed the STR from Comment 0 as follows: I opened the
> https://github.com/eligrey/FileSaver.js page → I clicked to open the
> "FileSaver.js demo" link → "from: blob" appeared on every dialog box that
> was shown up on "Save" click.
> 
> At this point, the description of this bug is a bit confusing. Andrea, could
> you please elaborate a bit on what this patch should actually fix? Am I
> missing something?

We will need to (file if it hasn't been and) fix the front-end to use the blob's origin in case of a blob URI. This bug added an API to extract said origin, as one didn't exist before so the frontend was 'unfixable' without adding the API first.

I don't know that it makes sense for QE to verify this bug separately, but I'll leave Andrea to comment on that.
> We will need to (file if it hasn't been and) fix the front-end to use the
> blob's origin in case of a blob URI. This bug added an API to extract said
> origin, as one didn't exist before so the frontend was 'unfixable' without
> adding the API first.

Correct. Actually this API has been converted to something better. When a blob URI is created, it contains the origin in the name. This can be used to create a proper UI.
Flags: needinfo?(amarchesini)
Blocks: 1101598
Setting qe-verify- for this bug and will follow up in Bug 1101598.
Flags: qe-verify+ → qe-verify-
You need to log in before you can comment on or make changes to this bug.