search

zeromq / rfc

ZeroMQ RFC project
https://rfc.zeromq.org
109 stars 64 forks source link

36/ZRE seems to have errors #156

Closed Wambou closed 4 years ago

Wambou commented 4 years ago

Hi,

Reading the ZRE RFC I noticed some discrepancy between the grammar and explanations for the WHISPER messages. This might be because of 20/ZRE version 1.

Explanation says:

The WHISPER command contains a single field, which is the message content defined as one 0MQ frame. ZRE does not support multi-frame message contents.

when the grammar says:

;     Send a multi-part message to a peer
whisper         = signature %d2 version sequence content
version         = number-1              ; Version number (3)
sequence        = number-2              ; Cyclic sequence number
content         = msg                   ; Wrapped message content

; A msg is zero or more distinct frames
msg             = *frame

So multi-frames messages are possible.

This error is also present in the version 3 43/ZRE. If you agree, I can make the modification on it.

But should there be an errata for 36/ZRE ? Or just update it ?

bluca commented 4 years ago

Check what the implementation does first, and then feel free to send a PR for the new version

Wambou commented 4 years ago

OK, I will do my best to find the implementation now. I just started reading ZeroMQ RFCs so I am still a bit lost on where to look at things.

Wambou commented 4 years ago

I looked into the code and it seems that I have found proof of multiframe messages.

Then, I looked at the history and found that commit Handles multi-part messages on SHOUT and WHISPER hintjens committed on 28 Nov 2013

Now I can update 43/ZYREv3 but should I also update 36/ZRE ?

bluca commented 4 years ago

Yes, given it's a very old change