[js-interop] Type/null check causes error on sandboxed iframe window access

[parlough]

Trying to access the contentWindow of a sandboxed iframe results in an error similar to the following:

DOMException: Failed to read a named property 'toString' from 'Window': Blocked a frame with origin "" from accessing a cross-origin frame.

I don't know enough about the web compilers or JS-interop to understand the source of the problem, but I'm assuming the .toString is added as a null check (in the case of optimized dart2js)? Whether that's why or not, it seems most property access outside of postMessage is disallowed for cross-origin frames. If I remove the toString from the generated JS, a following postMessage works fine.

I know the goal is for the new JS-interop to have less special handling by the compilers. However, to be compatible with the browser's security mechanism here, maybe a tiny bit of a special handling somewhere would be helpful?

Minimal reproduction:

<iframe id="pad" sandbox="allow-scripts allow-popups" src="frame.html"></iframe>

import 'package:web/web.dart';

void main() {
  final iFrame = document.getElementById('pad') as HTMLIFrameElement;
  iFrame.contentWindow;
}

[parlough]

Perhaps it's such a niche case we could just document it as a limitation or have some sort of helper in package:web for it rather than rely on a compiler change?

I've ended up implementing a simple extension to implement it to avoid the checks causing the error:

extension on HTMLIFrameElement {
  void safelyPostMessage(
    JSAny? message,
    String optionsOrTargetOrigin,
  ) {
    (this as JSObject)
        .getProperty<JSObject>('contentWindow'.toJS)
        .callMethod('postMessage'.toJS, message, optionsOrTargetOrigin.toJS);
  }
}

Maybe I could simplify this further too? Happy to try out other ideas if you have them :D

[srujzs]

We came across something similar here when we modified dart:html recently, and is why we use dynamic in a lot of places there. dart2js does do null checks using toString, and therefore we come across this error. Here's some related documentation: https://dart.dev/null-safety/faq#what-should-i-know-about-compiling-to-javascript-and-null-safety. Working around null checks will indeed avoid the issue.

This is kind of a tough one to solve, and will require a bit more thought on my end. Possible solutions:

1. Add helpers to avoid the null-checks. I don't exactly have a good gauge on the scope of this and what helpers users might need and where yet. It doesn't avoid the problem completely because users may still come across null checks simply because that's part of the type system.
2. Change toString to some other "safe" property get. If this is more verbose or less performant, null checks everywhere will suffer.
3. Do 2 but only for when the static type is known to be a JS object. This might be a cost that isn't too bad due to the limited scope.

[parlough]

Thanks for the details and response. It does seem a bit tough to solve, while also being limited in scope, so I'm not too worried about it yet as long as we can document it.

Do 2 but only for when the static type is known to be a JS object. This might be a cost that isn't too bad due to the limited scope.

Is this an issue for any other type besides the iframe window use case? If not, 2 and 3 seem likely not worth the implementation effort or potential size/perf impact.

---

Perhaps we can document this clearly, see how JS-interop and package:web usage evolves, and then consider introducing some helper(s) based on what we've learned.

[srujzs]

It should only affect cross-frame objects. Helpers are a good workaround and there is precedent in dart:html, but my worry is that they'll either need to carefully leverage some of the dart:js_interop_unsafe members like you are and/or abuse dynamic in some manner to avoid casts. The combination of null-checks not always being explicit/obvious makes me think that users may still come across this even when using the helpers.

To be fair, the weirdness of cross-frame objects doesn't stop here. instanceof checks also don't work as expected, so we should look to add workarounds anyways for that case.

And of course, suggestion 3 can only work when we know the static type, but I don't expect users to use Object/dynamic when we require them to use static interop.

For now, documentation is reasonable either in dart:js_interop or package:web (I'll move it to the other repo if the latter).

[sigmundch]

cc @rakudrama @fishythefish

Adding docs seems like a great start.

I worry (2) may not be feasible because it needs to be a property that all foreign cross-iframe object allow. Window allows postMessage, but is that enough? Do we have other kind of cross-iframe objects that don't have it?

Another idea similar to (3) could be to add a special type for cross-iframe objects (e.g. add JSCrossFrameObject as a subtype of JSObject), and only have special logic for that type instead.

[srujzs]

Another idea similar to (3) could be to add a special type for cross-iframe objects (e.g. add JSCrossFrameObject as a subtype of JSObject), and only have special logic for that type instead.

Extension types are erased early in dart2js, so this may require caching information to emit the right null-checks. My proposal in 3 wasn't clear, but I was thinking more when the static type (post-erasure) is interceptors.JSObject/any of the JavaScriptObject interceptors.

[sigmundch]

... I was thinking more when the static type (post-erasure) is interceptors.JSObject/any of the JavaScriptObject interceptors.

FWIW, my subsequent idea was also to make this distinction post erasure (adding an interceptors.JSCrossFrameObject that is a subtype of interceptors.JSObject)

[srujzs]

Got it, that'll work too.

[srujzs]

Adding a message here to remind myself to put this in the docs.