1708804 - Evaluate if we need (or want) to cache dataTransfer.files when accessed for the first time from a content script

Status: NEW

(WebExtensions :: General, enhancement, P3)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

This bugzilla issue is a follow up of Bug 1707214:
In Bug 1707214 we opted to fix the most critical issue ([1]) with the less risky approach that was still going to provide a reasonable behavior also for the extension (besides unbreaking that scenario from a webpage perspective):

When an extension content script (or extension user script) access the dataTransfer.files property before the webpage can, we don't cache the FileList instance as we don't when it is accessed for the first time from system principal code.

This should be a reasonable behavior also for the extensions in most (if not all) the cases, but if for any reason we want to instead cache the FileList instance, then we should be sure to create the instance that will be cached with the same principal of the webpage.

As part of the review for the patch attached Bug 1707214, Nika provided some additional guidance about how it would be better to achieve that, which I'm quoting here for reference if we will want (or need) to follow up about this:

I think my preferred solution here would be something along the lines of this where we'd use the principal of the document which the DataTransfer is being exposed into as the principal which we cache, and ensure that we subsume that principal before fetching any information from the datatransfer (with the existing chrome exception in place for compat reasons). I don't think I'd feel super comfortable pulling that information out of the content script's expanded principal though, and ensuring that we pipe the relevant information through correctly everywhere is a much larger problem, so this feels like the smallest reasonable approach we can take.

If we were to take the other approach I was mentioning, it'd probably end up being pulled from the same logic we use for bindings to decide which global should own the JS wrapper object. Right now that value is passed into the DataTransfer constructor in most callsites as the first argument, and returned from GetParentObject, but isn't super easy to cheap to access from arbitrary code. Changing the first argument to the DataTransfer constructor to nsIGlobalObject, and keeping that value around to track which global each DataTransfer is associated with, would probably be the first step.

---

[1]: which I would summarize as "Prevent that an extension content script may be able to break the expectations of a webpage js code by intercepting a drop event and accessing dataTransfer.files before the webpage had a change to access it"

Comment 1

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

"DOM: Copy & Paste and Drag & Drop" might be more appropriate for product and component of this ticket.