Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AV.Asset.fromFile doesn't work with user-submitted files in Firefox WebExtensions #176

Open
lxr opened this issue Nov 17, 2016 · 3 comments

Comments

@lxr
Copy link

lxr commented Nov 17, 2016

When supplying a File received from an <input type=file> or a drag-'n'-drop event to AV.Asset.fromFile in extension code, trying to actually decode the asset throws an Error: Constructing buffer with unknown type. This is because files supplies by the user come from the page context, and Firefox Xrays them before passing them on to extension code. The Xrayed property apparently propagates through the FileReader interaction in FileSource into the Uint8Array passed to the AVBuffer constructor, whose if-else switch doesn't recognize it as an instance of Uint8Array - because it's checking against the non-Xrayed type.

Firefox does provide an API for un-Xraying objects, but it's not yet available to WebExtensions, and shouldn't be necessary besides - Xrayed objects can still be read, which is all that aurora.js really cares about. An ideal fix would be one that smuggles the Xrayed buffers past the type check while still confirming that they have all the properties that AVBuffer needs, but I'm not sure what those are.

One fix that does work is simply doubly constructing the Uint8Array in the FileReader.onload callback, as in:

--- a/src/sources/browser/file.coffee
+++ b/src/sources/browser/file.coffee
@@@ -20,7 +20,7 @@@ class FileSource extends EventEmitter
         @active = true
         
         @reader.onload = (e) =>
-            buf = new AVBuffer(new Uint8Array(e.target.result))
+            buf = new AVBuffer(new Uint8Array(new Uint8Array(e.target.result)))
             @offset += buf.length
         
             @emit 'data', buf 

This works because the second Uint8Array constructor performs a memory-to-memory copy from Xrayed memory to extension memory, but it's a rather costly workaround for (to my knowledge) a Firefox-only problem.

@devongovett
Copy link
Member

Interesting. What does Object.prototype.toString.call(e.target.result) return? Normally, this returns "[object Uint8Array]". If these Xrayed objects do the same, we could use that instead of instanceof.

@lxr
Copy link
Author

lxr commented Nov 18, 2016

Aha, that works - e.target.result shows up as "[object Opaque]", but the Uint8Array created from it is indeed an "[object Uint8Array]".

Now there's a different problem, though - the subarray call in AVBuffer.slice causes an Error: Permission denied to access property "constructor". This is probably a bug in how Firefox internally implements Uint8Array.prototype.subarray, as rewriting the subarray call to something like

return new Uint8Array(this.data.buffer, this.data.byteOffset+position, Math.min(length, this.length-position))

works.

lxr pushed a commit to lxr/Google-Play-Music-Uploader that referenced this issue Nov 20, 2016
@lxr
Copy link
Author

lxr commented Nov 29, 2016

I brought the subarray thing up with the Mozilla people and they agreed it's a bug, so it'll probably get fixed in a future version of Firefox. The Uint8Array type difference still needs to be worked around by aurora.js, as they can't change it without contradicting the spec. For that, using instanceof window.Uint8Array is clearer than the Object.prototype.toString trick. It should be cross-browser compatible, though ATM one needs to use new window.Uint8Array in the FileReader.onload callback for Firefox to in fact recognize the object as a window.Uint8Array.

lxr pushed a commit to lxr/Google-Play-Music-Uploader that referenced this issue Dec 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants