Skip to content
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

message type tag encoding #2

Open
mhaberler opened this issue Aug 12, 2014 · 9 comments
Open

message type tag encoding #2

mhaberler opened this issue Aug 12, 2014 · 9 comments

Comments

@mhaberler
Copy link
Member

with the original ZWS spec ws frames are type-tagged with a single character code.

Since the protocol does now assume a binary-safe link, we could use a normal binary int field to represent the ws frame tag. This would for instance allow encoding of bit fields, for instance the ZMQ_MORE flag.

Options I see is:

  • fixed size int (8,16,32..)
  • varint-encoding protobuf style

My personal preference would be the latter. It is well specified, implementation is well understood (all of google protobuf breaks if this breaks), and it is extensible with respect to the size of the value, while being a single byte in the common case.

On varint encoding:
https://developers.google.com/protocol-buffers/docs/encoding
http://stackoverflow.com/questions/19758270/read-varint-from-linux-sockets

the nice part: https://github.com/dcodeIO/ByteBuffer.js/wiki/API already takes care of varint encoding, and base64 if required, so not much to do. Meaning: if we use ByteBuffer.js to encode the ws frames, support is outsourced.

@mhaberler
Copy link
Member Author

Actually this poses the question why we dont define the ZWS protocol as a Protobuf .proto definition to start with. It might sound like a long shot to start with... I'll look into doing an example.

@somdoron
Copy link
Member

I don't think it should be protobuf based protocol, I think it should be
similar to ZMTP as much as possible and not to have dependency on other
libraries. Also currently we only have one flag to pass, the more flag. I
don't think we need varint at the moment.

On Tue, Aug 12, 2014 at 2:29 PM, Michael Haberler notifications@github.com
wrote:

Actually this poses the question why we dont define the ZWS protocol as a
Protobuf .proto definition to start with. It might sound like a long shot
to start with... I'll look into doing an example.


Reply to this email directly or view it on GitHub
#2 (comment).

@mhaberler
Copy link
Member Author

well 'describe as .proto definition' does not necessarily imply a dependency on a library; you can use a library but a protobuf a can be perfectly constructed and parsed without that if the dependency is a concern (which I understand for a framing level protocol)

see for instance the decoder Josh Haberman wrote (all of 113 lines of C, so certainly complexity is not an issue): https://github.com/mhaberler/machinekit/blob/machinetalk-preview-3/src%2Fmachinetalk%2Fsupport%2Fpb.c

I dont see ZMTP as a desirable blueprint - see for example the versioning problems it has. As far as I am concerned it is in fact a pretty bad example of protocol header design, and one certainly should not repeat it, in particular not with leaving evolution, extensions and versioning out of scope - even 'for the moment' turned out good enough for that particular moment. Fact is - protocols evolve, and then you're stuck with an inflexible approach.

I know it's a long shot - that's why I said so ;) but eventually it'd be natural. Let me explore it first before we decide the issue.

@mhaberler
Copy link
Member Author

here is a prototype of protobuf-based framing, including negotiated base64 wrapping:

http://git.mah.priv.at/gitweb?p=emc2-dev.git;a=commit;h=7aef0caa38a021da354c0deb5d9ca2312c0b681a

zws.proto is the protobuf Frame spec which wraps multipart messages or control frames.
this is the relay function in the proxy: http://git.mah.priv.at/gitweb?p=emc2-dev.git;a=blob;f=src/machinetalk/webtalk/webtalk_pbzws_policy.cc;h=41a681694d45188440780293b9a8aa4086c47f5b;hb=7aef0caa38a021da354c0deb5d9ca2312c0b681a

client example, still using Python protobuf: http://git.mah.priv.at/gitweb?p=emc2-dev.git;a=blob;f=src/machinetalk/webtalk/tests/zwsproto1.py;h=d4331ff2c669ccd8d610729c9c9a39af2112b4b1;hb=7aef0caa38a021da354c0deb5d9ca2312c0b681a

worked on first try - zero surprises.

what do you think?

@krisrok
Copy link
Contributor

krisrok commented Jan 16, 2015

i don't exactly know if it's related to this issue, but in the current implementation of jsmq the returned frame includes the more-flag, at least in combination with netmq.websockets.

in the example code you can see it pretty clearly by using firefox: it's String.fromCharCode()-impl does not know how to handle the leading 0 or 1, so it prints an unknown character. chrome's impl seems to be a bit more precautious and cuts them off.

if this is the wrong issue or relates to a problem in netmq, i'm sorry ;)

@somdoron
Copy link
Member

@krisrok the leading more flag shouldn't seen by the user. Do you see it in JSMQ or in NetMQ (it does pass on the wire but should be handled by the framework).

@krisrok
Copy link
Contributor

krisrok commented Jan 17, 2015

in jsmq

i guess a fix would be to return a trimmed version of the frame here:
https://github.com/zeromq/JSMQ/blob/master/JSMQ.js#L419

or to add the frame without the flag to the buffer in the first place here:
https://github.com/zeromq/JSMQ/blob/master/JSMQ.js#L88

@somdoron
Copy link
Member

I think the second solution is more correct. Do you want to provide a pull request or I will ?

@krisrok
Copy link
Contributor

krisrok commented Jan 18, 2015

just did so. expect a pr on netmq.websockets too :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants