Closed Bug 2017797 Opened 1 month ago Closed 20 days ago

Clipboard2File stopped working in Firefox Nightly

Categories

(WebExtensions :: General, defect)

Firefox 149
defect

Tracking

(firefox-esr140 unaffected, firefox147 unaffected, firefox148 unaffected, firefox149 fixed, firefox150 fixed)

RESOLVED FIXED
150 Branch
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
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
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

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

Status: UNCONFIRMED → NEW
Component: Untriaged → DOM: Content Processes
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
Regressed by: 2013389

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.

Some notes:

  1. The failure is happening because the webextension sets input.files directly with a FileList object which is owned by the extension's content script global, and is therefore inaccessible from the untrusted web content.
  2. The extension itself is using structuredClone to try to copy the FileList object from its global into the target global,
  3. Due to the changes from the regressor, the FileList is 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: nobody → nika
Flags: needinfo?(nika)

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:

  1. Blob/File
  2. Directory
  3. FileList
  4. FormData
  5. ImageBitmap
  6. VideoFrame
  7. EncodedVideoChunk
  8. AudioData
  9. EncodedAudioChunk
  10. RTCEncodedVideoFrame
  11. RTCEncodedAudioFrame
  12. MessagePort
  13. OffscreenCanvas
  14. ReadableStream
  15. WritableStream
  16. TransformStream
  17. RTCDataChannel

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 structuredClone({}) would probably return an x-ray within the content script, and likely be unusable. Looked at the spec a bit (https://html.spec.whatwg.org/multipage/structured-data.html#structured-cloning), and it appears that the 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

This better aligns with the behaviour as defined in the spec, and allows us to
start passing some WPTs we were previously failing.

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?

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.

(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 FileList that 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 do cloneInto(obj, this) (this being 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.

Flags: needinfo?(rob)

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:

  • structuredClone of FormData regressed by bug 2013389, to be an instance of the content script's global instead of the web page.
    - structuredClone of FormData regression is fixed by the above patch, to be an instance of the web page's FormData instance.
  • the patch introduces a new "regression": structuredClone of 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 cloneInto first.
  • 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 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.

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 
Flags: needinfo?(rob)

(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 structuredClone method on the Sandbox object which shadows the one inherited from the Window object.

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 structuredClone which 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 structuredClone implementation 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 the structuredClone implementation.

One possibility of how to provide the webext subsystem with control over this is:

// 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); } 

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.

Flags: needinfo?(nika)
Attachment #9546603 - Attachment description: Bug 2017797 - Allow calls to structuredClone to change globals, r=asuth! → Bug 2017797 - Part 1: Allow calls to structuredClone to change globals, r=asuth!

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.

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.

Flags: needinfo?(nika)

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.

Component: DOM: Content Processes → General
Product: Core → WebExtensions

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

Attachment #9548083 - Flags: approval-mozilla-beta?

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

Attachment #9548084 - Flags: approval-mozilla-beta?

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
Attachment #9548085 - Flags: approval-mozilla-beta?

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

Regressions: 2019512
Pushed by rperta@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/a388a81f57ad https://hg.mozilla.org/integration/autoland/rev/a7cd0bac3b59 Revert "Bug 2017797 - Part 3: Preserve legacy structuredClone behaviour for WebExtensions, r=robwu,asuth" for causing build bustages at Sandbox.cpp

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 
Flags: needinfo?(nika)
Flags: needinfo?(nika)
Attachment #9548085 - Attachment is obsolete: true
Attachment #9548085 - Flags: approval-mozilla-beta?
Attachment #9548084 - Attachment is obsolete: true
Attachment #9548084 - Flags: approval-mozilla-beta?
Attachment #9548083 - Attachment is obsolete: true
Attachment #9548083 - Flags: approval-mozilla-beta?

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

Attachment #9548345 - Flags: approval-mozilla-beta?

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

Attachment #9548346 - Flags: approval-mozilla-beta?

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
Attachment #9548347 - Flags: approval-mozilla-beta?

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

Pushed by rperta@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/147fa4a7e5da https://hg.mozilla.org/integration/autoland/rev/cb17c630f6f1 Revert "Bug 2017797 - Part 3: Preserve legacy structuredClone behaviour for WebExtensions, r=robwu,asuth" for causing hazard failures at js/xpconnect/src/Sandbox.cpp:X
Status: RESOLVED → REOPENED
Flags: needinfo?(nika)
Resolution: FIXED → ---
Target Milestone: 150 Branch → ---

Backed out for causing hazard failures at js/xpconnect/src/Sandbox.cpp:X
Backout link
Push with failures
Failure log(s)

I could've sworn I ran hazard analysis. My bad.

Flags: needinfo?(nika)
Attachment #9548345 - Attachment is obsolete: true
Attachment #9548345 - Flags: approval-mozilla-beta?
Attachment #9548346 - Attachment is obsolete: true
Attachment #9548346 - Flags: approval-mozilla-beta?
Attachment #9548347 - Attachment is obsolete: true
Attachment #9548347 - Flags: approval-mozilla-beta?
Status: REOPENED → RESOLVED
Closed: 22 days ago20 days ago
Resolution: --- → FIXED
Target Milestone: --- → 150 Branch

The patch landed in nightly and beta is affected.
:nika, is this bug important enough to require an uplift?

For more information, please visit BugBot documentation.

Flags: needinfo?(nika)

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

Attachment #9549214 - Flags: approval-mozilla-beta?

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

Attachment #9549215 - Flags: approval-mozilla-beta?

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
Attachment #9549216 - Flags: approval-mozilla-beta?

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

Attachment #9549214 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9549215 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9549216 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: needinfo?(nika)
Depends on: 2020773

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:

Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.