Add ability to send binary data using native messaging |
||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.86 Safari/537.36 Steps to reproduce the problem: Not applicable What is the expected behavior? What went wrong? Native messaging currently only allows communication through standard I/O using JSON. An alternative method that allows to send binary data both ways using ArrayBuffers would be of great benefit to many developers. It would allow for better performance and faster development of a cooperating native client. Did this work before? N/A Does this work in other browsers? N/A Chrome version: 59.0.3071.86 Channel: stable OS Version: 10.0 Flash Version: Shockwave Flash 25.0 r0
,
Jun 12 2017
,
Jun 12 2017
,
Jun 12 2017
With native messaging I assume this is talking about the extensions API, not web platform postMessage?
,
Jun 12 2017
Yes, I meant the extension API native messaging API.
,
Jun 19 2017
I've attached a patch, which allows sending binary data using an ArrayBuffer to a native messaging host from JavaScript. Unit tests are currently missing. I've also created a manual test suite for testing this feature: https://gitlab.com/tambre/BinaryNativeMessagingTest (Windows-only at this time, sorry) The patch doesn't yet add support for receiving binary data, as it's assumed that all data received from the native host is JSON. I've managed to come up with two possible solutions: 1. Treat data received from a host as binary data if the first data byte matches a certain magic number. This adds overhead and feels slightly hacky. 2. Add an option to the native host manifest to treat all data received from the host as binary data. This adds practically no overhead and ensures compatiblity with existing native hosts. While the second solution seems like the way-to-go to me, I might've missed something and would appreciate feedback on an appropriate solution to this problem. Considering that the milestone is currently set for Chrome 62, is there anything extra that I'd need to do to have this feature merged in time for Chrome 60, or will having the patch ready in time be enough?
,
Jun 19 2017
It would be great if this was also enabled for the extension message api for messages between different internal parts and other extensions, runtime.sendMessage() and tabs.sendMessage().
,
Jun 24 2017
With my changes all the postMessage() functions should technically support sending ArrayBuffers. But I don't have anything to test it with, nor would receiving the messages currently work. I'm attaching the latest version of my changes, which includes the 2nd solution I described in my previous comment. Unfortunately it crashes magically in DeliverMessageToScriptContext while creating v8::ArrayBuffer for no apparent reason. I also asked for help on #chromium and created the small document for that (https://gist.github.com/tambry/ff3269769ab92644528b4573f6b29879), but got no help. Looking at assembly, it loads a null-pointer at the beginning of Factory::NewJSArrayBuffer and then tries to access it. It is unclear to me, as to what variable it's accessing, as the VS debugger fails to display contents of the Factory and Isolate classes. It seems that the assembly was for somekind of inline debug check, so I tried to disable inlining. Thankfully V8 has an option for that, which doesn't work on MSVC. After manually hacking around it, it still seems to use a default optimization flag /O2, which implies inlining, even when I explicitly disable inlining using /Ob0. I tried to disable compiler optimization for V8, but I was unable to accomplish that, without turning it off for everything, which would've likely resulted in a full rebuild, which would've taken roughly 8 hours on my system. Due to extremely long build times, various build system problems and many other annoyances I encountered while trying to implement this feature, I'm giving up. I'll be looking forward to the release of this feature in Chrome and Firefox. Equivalent Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1376074
,
Nov 9
assigning to sergeyu@ for native messaging
,
Nov 10
Devlin, I don't think I'm the right person to own this bug. I'm unlikely to have time to work on it. This feature will require non-trivial amount of work to design and implement it properly. I also not convinced it's worthwhile to spend any time on this. It's already possible to pass binary data over the provided messaging channel (e.g. by converting it to/from base64), so the only potential benefit here is some small perf improvements when passing significant amount of data to/from native applications. Native Messaging was never designed for such applications. If there is a specific use-case where this feature can provide significant benefits to the user then it may be better to understand that use-case and figure out if native messaging is the right solution (e.g. maybe some kind of shared memory API would be a better fit). Raul, What's the reason you need to send binary data over native messaging? Do you have any measurements to show perf improvements compared to simple JSON serialization?
,
Nov 12
My use case was to allow one to more easily create a small native application, due to not having to include a JSON library (doing that is cumbersome in some languages). I haven't measured the performance side, but I'd imagine it'd indeed be slightly faster. I already "designed" and implemented this feature a long time back (see binary_native_messaging.patch), but I was unable to get it to work due to my limited understanding of V8's innerworkings (the problem was a crash, likely due to a debug check, when passing an ArrayBuffer to JS). Presumably it shouldn't be too hard to update the patch. If no one picks it up earlier, I might try and have a go at updating my patch sometime during Christmas.
,
Nov 18
Had some spare time this weekend and implemented this. Initial changeset with support for only native messaging and no tests: https://chromium-review.googlesource.com/c/chromium/src/+/1340811
,
Dec 19
For posterity I'm copying comments from that CL here: sergeyu@chromium.org: As I explained in the bug it's not clear if this feature provides enough benefits to make it worthwhile. It's already possible to pass binary data using the existing protocol. There are plenty of libraries available for JSON and BASE64 for most languages. What's the reason you need this feature? raul.tambre@gmail.com: Using JSON with Base64 feels like a hack, which I don't think is necessary. Binary message support would certainly make developing native messaging applications in many cases a bit easier. This CL also lays the foundation for expanding the use of binary messages to other APIs (e.g. runtime.sendMessage() and tabs.sendMessage()), which was requested by another user in the bug. I've also updated the patch set to fix the compile failures seen in the try jobs. jamiewalch@chromium.org: Something else to bear in mind is that the only activity on the Firefox bug since it was opened 2 years ago was to lower its priority, so it seems unlikely that Mozilla will ever implement this, decreasing compatibility between the two browsers. Granted, that's less of a concern for an extension API than for a web API, but it's still undesirable, IMO. I agree with Sergey that a better (simpler, more maintainable) solution is for developers to encode binary data as text and use the existing protocol. sergeyu@chromium.org: There are several downsides: 1. It increases browser complexity, making it harder to maintain and increasing probability of security vulnerabilities. 2. It increases interface complexity. The developers would have to decide if they want to use binary or JSON format. We would also want to support migration from the JSON version of the interface to binary and vice versa (AFAICT with the current version of this CL it's not possible for an extension to detect if a particular native app expects binary or JSON messages). Trying to solve that would only increase complexity further. 3. Binary message processing is known to be error prone in low-level languages. By giving this hummer to the developers we increase the chance that some of the applications will be buggy, which may also compromise security. raul.tambre@gmail.com: 1. That's par for the course for almost any feature. That said, I think the complexity increase as a result of this feature is incredibly small, being mostly restricted to some 20 lines in messaging_util.cc for converting to and from ArrayBuffers and binary (rest is plumbing for the output type, documentation, examples, tests). On the other hand, this will lower the complexity of native hosts intending to use a binary protocol. 2. Considering that it's a single binary option, I don't think it's going to be a much of an overhead for one to decide which to use. Many are probably still going to use JSON, but those that do want to use binary without workarounds, will now have the option. Furthermore, I don't think providing an explicit migration mechanism is necessary. One could easily have different manifests for both output types (e.g. com.my_company.my_application and com.my_company.my_application_binary) and first try to open the native host with the preferable output type, then if it fails try the other one. Besides, there probably aren't going to be too many extensions migrating between these types. 3. Developers could always use something like Protocol Buffers. JSON also isn't without problems, since libraries for it in low-level languages may too have vulnerabilities. I think because everyone doesn't want to and isn't capable of using a tool, isn't a good reason to not provide it.
,
Dec 19
Raul, if you'd like this work to move forward I suggest writing a design document (e.g. in google docs) which describes how this feature would work. Particularly I'd like to have clear understanding of how the two messaging protocols would coexist and how the browser should decide which one to use.
,
Dec 30
I'll look into writing a design document.
,
Dec 30
,
Dec 30
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by raul.tam...@gmail.com
, Jun 12 2017