Problem: zap_msg_available duplicated between curve_server_t and plain_server_t (with deviating behaviour)

Solution: pull up into zap_client_common_handshake_t, along with handle_zap_status_code and error_detail/current_error_detail

src/curve_server.cpp

@@ -42,8 +42,8 @@ zmq::curve_server_t::curve_server_t (session_base_t *session_,
                                      const std::string &peer_address_,
                                      const options_t &options_) :
     mechanism_t (options_),
-    zap_client_common_handshake_t (session_, peer_address_, options_),
+    zap_client_common_handshake_t (
-    current_error_detail (no_detail),
+      session_, peer_address_, options_, sending_ready),
     cn_nonce (1),
     cn_peer_nonce (1)
 {
@@ -244,26 +244,6 @@ int zmq::curve_server_t::decode (msg_t *msg_)
     return rc;
 }
 
-int zmq::curve_server_t::zap_msg_available ()
-{
-    //  TODO I don't think that it is possible that this is called in any 
-    //  state other than expect_zap_reply. It should be changed to
-    //  zmq_assert (state == expect_zap_reply);
-    if (state != waiting_for_zap_reply) {
-        errno = EFSM;
-        return -1;
-    }
-    const int rc = receive_and_process_zap_reply ();
-    if (rc == 0)
-        handle_zap_status_code ();
-    return rc;
-}
-
-zmq::mechanism_t::error_detail_t zmq::curve_server_t::error_detail() const
-{
-    return current_error_detail;
-}
-
 int zmq::curve_server_t::process_hello (msg_t *msg_)
 {
     if (msg_->size () != 200) {
@@ -601,33 +581,4 @@ int zmq::curve_server_t::receive_and_process_zap_reply ()
     return rc;
 }
 
-void zmq::curve_server_t::handle_zap_status_code ()
-{
-    //  we can assume here that status_code is a valid ZAP status code, 
-    //  i.e. 200, 300, 400 or 500
-    if (status_code [0] == '2') {
-        state = sending_ready;
-    } else {
-        state = sending_error;
-
-        int err = 0;
-        switch (status_code [0]) {
-            case '3':
-                err = EAGAIN;
-                break;
-            case '4':
-                err = EACCES;
-                break;
-            case '5':
-                err = EFAULT;
-                break;
-        }
-        //  TODO use event_handshake_failed_zap here? but this is not a ZAP 
-        //  protocol error
-        
-        session->get_socket ()->event_handshake_failed_no_detail (
-          session->get_endpoint (), err);
-    }
-}
-
 #endif

src/curve_server.hpp

@@ -73,14 +73,9 @@ namespace zmq
         virtual int process_handshake_command (msg_t *msg_);
         virtual int encode (msg_t *msg_);
         virtual int decode (msg_t *msg_);
-        virtual int zap_msg_available ();
-        virtual error_detail_t error_detail () const;
 
     private:
 
-        //  Details about the current error state
-        error_detail_t current_error_detail;
-
         uint64_t cn_nonce;
         uint64_t cn_peer_nonce;
 
@@ -110,7 +105,6 @@ namespace zmq
 
         int send_zap_request (const uint8_t *key);
         int receive_and_process_zap_reply ();
-        void handle_zap_status_code ();
     };
 
 }

src/plain_server.cpp

@@ -41,7 +41,8 @@ zmq::plain_server_t::plain_server_t (session_base_t *session_,
                                      const std::string &peer_address_,
                                      const options_t &options_) :
     mechanism_t (options_),
-    zap_client_common_handshake_t (session_, peer_address_, options_)
+    zap_client_common_handshake_t (
+      session_, peer_address_, options_, sending_welcome)
 {
 }
 
@@ -103,20 +104,6 @@ int zmq::plain_server_t::process_handshake_command (msg_t *msg_)
     return rc;
 }
 
-int zmq::plain_server_t::zap_msg_available ()
-{
-    if (state != waiting_for_zap_reply) {
-        errno = EFSM;
-        return -1;
-    }
-    const int rc = receive_and_process_zap_reply ();
-    if (rc == 0)
-        state = status_code == "200"
-            ? sending_welcome
-            : sending_error;
-    return rc;
-}
-
 int zmq::plain_server_t::process_hello (msg_t *msg_)
 {
     const unsigned char *ptr = static_cast <unsigned char *> (msg_->data ());
@@ -187,9 +174,7 @@ int zmq::plain_server_t::process_hello (msg_t *msg_)
         return -1;
     rc = receive_and_process_zap_reply ();
     if (rc == 0)
-        state = status_code == "200"
+        handle_zap_status_code ();
-            ? sending_welcome
-            : sending_error;
     else
     if (errno == EAGAIN)
         state = waiting_for_zap_reply;

src/plain_server.hpp

@@ -52,12 +52,9 @@ namespace zmq
         // mechanism implementation
         virtual int next_handshake_command (msg_t *msg_);
         virtual int process_handshake_command (msg_t *msg_);
-        virtual int zap_msg_available ();
 
     private:
 
-        state_t state;
-
         int produce_welcome (msg_t *msg_) const;
         int produce_ready (msg_t *msg_) const;
         int produce_error (msg_t *msg_) const;

src/zap_client.cpp

@@ -224,9 +224,13 @@ int zap_client_t::receive_and_process_zap_reply ()
 zap_client_common_handshake_t::zap_client_common_handshake_t (
   session_base_t *const session_,
   const std::string &peer_address_,
-  const options_t &options_) :
+  const options_t &options_,
+  state_t zap_reply_ok_state_) :
+    mechanism_t (options_),
     zap_client_t (session_, peer_address_, options_),
-    state (waiting_for_hello)
+    state (waiting_for_hello),
+    current_error_detail (no_detail),
+    zap_reply_ok_state (zap_reply_ok_state_)
 {
 }
 
@@ -239,4 +243,56 @@ zmq::mechanism_t::status_t zap_client_common_handshake_t::status () const
     else
         return mechanism_t::handshaking;
 }
+
+int zap_client_common_handshake_t::zap_msg_available ()
+{
+    //  TODO I don't think that it is possible that this is called in any
+    //  state other than expect_zap_reply. It should be changed to
+    //  zmq_assert (state == expect_zap_reply);
+    if (state != waiting_for_zap_reply) {
+        errno = EFSM;
+        return -1;
+    }
+    const int rc = receive_and_process_zap_reply ();
+    if (rc == 0)
+        handle_zap_status_code ();
+    else if (errno == EPROTO)
+        current_error_detail = mechanism_t::zap;
+    return rc;
+}
+
+void zap_client_common_handshake_t::handle_zap_status_code ()
+{
+    //  we can assume here that status_code is a valid ZAP status code,
+    //  i.e. 200, 300, 400 or 500
+    if (status_code[0] == '2') {
+        state = zap_reply_ok_state;
+    } else {
+        state = sending_error;
+
+        int err = 0;
+        switch (status_code[0]) {
+            case '3':
+                err = EAGAIN;
+                break;
+            case '4':
+                err = EACCES;
+                break;
+            case '5':
+                err = EFAULT;
+                break;
+        }
+        //  TODO use event_handshake_failed_zap here? but this is not a ZAP
+        //  protocol error
+
+        session->get_socket ()->event_handshake_failed_no_detail (
+          session->get_endpoint (), err);
+    }
+}
+
+mechanism_t::error_detail_t zap_client_common_handshake_t::error_detail () const
+{
+    return current_error_detail;
+}
+
 }

src/zap_client.hpp

@@ -68,13 +68,6 @@ class zap_client_t : public virtual mechanism_t
 class zap_client_common_handshake_t : public zap_client_t
 {
   protected:
-    zap_client_common_handshake_t (session_base_t *const session_,
-                                   const std::string &peer_address_,
-                                   const options_t &options_);
-
-    // methods from mechanism_t
-    status_t status () const;
-
     enum state_t
     {
         waiting_for_hello,
@@ -87,8 +80,28 @@ class zap_client_common_handshake_t : public zap_client_t
         ready
     };
 
+    zap_client_common_handshake_t (session_base_t *const session_,
+                                   const std::string &peer_address_,
+                                   const options_t &options_,
+                                   state_t zap_reply_ok_state);
+
+    // methods from mechanism_t
+    status_t status () const;
+    int zap_msg_available ();
+    error_detail_t error_detail () const;
+
+    // own methods
+    void handle_zap_status_code ();
+
     //  Current FSM state
     state_t state;
+
+    //  Details about the current error state
+    //  TODO remove this
+    error_detail_t current_error_detail;
+
+  private:
+    const state_t zap_reply_ok_state;
 };
 }