Skip to content

dart:j2 - Cross-frame native objects are always proxied in dart2js, but transferred directly in Dartium #15053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DartBot opened this issue Nov 13, 2013 · 21 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-obsolete Closed as the reported issue is no longer relevant library-js P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@DartBot
Copy link

DartBot commented Nov 13, 2013

This issue was originally filed by ross.dart...@gmail.com


0.8.10_r30107
Windows 7x64

The documentation of dart:js states:

"The following types are transferred directly and not proxied:

...
Blob
..."

What I am seeing is that they are transferred directly in Dartium (VM) but still proxied in dart2js compiled javascript (Chrome Beta). This is causing me problems. I don't have an isolated repro handy but I could make one if need be...

thanks,

@anders-sandholm
Copy link
Contributor

Added Area-Library, Library-JS, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Jan 22, 2014

This comment was originally written by dev...@futureperfect.info


I have found the problem in js_dart2js.dart:

https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/sdk/lib/js/dart2js/js_dart2js.dart

In the function _convertToDart the test for _isLocalObject returns false. This is possibly a Chrome App specific issue. If I modify the code as follows, the issue goes away:

  } else if (/_isLocalObject(o)
      &&
/ (o is Blob || o is Event || o is KeyRange || o is ImageData
      || o is Node || o is TypedData || o is Window)) {
    // long line: dart2js doesn't allow string concatenation in the JS() form
    return JS('Blob|Event|KeyRange|ImageData|Node|TypedData|Window', '#', o);
  }

Is that test really needed?

@justinfagnani
Copy link
Contributor

The test should be there as part of our policy of limiting cross-frame access. In this case I think according to the behavior we want, that Dartium should be proxying the objects. Chrome apps are throwing new things at us in terms of objects crossing context boundaries, so we might have to revisit our thinking here. Adding Jacob and Pete.


cc @jacob314.
cc @blois.

@jacob314
Copy link
Member

As I understand it: that test makes sense to protect against objects that are really from a different frame but makes little intuitive sense for objects that are from the same frame but have a different JS security context.
Sra, how hard would it be to handle this case safely in Dart2Js? Will things quietly go off the rails if JS Blob objects from a different security context are used?


cc @rakudrama.
Set owner to @justinfagnani.

@blois
Copy link

blois commented Jan 28, 2014

There should not be a security concern at this level- that's enforced by the browser elsewhere.

Quick glance and this appears to be because of differences between:
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/sdk/lib/html/html_common/conversions.dart#­145

And:
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/sdk/lib/html/html_common/conversions.dart#­245

We write File and Blob with no conversion, but do not read them.

@DartBot
Copy link
Author

DartBot commented Jan 28, 2014

This comment was originally written by dev...@futureperfect.info


Thanks for looking into this guys. I agree that security should not be a concern of the interop layer; I can perform all of the same operations with the proxy object as with the non-proxy object, it just requires me to write a lot more wrapper code.

@blois
Copy link

blois commented Jan 28, 2014

Can you clarify if the issue is dart:html serialization or dart:js interop? What is the API which is failing to transfer the type?

@DartBot
Copy link
Author

DartBot commented Jan 29, 2014

This comment was originally written by dev...@futureperfect.info


I believe it to be a dart:js interop issue but there may be additional issues elsewhere. Removing the test for _isLocalObject(o) fixes the issue as I mentioned above and if that test is solely for security I think it should be removed.

This issue affects much of the chrome.fileSystem API surface. I have uploaded a small reproduction Chrome App with instructions here:

https://gist.github.com/rmsmith/8684111

The example uses chooseEntry for a convenient example but the issue is not limited to this example.

Thanks

@justinfagnani
Copy link
Contributor

Removed Priority-Unassigned label.
Added Priority-Medium, New labels.

@justinfagnani
Copy link
Contributor

Added Triaged label.

@justinfagnani
Copy link
Contributor

Issue #17330 has been merged into this issue.

@justinfagnani
Copy link
Contributor

Issue #17086 has been merged into this issue.

@justinfagnani
Copy link
Contributor

Changed the title to: "dart:j2 - Cross-frame native objects are always proxied in dart2js, but transferred directly in Dartium".

@justinfagnani
Copy link
Contributor

So it looks like Dartium is proxying cross-frame objects for web pages. It's only in Apps and Extensions that it's transferring directly. This means that simply removing the _isLocalObject(o) check will not result in consistent behavior between the two.

@DartBot
Copy link
Author

DartBot commented Mar 8, 2014

This comment was originally written by dev...@futureperfect.info


As a user, I believe that these types should be transferred directly in all contexts where the user has access to the object. If the user has access to the javascript object, then that means it "just works" if they write their app in pure javascript. Giving the user a JsObject in some situations and a typed Dart object in others does nothing security-wise, it just causes hassle and user frustration. I have hundreds of lines of Dart code to "re-apply" a type system on top of these JsObjects because I really want to write Chrome Apps in Dart. Others are less likely to be so determined, and will simply stick to javascript. I also think there is a big performance hit being taken w/ the proxy vs. direct transfer, because my Chrome App is abnormally slow in some file operations where it is lightening fast in Dartium. I don't have a benchmark or numbers, but I think there is definitely something there.

Thanks,

@DartBot
Copy link
Author

DartBot commented Mar 19, 2014

This comment was originally written by dev...@futureperfect.info


To follow up on the last sentence in my comment #­15, I wrote a small benchmark today to try and figure out what exactly is the bottleneck I am seeing when interacting with the chrome.fileSystem and I do not think that it is directly related to this issue. I believe it may be more related to the proxy objects for Entry objects (this issue is about File, Blob and friends which are already directly transferred in most cases. I'm not sure if there is a separate issue regarding direct transfer of Entry objects?

In my benchmark I observe that recursively listing the contents of a largish directory and calling chrome.fileSystem.getDisplayPath on each file is on the order of 3 times faster in Dartium than when compiled as js and run in Chrome (33).

@DartBot
Copy link
Author

DartBot commented Jul 22, 2014

This comment was originally written by dev...@futureperfect.info


I'm happy to see issue #20146, I am hoping that the rationale in that issue will also apply to the _isLocalObject check mentioned in this issue?

@DartBot DartBot added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-js labels Jul 22, 2014
@DanielJoyce
Copy link

Bumping into this exact same bug with a multi-window dart chrome app. If I was writing this in native code, sharing common objects between windows would be trivial. Here in Dart Land its nearly impossible. I am currently trying to abuse js-interop but having little luck.

@DanielJoyce
Copy link

I want to share a EventBus across multiple windows in the same chrome app, but having little luck.

@DanielJoyce
Copy link

This frustration is REALLY making me think of moving to TypeScript when 2.0 hits as they will add Async and Await support. Right now dart is really lacking on the chrome app usability front.

@DanielJoyce
Copy link

Also the browser already enforces security on this front, and added complication just makes writing dart apps more futile. This adds nothing to security. I've wasted 3 days looking for a workaround.

@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed triaged labels Mar 1, 2016
@matanlurey matanlurey added the closed-obsolete Closed as the reported issue is no longer relevant label Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-obsolete Closed as the reported issue is no longer relevant library-js P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

8 participants