Don't pass on zmq identities

Question

Don't pass on zmq identities

rgbkrk opened this issue 9 years ago · 14 comments

jmp.Message currently has zmq identities as part of its object (which is a Buffer as well). These should only get used as part of iopubSocket.subscribe(ident) and should not end up being something as part of the message since it's not part of the main JSON payload.

Answer 1 · 2016-01-13T19:46:59.000Z

Same goes for message.signatureOK.

Answer 2 · 2016-01-14T14:34:18.000Z

In other words, the question is whether Message.idents, Message.signatureOK and Message.blobs could be removed. Here are my thoughts:

Message.idents: I really think this property belongs to a Message object. In fact, that's the way it is in ZMQ. I don't like to use authoritative arguments, so I'll try to explain further why:
- A ZMQ identity "identifies" a connection between two sockets. In general, it can't be used to identify a socket, because unlike traditional sockets, which are limited to 1-to-1 connections, ZMQ sockets can handle 1-to-N connections.
- Message.idents is defined as an array of ZMQ identities listing all the connections the message has travelled through. This array helps route any replies back to the message creator.
- I think the argument about subscribe is actually a misunderstanding. It should be iopubSocket.subscribe(topic). See this example.
Message.signatureOK: What could be the alternative?
- A private property Message._signature to store the signature and a method Message#isSignatureOK?
- Or perhaps a visible Message.signature?
Message.blobs: It is defined in the standard. Message.blobs is handy to extend Jupyter messages, but Message.metadata can be used for that too. Do you know if there are any kernels using Message.blobs?

Answer 3 · 2016-01-14T17:38:52.000Z

Maybe this means I just copy the payload over or just delete these properties before sending on. I'm mostly aiming at compatibility of the raw messages with what the websocket payload ends up being. Blobs should stay, I think I just need to convert them to one standard format.

For each message that comes through currently, I'm trimming out what we don't end up having at the websockets layer (and for now, tearing out blobs):

.map(msg => {
  // Conform to same message format as notebook websockets
  // See https://github.com/n-riesco/jmp/issues/10
  delete msg.idents;
  delete msg.signatureOK;

  // Once we know what type blobs will be when using a websocket version of
  // enchannel, we'll make it consistent here. For now, delete the prop!
  delete msg.blobs;

  // Recursive Object.freeze
  deepFreeze(msg);
  return msg;
})

The IPython widgets use the binary blobs section. Anything else relying on comms can use this for efficient transport as well.

/cc @minrk

Answer 4 · 2016-01-14T17:42:02.000Z

Lo and behold, with node v4.x, a Buffer is now a subclass of Uint8Array. The ArrayBuffer underneath would be msg.blobs.buffer.

Answer 5 · 2016-01-15T09:21:06.000Z

@rgbkrk Would it help if by default the jmp.Message constructor doesn't set idents, signatureOK and blobs?

I would like to remove Message#signatureOK from Message, but to do that probably means changing the signature of the on "message" handler. Something like this:

socket.on("message", function(msg, isSignatureOK) { ... });

Answer 6 · 2016-01-15T21:54:22.000Z

Likely so. It's only on the receiving end do I want them not to have the extras. Don't tear blobs out though, that was wrong on my part.

Answer 7 · 2016-02-01T11:12:46.000Z

15c250f closes this issue.

Answer 8 · 2016-02-01T15:15:26.000Z

When would someone use a message that has an invalid signature? For testing?

Answer 9 · 2016-02-01T16:19:48.000Z

Oh... well! I knew I should've waited to publish the new version! I think I see what you mean. Invalid messages should be logged and dropped without calling the listener.

Answer 10 · 2016-02-01T16:27:59.000Z

Can always unpublish that version (though you'll have to use a different version number, even if just a patch).

Answer 11 · 2016-02-01T16:38:46.000Z

I've updated the README.md to advise that v0.3.0 is an alpha release. I will create a dev branch and commit the next changes there, so that you have a chance to review them before I publish a new release on npm.

Answer 12 · 2016-02-01T23:31:07.000Z

This commit should address this issue:

Message#signatureOK has been removed
"message" listeners only receive one argument, the JMP message; i.e.;
socket.on("message", function(msg) {...});
and messages with an invalid signature are dropped

I've unpublished jmp@0.3.0 on npm and I will publish the next release as version 0.4.0 .

Answer 13 · 2016-02-09T09:47:20.000Z

I've just released version 0.4.0. In this version:

Message#signatureOK has been removed
I've reverted the change in v0.3.0 and "message" listeners simply receive JMP message.
And, from now on, all invalid JMP messages are dropped (including those with a bad signature).

Answer 14 · 2016-02-09T13:46:36.000Z

Great! One other reason dropping messages with a bad signature is a wise idea - this helps mitigate any possible denial of service in processing invalid messages (from a bad actor).