Clipboard2File stopped working in Firefox Nightly
Categories
(WebExtensions :: General, defect)
Tracking
(firefox-esr140 unaffected, firefox147 unaffected, firefox148 unaffected, firefox149 fixed, firefox150 fixed)
| Tracking | Status | |
|---|---|---|
| firefox-esr140 | --- | unaffected |
| firefox147 | --- | unaffected |
| firefox148 | --- | unaffected |
| firefox149 | --- | fixed |
| firefox150 | --- | fixed |
People
(Reporter: znuj8oxeu, Assigned: nika)
References
(Regression)
Details
(Keywords: dev-doc-needed, regression)
Attachments
(6 files, 6 obsolete files)
| 48 bytes, text/x-phabricator-request | Details | Review | |
| 48 bytes, text/x-phabricator-request | Details | Review | |
| 48 bytes, text/x-phabricator-request | Details | Review | |
| 48 bytes, text/x-phabricator-request | phab-bot : approval-mozilla-beta+ | Details | Review |
| 48 bytes, text/x-phabricator-request | phab-bot : approval-mozilla-beta+ | Details | Review |
| 48 bytes, text/x-phabricator-request | phab-bot : approval-mozilla-beta+ | Details | Review |
Steps to reproduce:
Install extension https://addons.mozilla.org/en-GB/firefox/addon/clipboard2file/
Copy image to clipboard
Then go to https://codepen.io/Rupesh-RajBanshi/full/EaavLav
Select browse files button
Actual results:
Nothing happens, error in devtools console
Expected results:
File uploads
Comment 1•29 days ago | ||
INFO: Narrowed integration regression window from [8b4bceb7, bbea4c24] (3 builds) to [8b4bceb7, a5c903ab] (2 builds) (~1 steps left)
INFO: No more integration revisions, bisection finished.
INFO: Last good revision: 8b4bceb7d9503ad2bca3aeafb06e80cd4fb9c56d
INFO: First bad revision: a5c903ab0bf920957945c3c7ae7c135f8722cdcb
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8b4bceb7d9503ad2bca3aeafb06e80cd4fb9c56d&tochange=a5c903ab0bf920957945c3c7ae7c135f8722cdcb
Comment 2•29 days ago | ||
Set release status flags based on info from the regressing bug 2013389
:nika, since you are the author of the regressor, bug 2013389, could you take a look? Also, could you set the severity field?
For more information, please visit BugBot documentation.
| Assignee | ||
Comment 3•29 days ago | ||
Some notes:
- The failure is happening because the webextension sets
input.filesdirectly with aFileListobject which is owned by the extension's content script global, and is therefore inaccessible from the untrusted web content. - The extension itself is using
structuredCloneto try to copy theFileListobject from its global into the target global, - Due to the changes from the regressor, the
FileListis unfortunately copied into the extension content script global again, instead of the target window global.
I think this may have been broken for some types before my patch, hence me overlooking this case, but it was previously working that FileList structured clone calls within a WebExtension would change globals, and they no longer do. I'll post a patch to hopefully rectify this shortly.
| Assignee | ||
Comment 4•29 days ago • | ||
Looking at this a bit more, I believe this was an unintentional "feature" of the structuredClone implementation, which only worked because Clipboard2File was calling structuredClone on exactly a FileList object.
I believe that specifically, before bug 2013389, if window.structuredClone is called from a content script, the resulting object will only be in the Window's realm if it is one of the following DOM objects. All other objects are cloned into the content script's global:
Blob/FileDirectoryFileListFormDataImageBitmapVideoFrameEncodedVideoChunkAudioDataEncodedAudioChunkRTCEncodedVideoFrameRTCEncodedAudioFrameMessagePortOffscreenCanvasReadableStreamWritableStreamTransformStreamRTCDataChannel
As an example, structuredClone({ files: FileList }) would create an object in the content script's global, where files is an xray wrapper of a FileList object in the window's global. This is very inconsistent and probably not behaviour we want to restore, and changing to always create the object in the window's global could break things, as Looked at the spec a bit (https://html.spec.whatwg.org/multipage/structured-data.html#structured-cloning), and it appears that the structuredClone({}) would probably return an x-ray within the content script, and likely be unusable.structuredClone method should actually deserialize specifically into this's relevant realm, so perhaps we want to make the change anyway. Hopefully it doesn't break any extension content scripts.
As I understand it, the correct way to clone a JS object into the content scope in a WebExtension is to use cloneInto (mdn), which explicitly allows you to provide the content scope you want to clone the object into. I expect Clipboard2File should be updated to use cloneInto instead of structuredClone to fix this issue. I think this would only require changing this line: https://github.com/clipboard2file/clipboard2file/blob/886c2bafd25416f36169ba766748e91141a209f4/content/all_frames.js#L17
| Assignee | ||
Comment 5•29 days ago | ||
This better aligns with the behaviour as defined in the spec, and allows us to
start passing some WPTs we were previously failing.
Comment 6•29 days ago | ||
As I understand it, the correct way to clone a JS object into the content scope in a WebExtension is to use
cloneInto(mdn), which explicitly allows you to provide the content scope you want to clone the object into.
cloneInto is Firefox-only and predates the cross-browser structuredClone method. It is not exactly documented when an object is from the content script sandbox vs the web page (with xrays), and we generally try to make sure that it does not matter too much. So if possible I'd like to make sure that extensions can continue using the standard version of "structured cloning" where possible.
I think this would only require changing this line: https://github.com/clipboard2file/clipboard2file/blob/886c2bafd25416f36169ba766748e91141a209f4/content/all_frames.js#L17
I looked up the history of that use, and it appears to have been there from the beginning of that logic over 3 years ago: https://github.com/clipboard2file/clipboard2file/commit/97f9003b0db6836b1d5b4ed44f82b2a14775afa7
It is also a Firefox-only extension and still regularly updated, so in theory if cloneInto were to be really superior for some reason, the author could consider an update.
Nika, with the patch here, do we still expect incompatibility problems with extensions?
Comment 7•29 days ago | ||
Idle curiosity: Does it make sense that structuredClone is creating a FileList that can be accessed by the content page? I guess it does since its a method on Window. But if an extension wanted to clone an object in such a way that its not accessible by the page, would it have to do cloneInto(obj, this) (this being the sandbox global)?
I guess it doesn't matter much in practice, as I can't imagine a case where an extension would need to intentionally create an object that is exposed to the page but not actually accessible.
| Assignee | ||
Comment 8•29 days ago | ||
(In reply to Rob Wu [:robwu] from comment #6)
Nika, with the patch here, do we still expect incompatibility problems with extensions?
With the patch here, we should be aligning better with how the web standard describes structuredClone working, which is that, when called, it should clone into the scope of the this object (which in this case is the webpage itself).
This does have some risks in terms of the interaction with other extensions. If an extension's content script was using the structuredClone function to clone an object, and expecting that object to stay within the scope of the WebExtension, these changes will mean that the object has now moved into the content scope.
If we come to the conclusion that that is not the intended behaviour, we should add a new structuredClone method on the Sandbox object which shadows the one inherited from the Window object.
(In reply to Gregory Pappas [:gregp] from comment #7)
Idle curiosity: Does it make sense that structuredClone is creating a
FileListthat can be accessed by the content page?
This is what the spec says to do FWIW (https://html.spec.whatwg.org/multipage/structured-data.html#structured-cloning), as tested by this WPT: https://searchfox.org/firefox-main/rev/0fce12de99c5e7d74536d8f93cf9778ce6199a1f/testing/web-platform/tests/html/webappapis/structured-clone/structured-clone-cross-realm-method.html (which was failing before the patch on this bug).
I guess it does since its a method on
Window. But if an extension wanted to clone an object in such a way that its not accessible by the page, would it have to docloneInto(obj, this)(thisbeing the sandbox global)?
Yes, I think that is true.
I guess it doesn't matter much in practice, as I can't imagine a case where an extension would need to intentionally create an object that is exposed to the page but not actually accessible.
The main place it will come up is around xray wrappers. I think (I should probably double-check this), that the return value from structuredClone after my patch will be xray-wrapped, which could lead to some properties not being accessible without explicitly unwrapping the result.
:robwu, I don't suppose we have a way we could check if other extensions are calling the structuredClone method? Would be good to know how it's being used by other extensions in the ecosystem to make sure we're not breaking them.
Comment 9•28 days ago | ||
To make sure that I have a good understanding, I ran some tests in the console (see end of comment for snippets) that inspects uBlock Origin at example.com (EW docs: enable content script debugging). It has to be a MV2 content script because there fetch is from the content script's sandbox, in MV3 it is the web page's (bug 1578405 implemented the difference).
The test shows:
structuredCloneof FormData regressed by bug 2013389, to be an instance of the content script's global instead of the web page.
-structuredCloneof FormData regression is fixed by the above patch, to be an instance of the web page'sFormDatainstance.- the patch introduces a new "regression":
structuredCloneof a plain object returns an object from the page's scope.
A significant difference between plain objects cloned in the web page vs originated from the content script is:
- global of content script: cannot be shared with web page without using
cloneIntofirst. - global of web page: XrayWrapper around object. Limits object behavior, does not accept function property assignment for example.
To illustrate the difference:
// Demo: create object with XrayWrapper (same behavior across all versions) window.Object().x = function() {}; // throws: Uncaught Error: Not allowed to define cross-origin object as property on [Object] or [Array] XrayWrapper structuredClone({}).x = function() {}; // Fx 147: success // Fx 149: success // patch: Uncaught Error: Not allowed to define cross-origin object as property on [Object] or [Array] XrayWrapper I think that the difference is significant enough to cause breakage (see below).
:robwu, I don't suppose we have a way we could check if other extensions are calling the
structuredClonemethod? Would be good to know how it's being used by other extensions in the ecosystem to make sure we're not breaking them.
I searched in the sources of public AMO extensions (a year old copy), and looked for uses of structuredClone(. There are a few hundred public extensions that use it. I checked the top ten in terms of users, and most uses are not in content scripts. Most also use structuredClone as a way to create a deep copy of plain objects (I have not seen any use indicating DOM objects in the limited sample).
For example, the Ghostery add-on has the following snippet that runs in a content script:
// Real code in Ghostery function copyObject(data) { if (globalThis.structuredClone) { return structuredClone(data); } return JSON.parse(JSON.stringify(data)); } that snippet shows that the intended behavior: to create a deep copy of an object containing only JSON-serializable properties and values. This scenario is compatible with any of the mentioned structuredClone behaviors.
But the risk there, however, is when extensions want to do anything other than the most simple thing, e.g. assigning functions.
I'd like to be cautious with changing behaviors. Is it feasible to restore the original semantics, and introduce preferences to transition to the desired end state? Ideally we want to fix the regression in 149 first, and then have some reasonable amount of time to address the desired final state later, with longer baking in Nightly.
Appendix: test behavior of structuredClone
Snippet
r = await fetch("data:application/x-www-form-urlencoded,a=b") fd=await r.formData(); console.log(FormData === window.FormData ? "FormData is from window" : "FormData is from content script"); console.log(fd instanceof FormData ? "fd from content script as expected" : "fd NOT from content script???"); console.log(`structuredClone(fd) instanceof FormData = ${structuredClone(fd) instanceof FormData}`); console.log(`structuredClone(fd) instanceof window.FormData = ${structuredClone(fd) instanceof window.FormData}`); console.log(`structuredClone({}) instanceof Object = ${structuredClone({}) instanceof Object}`); console.log(`structuredClone({}) instanceof window.Object = ${structuredClone({}) instanceof window.Object}`); Firefox 147:
FormData is from content script fd from content script as expected structuredClone(fd) instanceof FormData = false structuredClone(fd) instanceof window.FormData = true structuredClone({}) instanceof Object = true structuredClone({}) instanceof window.Object = false Firefox Nightly (latest with patch from bug 2013389)
FormData is from content script fd from content script as expected structuredClone(fd) instanceof FormData = true --- regression structuredClone(fd) instanceof window.FormData = false --- regression structuredClone({}) instanceof Object = true structuredClone({}) instanceof window.Object = false Local build (latest code with above patch):
FormData is from content script fd from content script as expected structuredClone(fd) instanceof FormData = false structuredClone(fd) instanceof window.FormData = true structuredClone({}) instanceof Object = false structuredClone({}) instanceof window.Object = true Comment 10•27 days ago • | ||
(In reply to Rob Wu [:robwu] from comment #9)
I'd like to be cautious with changing behaviors. Is it feasible to restore the original semantics, and introduce preferences to transition to the desired end state? Ideally we want to fix the regression in 149 first, and then have some reasonable amount of time to address the desired final state later, with longer baking in Nightly.
A potential option here is to expand on Nika's proposal from comment 8:
(In reply to Nika Layzell [:nika] (ni? for response) from comment #8)
If we come to the conclusion that that is not the intended behaviour, we should add a new
structuredClonemethod on the Sandbox object which shadows the one inherited from theWindowobject.
From my perspective:
- We need our structuredClone implementation as exposed to the web to be spec-compliant. This is not just a web-extensions issue, the realm issue is manifested with iframes and pop-ups via window.open(). There are an immense number of standards issues the DOM teams look at, especially in security bugs, where the nuances of what realm an object is in are exceedingly significant because of edge cases related to windows not being "fully active" or destroyed. And with advancements in fuzzers we are also seeing more test cases where these cross-global edge cases potentially come up, although we also do regularly see situations in the wild.
- It sounds like all the usages we are seeing are using a bare invocation of
structuredClonewhich is obtained from the window global by means of the prototype chain defined on the sandbox which means we do have an opportunity here to do as Nika suggests, plus... - It is okay to put a weird/buggy
structuredCloneimplementation in the content script sandbox, but I think we should really minimize how weird and buggy it is and in particular limit its impact on the core StructuredCloneHolder implementation and ideally thestructuredCloneimplementation.
One possibility of how to provide the webext subsystem with control over this is:
- Create an optional global
sandboxStructuredClonein sandboxes that can be wanted by the content script sandbox creation and inherently will thereby clone into the sandbox global. - Leverage the evalInSandbox mechanism already used to poke stuff into the content script sandbox to poke in something like the following:
// Shim to temporarily maintain limited consistency with prior buggy behavior // in structuredClone documented in bug 2017797 where specifically handled // DOM objects like FormData would be cloned into the content global but // anything handled by the JS engine would instead be cloned into the calling // global. // // The spec is clear that structuredClone must clone into the realm (global) // that the function exists in, so this shim helps maintain the behavior that a // naive use of structuredClone in the content script on a regular JS object // will be cloned into the sandbox global using sandboxStructuredClone // (which is just structuredClone exposed in the sandbox global with a // different name), but attempts to directly clone an object of a type like // FormData that previously would have ended up in the content global will // be cloned using the window's structuredClone method. function structuredClone(obj) { // Additional checks can be added for when we should fall back to broken // legacy behavior. if (obj instanceof FormData) { return window.structuredClone(obj); } return sandboxStructuredClone(obj); } | Assignee | ||
Comment 11•27 days ago | ||
As :asuth mentioned, I think we want to align our behaviour here with web standards (i.e. we want to use the target global for everything). For something like window.structuredClone() we will probably need it to always be the new semantics, but if extensions are calling a bare structuredClone or globalThis.structuredClone, we could add a structuredClone method on the content script sandbox global which simulates the old broken behaviour where some objects would change globals, and others would not.
I think the approach would probably end up being similar to the example above. We'd have a limited allow-list of types which should be cloned into the target global, and the remaining objects would remain within the extension's global. I could see it making sense long-term for globalThis.structuredClone to continue to clone within the extension content script's global, and for only an explicit call like window.structuredClone to clone into the target window's global.
Leaving a ni? for myself so that I look into implementing this next week.
Updated•25 days ago |
| Assignee | ||
Comment 12•25 days ago | ||
The implementaion of IS_INSTANCE_OF previously specified the true native type T
to UnwrapObjectInternal. This is unnecessary, as it is not part of the actual
"IsInstanceOf" check, and is only used for a static cast (which is then
discarded).
This patch changes the passed-in type to be void. This change should have no
runtime effect (the value was assigned into a void* immediately anyways), but
avoids requiring callers which use IS_INSTANCE_OF to import the relevant
bindings headers.
| Assignee | ||
Comment 13•25 days ago | ||
This change attempts to preserve the behaviour from before bug 2013389 for
WebExtensions, putting the janky type-dependant behaviour behind a pref. This
is done by making the webextension content script globals shadow
'structuredClone' with an alternative implementation based in the content
script's global.
This alternative implementation will, when being called from a webextension
content script, only change the global of the cloned object when called on one
of the set of DOM objects which previously were impacted by the bug.
Note that previously 'structuredClone' would do this odd behaviour even for a
nested object, but this is very unlikely to happen intentionally (and would be
much more difficult to implement), so that behaviour has not been preserved.
| Assignee | ||
Updated•25 days ago |
Comment 14•24 days ago | ||
For better visibility to the WebExtensions team, I'm moving this bug to the WebExtensions product. Especially because the changes are intended to cover extension behavior only.
Updated•24 days ago |
| Assignee | ||
Comment 15•23 days ago | ||
This better aligns with the behaviour as defined in the spec, and allows us to
start passing some WPTs we were previously failing.
Original Revision: https://phabricator.services.mozilla.com/D284163
Updated•23 days ago |
| Assignee | ||
Comment 16•23 days ago | ||
The implementaion of IS_INSTANCE_OF previously specified the true native type T
to UnwrapObjectInternal. This is unnecessary, as it is not part of the actual
"IsInstanceOf" check, and is only used for a static cast (which is then
discarded).
This patch changes the passed-in type to be void. This change should have no
runtime effect (the value was assigned into a void* immediately anyways), but
avoids requiring callers which use IS_INSTANCE_OF to import the relevant
bindings headers.
Original Revision: https://phabricator.services.mozilla.com/D284522
Updated•23 days ago |
Comment 17•23 days ago | ||
firefox-beta Uplift Approval Request
- User impact if declined: Regression in structuredClone behaviour in extensions after bug 2013389.
- Code covered by automated testing: yes
- Fix verified in Nightly: yes
- Needs manual QE test: no
- Steps to reproduce for manual QE testing:
- Risk associated with taking this patch: medium
- Explanation of risk level: This is a fairly straightforward fix, though it does impact a somewhat complex component (content scripts).
- String changes made/needed: None
- Is Android affected?: yes
| Assignee | ||
Comment 18•23 days ago | ||
This change attempts to preserve the behaviour from before bug 2013389 for
WebExtensions, putting the janky type-dependant behaviour behind a pref. This
is done by making the webextension content script globals shadow
'structuredClone' with an alternative implementation based in the content
script's global.
This alternative implementation will, when being called from a webextension
content script, only change the global of the cloned object when called on one
of the set of DOM objects which previously were impacted by the bug.
Note that previously 'structuredClone' would do this odd behaviour even for a
nested object, but this is very unlikely to happen intentionally (and would be
much more difficult to implement), so that behaviour has not been preserved.
Original Revision: https://phabricator.services.mozilla.com/D284524
Comment 19•23 days ago | ||
Comment 20•22 days ago | ||
Comment 21•22 days ago | ||
Backed out for causing build bustages at Sandbox.cpp
Backout link
Push with failures
Failure log(s)
Failure lines:
/builds/worker/checkouts/gecko/js/xpconnect/src/Sandbox.cpp:X:25: error: no member named 'RTCEncodedVideoFrame' in namespace 'mozilla::dom::prototypes::id' /builds/worker/checkouts/gecko/js/xpconnect/src/Sandbox.cpp:X:25: error: no member named 'RTCEncodedAudioFrame' in namespace 'mozilla::dom::prototypes::id' /builds/worker/checkouts/gecko/js/xpconnect/src/Sandbox.cpp:X:25: error: no member named 'RTCDataChannel' in namespace 'mozilla::dom::prototypes::id' gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:670: Sandbox.o] Error 1 gmake[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:72: js/xpconnect/src/target-objects] Error 2 gmake[2]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:34: compile] Error 2 gmake[1]: *** [/builds/worker/checkouts/gecko/config/rules.mk:359: default] Error 2 gmake: *** [client.mk:59: build] Error 2 | Assignee | ||
Updated•22 days ago |
Updated•22 days ago |
Updated•22 days ago |
Updated•22 days ago |
| Assignee | ||
Comment 22•22 days ago | ||
This better aligns with the behaviour as defined in the spec, and allows us to
start passing some WPTs we were previously failing.
Original Revision: https://phabricator.services.mozilla.com/D284163
Updated•22 days ago |
| Assignee | ||
Comment 23•22 days ago | ||
The implementaion of IS_INSTANCE_OF previously specified the true native type T
to UnwrapObjectInternal. This is unnecessary, as it is not part of the actual
"IsInstanceOf" check, and is only used for a static cast (which is then
discarded).
This patch changes the passed-in type to be void. This change should have no
runtime effect (the value was assigned into a void* immediately anyways), but
avoids requiring callers which use IS_INSTANCE_OF to import the relevant
bindings headers.
Original Revision: https://phabricator.services.mozilla.com/D284522
Updated•22 days ago |
Comment 24•22 days ago | ||
firefox-beta Uplift Approval Request
- User impact if declined: Regression in structuredClone behaviour in extensions after bug 2013389.
- Code covered by automated testing: yes
- Fix verified in Nightly: yes
- Needs manual QE test: no
- Steps to reproduce for manual QE testing:
- Risk associated with taking this patch: medium
- Explanation of risk level: This is a fairly straightforward fix, though it does impact a somewhat complex component (content scripts).
- String changes made/needed: None
- Is Android affected?: yes
| Assignee | ||
Comment 25•22 days ago | ||
This change attempts to preserve the behaviour from before bug 2013389 for
WebExtensions, putting the janky type-dependant behaviour behind a pref. This
is done by making the webextension content script globals shadow
'structuredClone' with an alternative implementation based in the content
script's global.
This alternative implementation will, when being called from a webextension
content script, only change the global of the cloned object when called on one
of the set of DOM objects which previously were impacted by the bug.
Note that previously 'structuredClone' would do this odd behaviour even for a
nested object, but this is very unlikely to happen intentionally (and would be
much more difficult to implement), so that behaviour has not been preserved.
Original Revision: https://phabricator.services.mozilla.com/D284524
Comment 26•22 days ago | ||
Comment 27•22 days ago | ||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/8eb6ce545053
https://hg.mozilla.org/mozilla-central/rev/8ec4629a4bfe
https://hg.mozilla.org/mozilla-central/rev/47d4568f3691
Comment 28•21 days ago | ||
Comment 29•21 days ago | ||
Backed out for causing hazard failures at js/xpconnect/src/Sandbox.cpp:X
Backout link
Push with failures
Failure log(s)
| Assignee | ||
Comment 30•21 days ago | ||
I could've sworn I ran hazard analysis. My bad.
Updated•21 days ago |
Updated•21 days ago |
Updated•21 days ago |
Comment 31•21 days ago | ||
Comment 32•20 days ago | ||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/44e1ee2d5d38
https://hg.mozilla.org/mozilla-central/rev/c319e7304841
https://hg.mozilla.org/mozilla-central/rev/eef79e2a6453
Comment 33•18 days ago | ||
The patch landed in nightly and beta is affected.
:nika, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval. Also, don't forget to request an uplift for the patches in the regression caused by this fix.
- See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
- If no, please set
status-firefox149towontfix.
For more information, please visit BugBot documentation.
| Assignee | ||
Comment 34•18 days ago | ||
This better aligns with the behaviour as defined in the spec, and allows us to
start passing some WPTs we were previously failing.
Original Revision: https://phabricator.services.mozilla.com/D284163
Updated•18 days ago |
| Assignee | ||
Comment 35•18 days ago | ||
The implementaion of IS_INSTANCE_OF previously specified the true native type T
to UnwrapObjectInternal. This is unnecessary, as it is not part of the actual
"IsInstanceOf" check, and is only used for a static cast (which is then
discarded).
This patch changes the passed-in type to be void. This change should have no
runtime effect (the value was assigned into a void* immediately anyways), but
avoids requiring callers which use IS_INSTANCE_OF to import the relevant
bindings headers.
Original Revision: https://phabricator.services.mozilla.com/D284522
Updated•18 days ago |
Comment 36•18 days ago | ||
firefox-beta Uplift Approval Request
- User impact if declined: Regression in structuredClone behaviour in extensions after bug 2013389.
- Code covered by automated testing: yes
- Fix verified in Nightly: yes
- Needs manual QE test: no
- Steps to reproduce for manual QE testing:
- Risk associated with taking this patch: medium
- Explanation of risk level: This is a fairly straightforward fix, though it does impact a somewhat complex component (content scripts).
- String changes made/needed: None
- Is Android affected?: yes
| Assignee | ||
Comment 37•18 days ago | ||
This change attempts to preserve the behaviour from before bug 2013389 for
WebExtensions, putting the janky type-dependant behaviour behind a pref. This
is done by making the webextension content script globals shadow
'structuredClone' with an alternative implementation based in the content
script's global.
This alternative implementation will, when being called from a webextension
content script, only change the global of the cloned object when called on one
of the set of DOM objects which previously were impacted by the bug.
Note that previously 'structuredClone' would do this odd behaviour even for a
nested object, but this is very unlikely to happen intentionally (and would be
much more difficult to implement), so that behaviour has not been preserved.
Original Revision: https://phabricator.services.mozilla.com/D284524
Updated•17 days ago |
Updated•17 days ago |
Updated•17 days ago |
Updated•17 days ago |
Comment 38•17 days ago | ||
| uplift | ||
| Assignee | ||
Updated•17 days ago |
Comment 39•15 days ago | ||
dev-doc-needed: for Firefox 149.
web facing change:
In Firefox 149, the structuredClone implementation was changed to more closely match the specification, to instantiate objects in the realm of this. Previously, structuredClone.call(iframe.contentWindow) would incorrectly create objects in the caller's realm instead of the iframe's realm.
extension-specific impact (connected to the above web facing change)
For compatibility with previous Firefox versions, a new global structuredClone (globalThis.structuredClone) method is introduced to extension content scripts, which clones objects in the scope of the content script. This global shadows the (updated) window.structuredClone method, which now creates cloned values in the page's realm instead of the content script's.
The window.structuredClone method (identical to self.structuredClone) is a standard (cross-browser) way to clone structurally cloneable values in the page's realm. Firefox also has a cloneInto method that predated structuredClone, that offers functionality beyond structured cloning, such as exporting functions (these features of cloneInto are already documented).
Web pages and extensions commonly use structuredClone to make a deep copy of a value. They call structuredClone with the expectation that the result is a copy of the original. If the clone were to be using the realm of the web page, then the values are security wrappers around the values from the page, which forbids things such as function property assignments. To illustrate the difference:
let obj = {}; let copy = structuredClone(obj); copy.prop = function() {}; // expected to work. let pageCopy = window.structuredClone(obj); pageCopy.prop = function() {}; // Error: Not allowed to define cross-origin object as property In Firefox 148 and earlier, the structuredClone global is identical to the web page's window.structuredClone, and behaved like the first example above. In Firefox 149, structuredClone (also available as globalThis.structuredClone) is a special content script-specific version of the method that maintains the same pre-149 behavior, to shadow the (updated) window.structuredClone method that could have broken extensions that expected the original semantics.
Good to know for developers of cross-browser extensions: structuredClone, globalThis.structuredClone, window.structuredClone and self.structuredClone are identical in other browsers. Firefox is the only browser that have different behaviors due to differences in the content script environment.
Content to update:
- structuredClone article linking to extension-specific
cloneInto - mentioning structuredClone in cloneInto MDN article
- mentioning structuredClone in https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Sharing_objects_with_page_scripts
- look for more pages referencing cloneInto and adjust documentation if needed, e.g. Ctrl+F cloneInto at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Content_scripts
Description
•