Bugzilla

(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 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:
- Create an optional global `sandboxStructuredClone` in sandboxes that can be [wanted by the content script sandbox creation](https://searchfox.org/firefox-main/rev/35009c6b3c3a10d189dd24790b5aaa6a27b6058f/toolkit/components/extensions/ExtensionContent.sys.mjs#1048,1051) and inherently will thereby clone into the sandbox global.
- Leverage the [evalInSandbox mechanism already used to poke stuff into the content script sandbox](https://searchfox.org/firefox-main/rev/35009c6b3c3a10d189dd24790b5aaa6a27b6058f/toolkit/components/extensions/ExtensionContent.sys.mjs#1077-1095,1100) to poke in something like the following:

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

(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](https://searchfox.org/firefox-main/rev/35009c6b3c3a10d189dd24790b5aaa6a27b6058f/toolkit/components/extensions/ExtensionContent.sys.mjs#1060) 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:
- Create an optional global `sandboxStructuredClone` in sandboxes that can be [wanted by the content script sandbox creation](https://searchfox.org/firefox-main/rev/35009c6b3c3a10d189dd24790b5aaa6a27b6058f/toolkit/components/extensions/ExtensionContent.sys.mjs#1048,1051) and inherently will thereby clone into the sandbox global.
- Leverage the [evalInSandbox mechanism already used to poke stuff into the content script sandbox](https://searchfox.org/firefox-main/rev/35009c6b3c3a10d189dd24790b5aaa6a27b6058f/toolkit/components/extensions/ExtensionContent.sys.mjs#1077-1095,1100) to poke in something like the following:

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