Closed Bug 1109693 Opened 10 years ago Closed 9 years ago

canvas drawImage fails with data:image source and crossOrigin flag set

Categories

(Core :: Graphics: ImageLib, defect)

Product:
Core
Component:
Graphics: ImageLib
Platform:
x86
Windows 8.1
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox40 --- fixed

People

(Reporter: simon.sarris, Assigned: bzbarsky)

References

(
URL
)

Details

Attachments

(2 files)

test case that works on Chrome+IE11 and breaks on FF
7.75 KB, text/html
Details
More places that use CORS should in fact allow data: URIs
11.48 KB, patch
smaug
: review+
Details | Diff | Splinter Review 
If you create an HTML Image in JavaScript and set the img.crossOrigin flag to "anonymous", and the img.src to a data:image URI, two bad things will happen:

* The image's load event will not fire

* Attempting to draw the image to a canvas will result in the cryptic error: "NS_ERROR_NOT_AVAILABLE: "

If you do not set the crossOrigin flag (if it remains null) then these problems do not happen, so setting that flag breaks it, and that seems like a bug.

Programatically that's:


  var img = new Image();

  // This kills the fox:
  img.crossOrigin = "anonymous";

  // this won't fire if crossOrigin
  // flag is set (FF only)
  img.onload = function() {
    ctx.drawImage(img, 0,0);
  }
  img.src = "data:image/png;base64,........."

  // And this will result in
  // NS_ERROR_NOT_AVAILABLE:
  // on FF only
  setTimeout(function() {
    ctx.drawImage(img, 0,0);
  }, 1000)



Breaks in: (image does not load or draw on canvas)
  * Firefox stable and nightly
  * iOS Safari, probably desktop safari too
Works in: (load and render the image just fine)
  * Chrome
  * IE11 

Here is a live example: http://codepen.io/simonsarris/pen/raxXZa

For completeness the code is also included as an HTML attachment.
Related to bug 604496? But it's weird to see NS_ERROR_NOT_AVAILABLE rather than a security error.
I guess this is related to https://bugs.webkit.org/show_bug.cgi?id=69732
e.g. This will not display the image, but if you remove the attribute it does:
<!DOCTYPE html>
<html>
<head lang="en">
<meta charset="UTF-8">
<title></title>
</head>
<body>
<img crossorigin="anonymous" src="data:image/png;base64,R0lGODlhCwALANUAAKt5DN/ETtq+fN+/cvHkxNrBjd/MpriHEvrxctO4KfXpMrSNQNzJpffw4Pz59P/uJP79/NCzGtSoL+vZIbePQNi1Yf/bINrUY+vUaNrTdceXHdrKVcWgGTk0Fv/gKv/vOP/4ncmTEAUEAf/4gv/wONS1J0A8HRQRBP/hKtOyJz04Gv/dKZ5sCf/3HP///wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH5BAEAAC4ALAAAAAALAAsAAAZYQJfQNQgNhkiCBCQhIIUVzAhTeTY0GVNG00AKAhfVJSAYOg6ITWeDODiEhRQpcUp8SgUXBKBArf4eCgAQBhwPFhEiERYPHAYUEy2Sky0TFAwLLJqbLAsMQQA7">
</body>
</html>
Yeah, this is buggy.  We should be allowing data: here.  Specifically, https://html.spec.whatwg.org/#potentially-cors-enabled-fetch treats data: and same-origin equivalently.
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → NEW
Component: Canvas: 2D → ImageLib
Ever confirmed: true
And given that spec requirement, why exactly does nsCORSListenerProxy::Init default to not allowing data: URIs?
Flags: needinfo?(jonas)
OK, we have the following places that are currently not passing "true" here:

1)  EventSource.  Per spec at https://html.spec.whatwg.org/multipage/comms.html#the-eventsource-interface this uses "potentially CORS-enabled fetch" so looks buggy to me.

2)  Navigator.sendBeacon.  It just delegates to the fetch spec, with mode set to "CORS", so we land in https://fetch.spec.whatwg.org/#concept-fetch which only allows data: if the "same-origin data URL flag" is set.  sendBeacon does not set this flag, so disallowing data: here is correct.

3)  <script> loader.  Per spec at https://html.spec.whatwg.org/multipage/scripting.html#prepare-a-script step 14, this does "potentially CORS-enabled fetch", so we're buggy.

4)  nsSyncLoader::LoadDocument.  No spec, more restrictive here is probably OK.

5)  HTMLMediaElement::LoadResource.  https://html.spec.whatwg.org/multipage/embedded-content.html#concept-media-load-resource says to do "potentially CORS-enabled fetch".

6)  ChannelMediaResource::OpenChannel.  I expect that's the same thing as #5 in spec terms.

7)  CORS preflights.  Disallowing data: here probably makes sense.

8)  XSLT loading.  No spec here, afaict.

9)  Image loader.  That's this bug.

10) Font loader.  http://dev.w3.org/csswg/css-fonts-3/#font-fetching-requirements says to do "potentially CORS-enabled fetch"

11) CSS loader.  https://html.spec.whatwg.org/multipage/semantics.html#concept-link-obtain says to do "potentially CORS-enabled fetch".
This patch fixes items 3, 5, 6, 9, 11 from comment 6, I think.  I added tests for 3, 9,
and 11, but I'm really not sure how to go about adding tests for the media
code (hence the "I think") bit.  For the rest of the items, the situation is as follows:

Item 1: I filed bug 1156137 on the fact that this can't possibly work with
data: URIs in its current state.

Items 2, 7: Behavior is already per spec.

Items 4, 8: No spec, no change for now.  I'm not sure that's right for #8, btw.

Item 10: Turns out to not be an issue, because that code is never reached for
data: URIs; in the data: case the font loader doesn't install a
nsCORSListenerProxy at all.

It still seems to me like a bit of a footgun to have our default not match the
HTML spec default, though at least it's a footgun in the direction of "not
working" instead of "security bug"...
Attachment #8594576 - Flags: review?(jonas)
See Also: → 1156496
Blocks: 1156496
Comment on attachment 8594576 [details] [diff] [review]
More places that use CORS should in fact allow data: URIs

Jonas is out, apparently.
Attachment #8594576 - Flags: review?(jonas) → review?(bugs)
Comment on attachment 8594576 [details] [diff] [review]
More places that use CORS should in fact allow data: URIs

I think CORSListener should have a bit different kind of Init. Something which
takes an enum as the second param, and the enum values are clearly indicating whether
data: urls are accepted.
But such change could happen in a different bug.
Attachment #8594576 - Flags: review?(bugs) → review+
Blocks: 1157451
Filed bug 1157451 on making this an enum.
https://hg.mozilla.org/mozilla-central/rev/bcf0d7ae9eed
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
I have no recollection of why data: was disabled by default
Flags: needinfo?(jonas)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: