Last Comment Bug 702820 - Allow XHR to data URL
: Allow XHR to data URL
Status: RESOLVED FIXED
[parity-Opera]
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla12
Assigned To: Masatoshi Kimura [:emk]
:
Mentors:
Depends on: 716820 727530
Blocks: 716489
  Show dependency treegraph
 
Reported: 2011-11-15 16:10 PST by Masatoshi Kimura [:emk]
Modified: 2012-04-24 06:19 PDT (History)
9 users (show)
dao+bmo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (7.15 KB, patch)
2011-12-26 06:26 PST, Masatoshi Kimura [:emk]
jonas: review+
Details | Diff | Review
patch v2 (8.83 KB, patch)
2012-01-09 07:45 PST, Masatoshi Kimura [:emk]
jonas: review+
Details | Diff | Review

Description Masatoshi Kimura [:emk] 2011-11-15 16:10:57 PST
Per bug 699857, data URIs are considered as same-origin in Gecko.
We already allow XHR to blob URLs. Again, there is no point rejecting only data URLs.
Comment 1 Henri Sivonen (:hsivonen) 2011-11-24 04:17:08 PST
This makes it harder for JavaScript libraries to detect HTML parsing support in XHR locally (without GETing an HTTP resource).
Comment 2 Masatoshi Kimura [:emk] 2011-12-26 06:26:58 PST
Created attachment 584323 [details] [diff] [review]
patch
Comment 3 Olli Pettay [:smaug] 2011-12-26 07:28:17 PST
Comment on attachment 584323 [details] [diff] [review]
patch

Sicking should review all the CORS changes.

And this needs tests
Comment 4 Jonas Sicking (:sicking) 2012-01-09 01:25:24 PST
Comment on attachment 584323 [details] [diff] [review]
patch

The @font-face should possibly also use this new mechanism. It currently uses a different way to enable data-urls together with CORS-listeners.

However it's possible that the @font-face code needs to keep doing special things since it might want to sync-load data urls. If that's not the case, then please file a followup or attach another patch here.
Comment 5 :Ms2ger 2012-01-09 03:23:34 PST
I'm wondering if having xhr.status return 0 is a good idea. We do that file:// urls, and that's bitten me a number of times already
Comment 6 Masatoshi Kimura [:emk] 2012-01-09 03:31:33 PST
> However it's possible that the @font-face code needs to keep doing special
> things since it might want to sync-load data urls. If that's not the case,
> then please file a followup or attach another patch here.
Files bug 716489.
Comment 7 Henri Sivonen (:hsivonen) 2012-01-09 03:33:36 PST
(In reply to Ms2ger from comment #5)
> I'm wondering if having xhr.status return 0 is a good idea. We do that
> file:// urls, and that's bitten me a number of times already

FWIW, I have been confused by status returning 0 instead of 200 for successful non-HTTP responses. Maybe a follow-up bug?
Comment 8 Masatoshi Kimura [:emk] 2012-01-09 03:42:47 PST
(In reply to Henri Sivonen (:hsivonen) from comment #7)
> (In reply to Ms2ger from comment #5)
> > I'm wondering if having xhr.status return 0 is a good idea. We do that
> > file:// urls, and that's bitten me a number of times already
> 
> FWIW, I have been confused by status returning 0 instead of 200 for
> successful non-HTTP responses. Maybe a follow-up bug?

Filed bug 716491.
Comment 9 Masatoshi Kimura [:emk] 2012-01-09 07:45:33 PST
Created attachment 586993 [details] [diff] [review]
patch v2

The previous patch caused test failure.
Also added a test case.
Try result:
https://tbpl.mozilla.org/?tree=Try&rev=4407d2e1a107
Comment 10 Boris Zbarsky [:bz] 2012-01-09 21:29:05 PST
Hmm.  Do we really want to check for the "data" scheme here?  Or for the "inherits security context" flag?  Seems like the latter would make more sense to me....  (General note: any time you're checking string schemes the code is probably wrong.)
Comment 11 Masatoshi Kimura [:emk] 2012-01-09 23:07:54 PST
I'm not sure we should allow XHR to "wyciwyg:"/"javascript:" URIs.
(Opera do not allow XHR to "javascript:" URIs)
Comment 12 Boris Zbarsky [:bz] 2012-01-09 23:09:47 PST
> I'm not sure we should allow XHR to "wyciwyg:"/"javascript:" URIs.

Why not?

And even if we grant that, do you want to allow it to all LOCAL_RESOURCE urls that inherit the security context?
Comment 13 Boris Zbarsky [:bz] 2012-01-09 23:11:04 PST
Actually.... shouldn't this check just be identical to the SVG image stuff in nsDataDocumentContentPolicy::ShouldLoad?  That is, should XHR to a -moz-filedata URL, say, work?
Comment 14 Masatoshi Kimura [:emk] 2012-01-09 23:16:51 PST
XHR to blob URIs already works since Firefox 9.
Comment 15 Masatoshi Kimura [:emk] 2012-01-09 23:31:14 PST
(In reply to Masatoshi Kimura [:emk] from comment #14)
> XHR to blob URIs already works since Firefox 9.
Sorry, since Firefox 6 (that is, since MozBlobBuilder has been supported).
Comment 16 Masatoshi Kimura [:emk] 2012-01-10 00:24:18 PST
Filed bug 716820 because Workers also check the hard-coded scheme name.
Comment 17 Boris Zbarsky [:bz] 2012-01-10 06:53:34 PST
> XHR to blob URIs already works since Firefox 9.

OK, why?  What sort of check is _that_ doing?

Keep in mind that extensions can implement protocols too; the question is which of those XHR should work to.
Comment 18 Masatoshi Kimura [:emk] 2012-01-10 07:49:20 PST
(In reply to Boris Zbarsky (:bz) from comment #17)
> OK, why?  What sort of check is _that_ doing?
Apparently that's because blob URIs implement nsIURIWithPrincipal.
NS_SecurityCompareURIs will get a principal URI if the URI implements the interface.
For data URIs, NS_SecurityCompareURIs will fail because data URIs don't support nsIURIWithPrincipal and the scheme doesn't match with the loading document ("data" vs "http"/"file").
Comment 19 Boris Zbarsky [:bz] 2012-01-10 08:32:09 PST
Ah, ok.  That makes sense.
Comment 21 Ed Morley [:emorley] 2012-01-11 18:05:21 PST
https://hg.mozilla.org/mozilla-central/rev/9f1349e72752
Comment 22 Eric Shepherd [:sheppy] 2012-04-24 06:19:24 PDT
Documented:

https://developer.mozilla.org/en/DOM/XMLHttpRequest#Gecko_notes

Mentioned on Firefox 12 for developers.

Note You need to log in before you can comment on or make changes to this bug.