Problem: receive_and_process_zap_reply is duplicated in all mechanisms

Solution: extract receive_and_process_zap_reply into zap_client_t and convert zap_client_t into base class of the server mechanism classes

src/curve_server.cpp

@@ -42,9 +42,7 @@ zmq::curve_server_t::curve_server_t (session_base_t *session_,
                                      const std::string &peer_address_,
                                      const options_t &options_) :
     mechanism_t (options_),
-    session (session_),
+    zap_client_t (session_, peer_address_, options_),
-    peer_address (peer_address_),
-    zap_client (session, peer_address, options),
     state (expect_hello),
     current_error_detail (no_detail),
     cn_nonce (1),
@@ -603,88 +601,16 @@ int zmq::curve_server_t::produce_error (msg_t *msg_) const
 
 int zmq::curve_server_t::send_zap_request (const uint8_t *key)
 {
-    return zap_client.send_zap_request ("CURVE", 5, key,
+    return zap_client_t::send_zap_request ("CURVE", 5, key,
-                                        crypto_box_PUBLICKEYBYTES);
+                                           crypto_box_PUBLICKEYBYTES);
 }
 
 int zmq::curve_server_t::receive_and_process_zap_reply ()
 {
-    int rc = 0;
+    int rc = zap_client_t::receive_and_process_zap_reply ();
-    msg_t msg [7];  //  ZAP reply consists of 7 frames
+    if (rc == -1 && errno == EPROTO)
-
-    //  Initialize all reply frames
-    for (int i = 0; i < 7; i++) {
-        rc = msg [i].init ();
-        errno_assert (rc == 0);
-    }
-
-    for (int i = 0; i < 7; i++) {
-        rc = session->read_zap_msg (&msg [i]);
-        if (rc == -1)
-            return close_and_return (msg, -1);
-        if ((msg [i].flags () & msg_t::more) == (i < 6? 0: msg_t::more)) {
-            // CURVE I : ZAP handler sent incomplete reply message
-            current_error_detail = zap;
-            errno = EPROTO;
-            return close_and_return (msg, -1);
-        }
-    }
-
-    //  Address delimiter frame
-    if (msg [0].size () > 0) {
-        // CURVE I: ZAP handler sent malformed reply message
         current_error_detail = zap;
-        errno = EPROTO;
+    return rc;
-        return close_and_return (msg, -1);
-    }
-
-    //  Version frame
-    if (msg [1].size () != 3 || memcmp (msg [1].data (), "1.0", 3)) {
-        // CURVE I: ZAP handler sent bad version number
-        current_error_detail = zap;
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Request id frame
-    if (msg [2].size () != 1 || memcmp (msg [2].data (), "1", 1)) {
-        // CURVE I: ZAP handler sent bad request ID
-        current_error_detail = zap;
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Status code frame, only 200, 300, 400 and 500 are valid status codes
-    char *status_code_data = static_cast <char*> (msg [3].data());
-    if (msg [3].size () != 3 || status_code_data [0] < '2'
-          || status_code_data [0] > '5' || status_code_data [1] != '0'
-          || status_code_data [2] != '0') {
-        // CURVE I: ZAP handler sent invalid status code
-        current_error_detail = zap;
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Save status code
-    status_code.assign (static_cast <char *> (msg [3].data ()), 3);
-
-    //  Save user id
-    set_user_id (msg [5].data (), msg [5].size ());
-
-    //  Process metadata frame
-    rc = parse_metadata (static_cast <const unsigned char*> (msg [6].data ()),
-                         msg [6].size (), true);
-
-    if (rc != 0)
-        return close_and_return (msg, -1);
-
-    //  Close all reply frames
-    for (int i = 0; i < 7; i++) {
-        const int rc2 = msg [i].close ();
-        errno_assert (rc2 == 0);
-    }
-
-    return 0;
 }
 
 void zmq::curve_server_t::handle_zap_status_code ()

src/curve_server.hpp

@@ -59,7 +59,7 @@ namespace zmq
     class msg_t;
     class session_base_t;
 
-    class curve_server_t : public mechanism_t
+    class curve_server_t : public zap_client_t
     {
     public:
 
@@ -90,18 +90,9 @@ namespace zmq
             connected
         };
 
-        session_base_t * const session;
-
-        const std::string peer_address;
-
-        zap_client_t zap_client;
-
         //  Current FSM state
         state_t state;
 
-        //  Status code as received from ZAP handler
-        std::string status_code;
-
         //  Details about the current error state
         error_detail_t current_error_detail;
 

src/gssapi_mechanism_base.hpp

@@ -50,7 +50,7 @@ namespace zmq
     /// process context-level GSSAPI tokens (via INITIATE commands)
     /// and per-message GSSAPI tokens (via MESSAGE commands).
     class gssapi_mechanism_base_t:
-        public mechanism_t
+        public virtual mechanism_t
     {
     public:
         gssapi_mechanism_base_t (const options_t &options_);

src/gssapi_server.cpp

@@ -45,10 +45,9 @@
 zmq::gssapi_server_t::gssapi_server_t (session_base_t *session_,
                                        const std::string &peer_address_,
                                        const options_t &options_) :
+    mechanism_t (options_),
     gssapi_mechanism_base_t (options_),
-    session (session_),
+    zap_client_t (session_, peer_address_, options_),
-    peer_address (peer_address_),
-    zap_client (session, peer_address, options),
     state (recv_next_token),
     security_context_established (false)
 {
@@ -156,79 +155,14 @@ int zmq::gssapi_server_t::send_zap_request ()
 {
     gss_buffer_desc principal;
     gss_display_name (&min_stat, target_name, &principal, NULL);
-    int rc = zap_client.send_zap_request ("GSSAPI", 6, principal.value,
+    int rc = zap_client_t::send_zap_request ("GSSAPI", 6, principal.value,
-                                          principal.length);
+                                             principal.length);
 
     gss_release_buffer (&min_stat, &principal);
 
     return rc;
 }
 
-int zmq::gssapi_server_t::receive_and_process_zap_reply ()
-{
-    int rc = 0;
-    msg_t msg [7];  //  ZAP reply consists of 7 frames
-
-    //  Initialize all reply frames
-    for (int i = 0; i < 7; i++) {
-        rc = msg [i].init ();
-        errno_assert (rc == 0);
-    }
-
-    for (int i = 0; i < 7; i++) {
-        rc = session->read_zap_msg (&msg [i]);
-        if (rc == -1)
-            return close_and_return (msg, -1);
-        if ((msg [i].flags () & msg_t::more) == (i < 6? 0: msg_t::more)) {
-            errno = EPROTO;
-            return close_and_return (msg, -1);
-        }
-    }
-
-    //  Address delimiter frame
-    if (msg [0].size () > 0) {
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Version frame
-    if (msg [1].size () != 3 || memcmp (msg [1].data (), "1.0", 3)) {
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Request id frame
-    if (msg [2].size () != 1 || memcmp (msg [2].data (), "1", 1)) {
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Status code frame
-    if (msg [3].size () != 3 || memcmp (msg [3].data (), "200", 3)) {
-        errno = EACCES;
-        return close_and_return (msg, -1);
-    }
-
-    //  Save user id
-    set_user_id (msg [5].data (), msg [5].size ());
-
-    //  Process metadata frame
-    rc = parse_metadata (static_cast <const unsigned char*> (msg [6].data ()),
-                         msg [6].size (), true);
-
-    if (rc != 0)
-        return close_and_return (msg, -1);
-
-    //  Close all reply frames
-    for (int i = 0; i < 7; i++) {
-        const int rc2 = msg [i].close ();
-        errno_assert (rc2 == 0);
-    }
-
-    return 0;
-}
-
-
 int zmq::gssapi_server_t::encode (msg_t *msg_)
 {
     zmq_assert (state == connected);

src/gssapi_server.hpp

@@ -41,8 +41,8 @@ namespace zmq
     class msg_t;
     class session_base_t;
 
-    class gssapi_server_t :
+    class gssapi_server_t 
-        public gssapi_mechanism_base_t
+      : public gssapi_mechanism_base_t, public zap_client_t
     {
     public:
 
@@ -89,7 +89,6 @@ namespace zmq
         int produce_next_token (msg_t *msg_);
         int process_next_token (msg_t *msg_);
         int send_zap_request ();
-        int receive_and_process_zap_reply();
     };
 
 }

src/null_mechanism.cpp

@@ -43,9 +43,7 @@ zmq::null_mechanism_t::null_mechanism_t (session_base_t *session_,
                                          const std::string &peer_address_,
                                          const options_t &options_) :
     mechanism_t (options_),
-    session (session_),
+    zap_client_t (session_, peer_address_, options_),
-    peer_address (peer_address_),
-    zap_client (session, peer_address, options),
     ready_command_sent (false),
     error_command_sent (false),
     ready_command_received (false),
@@ -86,15 +84,14 @@ int zmq::null_mechanism_t::next_handshake_command (msg_t *msg_)
         zap_reply_received = true;
     }
 
-    if (zap_reply_received
+    if (zap_reply_received && status_code != "200") {
-    &&  strncmp (status_code, "200", sizeof status_code) != 0) {
+        const size_t status_code_len = 3;
-        const int rc = msg_->init_size (6 + 1 + sizeof status_code);
+        const int rc = msg_->init_size (6 + 1 + status_code_len);
         zmq_assert (rc == 0);
-        unsigned char *msg_data =
+        unsigned char *msg_data = static_cast<unsigned char *> (msg_->data ());
-            static_cast <unsigned char *> (msg_->data ());
         memcpy (msg_data, "\5ERROR", 6);
-        msg_data [6] = sizeof status_code;
+        msg_data [6] = status_code_len;
-        memcpy (msg_data + 7, status_code, sizeof status_code);
+        memcpy (msg_data + 7, status_code.c_str (), status_code_len);
         error_command_sent = true;
         return 0;
     }
@@ -194,82 +191,6 @@ zmq::mechanism_t::status_t zmq::null_mechanism_t::status () const
 
 int zmq::null_mechanism_t::send_zap_request ()
 {
-    return zap_client.send_zap_request ("NULL", 4, NULL, NULL, 0);
+    return zap_client_t::send_zap_request ("NULL", 4, NULL, NULL, 0);
 }
 
-int zmq::null_mechanism_t::receive_and_process_zap_reply ()
-{
-    int rc = 0;
-    msg_t msg [7];  //  ZAP reply consists of 7 frames
-
-    //  Initialize all reply frames
-    for (int i = 0; i < 7; i++) {
-        rc = msg [i].init ();
-        errno_assert (rc == 0);
-    }
-
-    for (int i = 0; i < 7; i++) {
-        rc = session->read_zap_msg (&msg [i]);
-        if (rc == -1)
-            return close_and_return (msg, -1);
-        if ((msg [i].flags () & msg_t::more) == (i < 6? 0: msg_t::more)) {
-            //  Temporary support for security debugging
-            puts ("NULL I: ZAP handler sent incomplete reply message");
-            errno = EPROTO;
-            return close_and_return (msg, -1);
-        }
-    }
-
-    //  Address delimiter frame
-    if (msg [0].size () > 0) {
-        //  Temporary support for security debugging
-        puts ("NULL I: ZAP handler sent malformed reply message");
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Version frame
-    if (msg [1].size () != 3 || memcmp (msg [1].data (), "1.0", 3)) {
-        //  Temporary support for security debugging
-        puts ("NULL I: ZAP handler sent bad version number");
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Request id frame
-    if (msg [2].size () != 1 || memcmp (msg [2].data (), "1", 1)) {
-        //  Temporary support for security debugging
-        puts ("NULL I: ZAP handler sent bad request ID");
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Status code frame
-    if (msg [3].size () != 3) {
-        //  Temporary support for security debugging
-        puts ("NULL I: ZAP handler sent bad status code");
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Save status code
-    memcpy (status_code, msg [3].data (), sizeof status_code);
-
-    //  Save user id
-    set_user_id (msg [5].data (), msg [5].size ());
-
-    //  Process metadata frame
-    rc = parse_metadata (static_cast <const unsigned char*> (msg [6].data ()),
-                         msg [6].size (), true);
-
-    if (rc != 0)
-        return close_and_return (msg, -1);
-
-    //  Close all reply frames
-    for (int i = 0; i < 7; i++) {
-        const int rc2 = msg [i].close ();
-        errno_assert (rc2 == 0);
-    }
-
-    return 0;
-}

src/null_mechanism.hpp

@@ -40,7 +40,7 @@ namespace zmq
     class msg_t;
     class session_base_t;
 
-    class null_mechanism_t : public mechanism_t
+    class null_mechanism_t : public zap_client_t
     {
     public:
 
@@ -57,14 +57,6 @@ namespace zmq
 
     private:
 
-        session_base_t * const session;
-
-        char status_code [3];
-
-        const std::string peer_address;
-
-        zap_client_t zap_client;
-
         bool ready_command_sent;
         bool error_command_sent;
         bool ready_command_received;
@@ -79,7 +71,6 @@ namespace zmq
             const unsigned char *cmd_data, size_t data_size);
 
         int send_zap_request ();
-        int receive_and_process_zap_reply ();
     };
 
 }

src/plain_server.cpp

@@ -41,9 +41,7 @@ zmq::plain_server_t::plain_server_t (session_base_t *session_,
                                      const std::string &peer_address_,
                                      const options_t &options_) :
     mechanism_t (options_),
-    session (session_),
+    zap_client_t (session_, peer_address_, options_),
-    peer_address (peer_address_),
-    zap_client (session, peer_address, options),
     state (waiting_for_hello)
 {
 }
@@ -264,83 +262,6 @@ int zmq::plain_server_t::send_zap_request (const std::string &username,
       reinterpret_cast<const uint8_t *> (username.c_str ()),
       reinterpret_cast<const uint8_t *> (password.c_str ())};
     size_t credentials_sizes[] = {username.size (), password.size ()};
-    return zap_client.send_zap_request ("PLAIN", 5, credentials,
+    return zap_client_t::send_zap_request ("PLAIN", 5, credentials,
-                                        credentials_sizes, 2);
+                                           credentials_sizes, 2);
-}
-
-int zmq::plain_server_t::receive_and_process_zap_reply ()
-{
-    int rc = 0;
-    msg_t msg [7];  //  ZAP reply consists of 7 frames
-
-    //  Initialize all reply frames
-    for (int i = 0; i < 7; i++) {
-        rc = msg [i].init ();
-        errno_assert (rc == 0);
-    }
-
-    for (int i = 0; i < 7; i++) {
-        rc = session->read_zap_msg (&msg [i]);
-        if (rc == -1)
-            return close_and_return (msg, -1);
-        if ((msg [i].flags () & msg_t::more) == (i < 6? 0: msg_t::more)) {
-            //  Temporary support for security debugging
-            puts ("PLAIN I: ZAP handler sent incomplete reply message");
-            errno = EPROTO;
-            return close_and_return (msg, -1);
-        }
-    }
-
-    //  Address delimiter frame
-    if (msg [0].size () > 0) {
-        //  Temporary support for security debugging
-        puts ("PLAIN I: ZAP handler sent malformed reply message");
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Version frame
-    if (msg [1].size () != 3 || memcmp (msg [1].data (), "1.0", 3)) {
-        //  Temporary support for security debugging
-        puts ("PLAIN I: ZAP handler sent bad version number");
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Request id frame
-    if (msg [2].size () != 1 || memcmp (msg [2].data (), "1", 1)) {
-        //  Temporary support for security debugging
-        puts ("PLAIN I: ZAP handler sent bad request ID");
-        errno = EPROTO;
-        return close_and_return (msg, -1);
-    }
-
-    //  Status code frame
-    if (msg [3].size () != 3) {
-        //  Temporary support for security debugging
-        puts ("PLAIN I: ZAP handler rejected client authentication");
-        errno = EACCES;
-        return close_and_return (msg, -1);
-    }
-
-    //  Save status code
-    status_code.assign (static_cast <char *> (msg [3].data ()), 3);
-
-    //  Save user id
-    set_user_id (msg [5].data (), msg [5].size ());
-
-    //  Process metadata frame
-    rc = parse_metadata (static_cast <const unsigned char*> (msg [6].data ()),
-                         msg [6].size (), true);
-
-    if (rc != 0)
-        return close_and_return (msg, -1);
-
-    //  Close all reply frames
-    for (int i = 0; i < 7; i++) {
-        const int rc2 = msg [i].close ();
-        errno_assert (rc2 == 0);
-    }
-
-    return 0;
 }

src/plain_server.hpp

@@ -40,7 +40,7 @@ namespace zmq
     class msg_t;
     class session_base_t;
 
-    class plain_server_t : public mechanism_t
+    class plain_server_t : public zap_client_t
     {
     public:
 
@@ -68,15 +68,6 @@ namespace zmq
             ready
         };
 
-        session_base_t * const session;
-
-        const std::string peer_address;
-
-        zap_client_t zap_client;
-
-        //  Status code as received from ZAP handler
-        std::string status_code;
-
         state_t state;
 
         int produce_welcome (msg_t *msg_) const;
@@ -88,7 +79,6 @@ namespace zmq
 
         int send_zap_request(const std::string &username,
                              const std::string &password);
-        int receive_and_process_zap_reply ();
     };
 
 }

src/zap_client.cpp

@@ -31,15 +31,16 @@
 
 #include "zap_client.hpp"
 #include "msg.hpp"
+#include "session_base.hpp"
 
 namespace zmq
 {
 zap_client_t::zap_client_t (session_base_t *const session_,
                             const std::string &peer_address_,
                             const options_t &options_) :
+    mechanism_t (options_),
     session (session_),
-    peer_address (peer_address_),
+    peer_address (peer_address_)
-    options (options_)
 {
 }
 
@@ -144,4 +145,79 @@ int zap_client_t::send_zap_request (const char *mechanism,
 
     return 0;
 }
+
+int zap_client_t::receive_and_process_zap_reply ()
+{
+    int rc = 0;
+    msg_t msg[7]; //  ZAP reply consists of 7 frames
+
+    //  Initialize all reply frames
+    for (int i = 0; i < 7; i++) {
+        rc = msg[i].init ();
+        errno_assert (rc == 0);
+    }
+
+    for (int i = 0; i < 7; i++) {
+        rc = session->read_zap_msg (&msg[i]);
+        if (rc == -1)
+            return close_and_return (msg, -1);
+        if ((msg[i].flags () & msg_t::more) == (i < 6 ? 0 : msg_t::more)) {
+            // CURVE I : ZAP handler sent incomplete reply message
+            errno = EPROTO;
+            return close_and_return (msg, -1);
+        }
+    }
+
+    //  Address delimiter frame
+    if (msg[0].size () > 0) {
+        // CURVE I: ZAP handler sent malformed reply message
+        errno = EPROTO;
+        return close_and_return (msg, -1);
+    }
+
+    //  Version frame
+    if (msg[1].size () != 3 || memcmp (msg[1].data (), "1.0", 3)) {
+        // CURVE I: ZAP handler sent bad version number
+        errno = EPROTO;
+        return close_and_return (msg, -1);
+    }
+
+    //  Request id frame
+    if (msg[2].size () != 1 || memcmp (msg[2].data (), "1", 1)) {
+        // CURVE I: ZAP handler sent bad request ID
+        errno = EPROTO;
+        return close_and_return (msg, -1);
+    }
+
+    //  Status code frame, only 200, 300, 400 and 500 are valid status codes
+    char *status_code_data = static_cast<char *> (msg[3].data ());
+    if (msg[3].size () != 3 || status_code_data[0] < '2'
+        || status_code_data[0] > '5' || status_code_data[1] != '0'
+        || status_code_data[2] != '0') {
+        // CURVE I: ZAP handler sent invalid status code
+        errno = EPROTO;
+        return close_and_return (msg, -1);
+    }
+
+    //  Save status code
+    status_code.assign (static_cast<char *> (msg[3].data ()), 3);
+
+    //  Save user id
+    set_user_id (msg[5].data (), msg[5].size ());
+
+    //  Process metadata frame
+    rc = parse_metadata (static_cast<const unsigned char *> (msg[6].data ()),
+                         msg[6].size (), true);
+
+    if (rc != 0)
+        return close_and_return (msg, -1);
+
+    //  Close all reply frames
+    for (int i = 0; i < 7; i++) {
+        const int rc2 = msg[i].close ();
+        errno_assert (rc2 == 0);
+    }
+
+    return 0;
+}
 }

src/zap_client.hpp

@@ -30,11 +30,13 @@
 #ifndef __ZMQ_ZAP_CLIENT_HPP_INCLUDED__
 #define __ZMQ_ZAP_CLIENT_HPP_INCLUDED__
 
-#include "session_base.hpp"
+#include "mechanism.hpp"
 
 namespace zmq
 {
-class zap_client_t
+class session_base_t;
+
+class zap_client_t : public virtual mechanism_t
 {
   public:
     zap_client_t (session_base_t *const session_,
@@ -52,10 +54,15 @@ class zap_client_t
                           size_t *credentials_sizes,
                           size_t credentials_count);
 
-  private:
+
+    int receive_and_process_zap_reply ();
+
+  protected:
     session_base_t *const session;
-    const std::string &peer_address;
+    const std::string peer_address;
-    const options_t &options;
+
+    //  Status code as received from ZAP handler
+    std::string status_code;
 };
 }