From 95fd61b8dc652d2d20a555c84fe7bcd370f7016e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Mon, 6 May 2019 15:50:07 +0300
Subject: [PATCH] Fix listener search functions

The functions that searched for listeners compared both sockets and
addresses in the same function. This made its use error prone and caused
false positives in some cases.
---
 include/maxscale/listener.hh  | 20 +++++++++++++-------
 server/core/config.cc         | 23 ++++++++++++++---------
 server/core/config_runtime.cc |  2 +-
 server/core/listener.cc       | 24 +++++++++++++++++++-----
 4 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/include/maxscale/listener.hh b/include/maxscale/listener.hh
index 0549c21d3..2d1b43554 100644
--- a/include/maxscale/listener.hh
+++ b/include/maxscale/listener.hh
@@ -350,17 +350,23 @@ SListener listener_find(const std::string& name);
 std::vector<SListener> listener_find_by_service(const SERVICE* service);
 
 /**
- * Find listener by configuration
+ * Find listener by socket
  *
- * @param socket  Optional path to a socket file
- * @param address Address where the listener listens
- * @param port    The port on which the listener listens
+ * @param socket  Path to a socket file
  *
  * @return The matching listener if one was found
  */
-SListener listener_find_by_config(const std::string& socket,
-                                  const std::string& address,
-                                  unsigned short port);
+SListener listener_find_by_socket(const std::string& socket);
+
+/**
+ * Find listener by address and port
+ *
+ * @param address Network address
+ * @param port    Network port
+ *
+ * @return The matching listener if one was found
+ */
+SListener listener_find_by_address(const std::string& address, unsigned short port);
 
 int  listener_set_ssl_version(SSL_LISTENER* ssl_listener, const char* version);
 void listener_set_certificates(SSL_LISTENER* ssl_listener, const std::string& cert,
diff --git a/server/core/config.cc b/server/core/config.cc
index 83eb4225a..092a94323 100644
--- a/server/core/config.cc
+++ b/server/core/config.cc
@@ -4172,17 +4172,22 @@ int create_new_listener(CONFIG_CONTEXT* obj)
             address = "";
         }
 
-        if (auto l = listener_find_by_config(socket, address, port))
+        if (socket_defined)
+        {
+            if (auto l = listener_find_by_socket(socket))
+            {
+                MXS_ERROR("Creation of listener '%s' for service '%s' failed, because "
+                          "listener '%s' already listens on socket %s.",
+                          obj->name(), service->name(), l->name(), socket.c_str());
+                return 1;
+            }
+        }
+        else if (auto l = listener_find_by_address(address, port))
         {
-            string socket_type = socket_defined ? "socket" : "port";
-            string socket_definition = socket_defined ? socket : obj->m_parameters.get_string(CN_PORT);
             MXS_ERROR("Creation of listener '%s' for service '%s' failed, because "
-                      "listener '%s' already listens on the %s %s.",
-                      obj->name(),
-                      service->name(),
-                      l->name(),
-                      socket_type.c_str(),
-                      socket_definition.c_str());
+                      "listener '%s' already listens on port %s.",
+                      obj->name(), service->name(), l->name(),
+                      obj->m_parameters.get_string(CN_PORT).c_str());
             return 1;
         }
 
diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc
index 01970c5b3..b14c3e92b 100644
--- a/server/core/config_runtime.cc
+++ b/server/core/config_runtime.cc
@@ -1172,7 +1172,7 @@ bool runtime_create_listener(Service* service,
     {
         config_runtime_error("Listener '%s' already exists", name);
     }
-    else if (SListener l = listener_find_by_config("", addr, u_port))
+    else if (SListener l = listener_find_by_address(addr, u_port))
     {
         config_runtime_error("Listener '%s' already listens on [%s]:%u", l->name(), addr, u_port);
     }
diff --git a/server/core/listener.cc b/server/core/listener.cc
index 6ccca1d62..1f310a953 100644
--- a/server/core/listener.cc
+++ b/server/core/listener.cc
@@ -338,9 +338,24 @@ static bool is_all_iface(const std::string& a, const std::string& b)
     return addresses.count(a) || addresses.count(b);
 }
 
-SListener listener_find_by_config(const std::string& socket,
-                                  const std::string& address,
-                                  unsigned short port)
+SListener listener_find_by_socket(const std::string& socket)
+{
+    SListener rval;
+    std::lock_guard<std::mutex> guard(listener_lock);
+
+    for (const auto& listener : all_listeners)
+    {
+        if (listener->address() == socket)
+        {
+            rval = listener;
+            break;
+        }
+    }
+
+    return rval;
+}
+
+SListener listener_find_by_address(const std::string& address, unsigned short port)
 {
     SListener rval;
     std::lock_guard<std::mutex> guard(listener_lock);
@@ -348,8 +363,7 @@ SListener listener_find_by_config(const std::string& socket,
     for (const auto& listener : all_listeners)
     {
         if (port == listener->port()
-            && (listener->address() == address || listener->address() == socket
-                || is_all_iface(listener->address(), address)))
+            && (listener->address() == address || is_all_iface(listener->address(), address)))
         {
             rval = listener;
             break;