zeromq/zmqpp

Get socket_option::identity returns wrong value

johanLsp opened this issue · 3 comments

I stumbled across this : the string returned by calling get for the identity socket option is missing the last character.
I started to write additional tests to cover this use case.
The issue comes from socket.cpp:862, I'll try and make a pull request later to fix it. I've got the feeling that it boils down to the identity option being the oldest string type option in zmq (< v 3.2), and is not consistent with newer options such as plain_password or plain_username.

I've stumbled upon this today!

Couldn't figure out why my STREAM socket is failing while trying to send the first message as a client.

I like your pull request with fixes. Instead of adding additional if branch inside case, I would use already present case socket_option::identity to add specific behaviour there.

diff --git a/src/zmqpp/socket.cpp b/src/zmqpp/socket.cpp
index fc8d9a4..c705575 100644
--- a/src/zmqpp/socket.cpp
+++ b/src/zmqpp/socket.cpp
@@ -838,6 +838,13 @@ void socket::get(socket_option const option, std::string& value) const
        switch(option)
        {
        case socket_option::identity:
+               if(0 != zmq_getsockopt(_socket, static_cast<int>(option), buffer.data(), &size))
+               {
+                       throw zmq_internal_exception();
+               }
+
+               value.assign(buffer.data(), size);
+               break;
 #if (ZMQ_VERSION_MAJOR > 3) || ((ZMQ_VERSION_MAJOR == 3) && (ZMQ_VERSION_MINOR >= 2))
        case socket_option::last_endpoint:
 #endif

I made it explicit as you suggested. Unfortunately, this repo doesn't seem to be so active anymore.

Sorry it took a while, I've now merged this pull request, I'll slowly work through outstanding requests as I can find some time. Of course I have to make sure they work together now as well, always penalties for being lazy :)