gimite / web-socket-js

HTML5 Web Socket implementation powered by Flash
BSD 3-Clause "New" or "Revised" License
2.73k stars 489 forks source link

Hybi 08 #89

Closed kanaka closed 12 years ago

kanaka commented 13 years ago

I've create a new hybi-08 branch that merges master and also adds binary API support.

Note that in the official API definition the binaryType modes are "arraybuffer" and "blob". However, in general, browsers that have ArrayBuffers and Blobs also have a native WebSocket implementation (although none have actually binary API support in WebSocket as of yet).

I have not implemented Blob support at all (well, technically I did but then realized it was both messy and unnecessary). I have implemented ArrayBuffer binaryType support, however it's usefulness will probably be limited to cases where web-socket-js is forced or to browsers which have ArrayBuffer but no WebSocket (IE might be the only browser in that situation). I also have a custom binaryType mode in this branch which is 'array'. This allows an array of numbers 0-255 to be sent as a raw binary frame. The 'array' mode is not in the spec, but if older browsers want to be able to use WebSocket with binary data then they need to be able to pass some sort of object that is translated to binary data.

In other words:

ws.send([1,2,3]);

Will send a binary HyBi data frame to the server containing '\x01\x02\x03'.

When browsers begin supporting the binary API then the equivalent to the above will be:

ws.send(Uint8Array([1,2,3].buffer);

This:

 ws.send("123")

will send a text HyBi frame to the server containing '123'.

Anyways, if any of that is confusing, just ask and I'll try and clarify. We'll probably want to document the special 'array' mode for older browsers.

dvv commented 13 years ago

Doesn't work with latest https://github.com/Worlize/WebSocket-Node example.

In fact, the connection isn't established, and I get "INVALID_STATE_ERR: Web Socket connection has not been established"

Wonder if you could confirm/fix that. TIA, --Vladimir

kanaka commented 13 years ago

@gimite, I've fixed the location of SHA1 so that it builds again after the merge with master.

@dvv, the failure is probably because I didn't update the swf file. It's a binary file so generally I avoid rebuilding except on master. However, I've updated the build (and fixed an issue) so that you can test it. So in your test you were likely using an old build with the old handshake.

dvv commented 13 years ago

Thanks! That's what I get now:

[WebSocket] debug enabled
[WebSocket] policy file: xmlsocket://localhost:843
[WebSocket] connected
[WebSocket] request header: GET / HTTP/1.1 Host: localhost:8080 Upgrade: websocket Connection: Upgrade Sec-WebSocket-Key: MTA0MjIxMTAzOTk3MTAyOTYxMDY5OTMyNTQxMDc2MDExMzc5NQ== Sec-WebSocket-Origin: http://localhost:8080 Sec-WebSocket-Version: 8 Cookie: Sec-WebSocket-Protocol: whiteboard-example
[WebSocket] response header: HTTP/1.1 101 Switching Protocols Upgrade: websocket Connection: Upgrade Sec-WebSocket-Accept: wXvc7w39Nnr09itX14aFpZy7BFg= Sec-WebSocket-Protocol: whiteboard-example Sec-WebSocket-Origin: http://localhost:8080

[WebSocket] received unknown opcode: 9
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[WebSocket] close

What could that mean? TIA

dvv commented 13 years ago

Oh, it's PING. Is it implemented?

kanaka commented 13 years ago

Nope, ping/pong isn't implemented yet. I'll need to review the spec again to understand exactly what the semantics should be (since it changed a couple times in the past few months), but it shouldn't be too big of an issue. Might be able to get to it in the next few days. I'm also working on the hybi-13 branch which adds support for the latest rev of the spec (really it's mostly a version number change) and also adds code/reason parameters to close events and refactors the semantics of closing a connection to match the spec.

dvv commented 13 years ago

Will be waiting. Please, provide compiled *.swf when you're done. Thanks.

gimite commented 12 years ago

I thought I wrote something like below here but it seems I failed to do that... Writing again.

Thanks for the patch.

Goal of web-socket-js is to implement WebSocket spec and not to do something more. So I think supporting binaryType == "array" is out of scope of this library. It is only useful when you specify WEB_SOCKET_FORCE_FLASH, which seems weird. But I also agree that supporting binary protocol with "arraybuffer" and "blob" is not very useful. So there are two options I can think of:

@kanaka, what do you think?

gimite commented 12 years ago

Pulled to hybi-10 branch (which will soon be the master branch). Haven't pulled binary protocol support for now.

kanaka commented 12 years ago

@gimite, regarding the binary data support. I'm fine with binary support being an extension to the default support. I do think there are some developers out there that will want binary support on older browsers (and array and arraybuffer is actually very similar from a develop standpoint). I propose that a sibling file be created called web_socket_binary.js which extends the WebSocket object from web_socket.js to add binaryString and binary array support. There might be some minor changes to the WebSocket.as to support both modes but I would like to keep this as a single project with a single swf (well two including the insecure mode).

In other words, if developers just want text support (but the new protocol), they include web_socket.js (and swfobject.js). If they want binary support then they also include web_socket_binary.js which adds the binary support too.

Is that is agreeable to you?

gimite commented 12 years ago

Having web_socket_binary.js is fine. But I want to make it another project e.g. web-socket-binary-js, and I won't maintain it myself, because I think supporting binaryType == "array" is out of scope of web-socket-js.

I haven't pulled any of binary support changes for now. But I'm OK to just pull ActionScript part. This would make it easier to implement web-socket-binary-js because you can reuse SWF file from web-socket-js.

kanaka commented 12 years ago

Support for sending arraybuffers is relevant to users of Firefox 4 and 5 because those versions have arraybuffer support but don't have WebSocket support enabled by default. So for firefox 4/5 arraybuffer is in scope for web-socket-js (at least in my opinion).

Arraybuffer binary support is also relevant if the developer wants to force web-socket-js on Chrome 5-13, Firefox 6,7,8, Safari 5.1 and 6. Those browser versions also have arraybuffers and websocket support but the builtin websocket support in those versions is missing binary support (or is not even using HyBi yet).

So adding support for sending and receiving ArrayBuffer data certainly seems within scope. Adding the additional support for binaryType ="array" is 3-5 extra lines in web_socket.js. Is that really worth creating a new project for that? A few extra lines that enable the ability to send binary data on every platform where web-socket-js can run seems line a pretty big win to me.

gimite commented 12 years ago

Goal of web-socket-js is to provide WebSocket API when it's missing in the browser. So WEB_SOCKET_FORCE_FLASH, which overwrites native WebSocket, is a really hacky option. So I don't want to add something which is useful only when you force Flash. Based on that, I think:

I agree that things get trickier recently because now some browsers have WebSocket class but miss some features (such as binary). So there would be some use case where something like FORCE_FLASH is useful. But that's not a (main) goal of this library. Something focusing on that direction should be a separated project.

kanaka commented 12 years ago

@gimite, I think I wasn't clear the first time. Firefox 4 and Firefox 5 support arraybuffer data types, but do not have WebSocket (at least not enabled by default). So for firefox 4 and firefox 5, allowing arraybuffers to be sent in web-socket-js makes perfect sense. In other words, whether or not we have support for binaryType="array" we should have support for binaryType="arraybuffer" in order to support firefox 4 and firefox 5 more fully.

So I think we should add binaryType="arrayBuffer", adding support for binaryType="array" is a separate issue that shouldn't affect that decision.

Also binaryType="array" is useful for more than just with WEB_SOCKET_FORCE_FLASH. It's useful on IE 6,7,8,9 and firefox 3.0,3.5,3.6 and Opera 10 and 11 (and probably 12 too) because none of those browsers have websocket support enabled by default.

Also, I'm willing to support both arraybuffer (and normal array) functionality in web-socket-js if that is your concern.

gimite commented 12 years ago

So I think we should add binaryType="arrayBuffer", adding support for binaryType="array" is a separate issue that shouldn't affect that decision.

I agree that it's separated issue. My first bullet point in previous post is about ArrayBuffer support. My second point is about binaryType="array" support.

For ArrayBuffer support, actually it depends on whether we use MozWebSocket (instead of Flash implementation) when available. I think we should, though we haven't yet. When we do that, even if we add ArrayBuffer support, it is available only on some old browsers (Firefox 4, 5), and is not available in latest version of all major browsers (Firefox 6, Chrome 14, etc.) assuming FORCE_FLASH is false. This doesn't seem very useful.