Refactor getBestPublicIp for both protocols

include/common.h

@@ -16,7 +16,7 @@
 
 namespace kiwix {
 
-enum class IpMode { IPV4, IPV6, ALL };
+enum class IpMode { IPV4, IPV6, ALL, AUTO }; // AUTO: Server decides the protocol
 typedef zim::size_type size_type;
 typedef zim::offset_type offset_type;
 

include/server.h

@@ -22,7 +22,7 @@
 
 #include <string>
 #include <memory>
-#include "common.h"
+#include "tools.h"
 
 namespace kiwix
 {
@@ -52,7 +52,7 @@ namespace kiwix
        void stop();
 
        void setRoot(const std::string& root);
-       void setAddress(const std::string& addr) { m_addr = addr; }
+       void setAddress(const std::string& addr);
        void setPort(int port) { m_port = port; }
        void setNbThreads(int threads) { m_nbThreads = threads; }
        void setMultiZimSearchLimit(unsigned int limit) { m_multizimSearchLimit = limit; }
@@ -65,14 +65,14 @@ namespace kiwix
         { m_blockExternalLinks = blockExternalLinks; }
        void setIpMode(IpMode mode) { m_ipMode = mode; }
        int getPort() const;
-       std::string getAddress() const;
+       IpAddress getAddress() const;
        IpMode getIpMode() const;
 
      protected:
        std::shared_ptr<Library> mp_library;
        std::shared_ptr<NameMapper> mp_nameMapper;
        std::string m_root = "";
-       std::string m_addr = "";
+       IpAddress m_addr;
        std::string m_indexTemplateString = "";
        int m_port = 80;
        int m_nbThreads = 1;
@@ -81,7 +81,7 @@ namespace kiwix
        bool m_withTaskbar = true;
        bool m_withLibraryButton = true;
        bool m_blockExternalLinks = false;
-       IpMode m_ipMode = IpMode::IPV4;
+       IpMode m_ipMode = IpMode::AUTO;
        int m_ipConnectionLimit = 0;
        std::unique_ptr<InternalServer> mp_server;
   };

include/tools.h

@@ -24,6 +24,7 @@
 #include <vector>
 #include <map>
 #include <cstdint>
+#include "common.h"
 
 namespace kiwix
 {
@@ -214,9 +215,10 @@ std::map<std::string, IpAddress> getNetworkInterfacesIPv4Or6();
 std::map<std::string, std::string> getNetworkInterfaces();
 
 /** Provides the best IP address
- * This function provides the best IP address from the list given by getNetworkInterfacesIPv4Or6()
+ * This function provides the best IP addresses for both ipv4 and ipv6 protocols,
+ * in an IpAddress struct, based on the list given by getNetworkInterfacesIPv4Or6()
  */
-std::string getBestPublicIp(bool ipv6);
+IpAddress getBestPublicIps();
 
 /** Provides the best IPv4 adddress
  * Equivalent to getBestPublicIp(false). Provided for backward compatibility

src/server.cpp

@@ -75,12 +75,24 @@ void Server::setRoot(const std::string& root)
   }
 }
 
+// FIXME: this method is implemented under the assumption that it is invoked only once (per object).
+void Server::setAddress(const std::string& addr)
+{
+  if (addr.empty()) return;
+
+  if (addr.find(':') != std::string::npos) { // IPv6
+    m_addr.addr6 = (addr[0] == '[') ? addr.substr(1, addr.length() - 2) : addr; // Remove brackets if any
+  } else {
+    m_addr.addr = addr;
+  }
+}
+
 int Server::getPort() const
 {
   return mp_server->getPort();
 }
 
-std::string Server::getAddress() const
+IpAddress Server::getAddress() const
 {
   return mp_server->getAddress();
 }

src/server/internalServer.cpp

@@ -407,7 +407,7 @@ public:
 
 InternalServer::InternalServer(LibraryPtr library,
                                std::shared_ptr<NameMapper> nameMapper,
-                               std::string addr,
+                               IpAddress addr,
                                int port,
                                std::string root,
                                int nbThreads,
@@ -461,21 +461,30 @@ bool InternalServer::start() {
   sockAddr6.sin6_family = AF_INET6;
   sockAddr6.sin6_port = htons(m_port);
 
-  if (m_addr.empty()) {
-    if (0 != INADDR_ANY) {
-      sockAddr6.sin6_addr = in6addr_any;
-      sockAddr4.sin_addr.s_addr = htonl(INADDR_ANY);
-    }
-    m_addr = kiwix::getBestPublicIp(m_ipMode == IpMode::IPV6 || m_ipMode == IpMode::ALL);
+  if (m_addr.addr.empty() && m_addr.addr6.empty()) { // No ip address provided
+    if (m_ipMode == IpMode::AUTO) m_ipMode = IpMode::ALL;
+    sockAddr6.sin6_addr = in6addr_any;
+    sockAddr4.sin_addr.s_addr = htonl(INADDR_ANY);
+    IpAddress bestIps = kiwix::getBestPublicIps();
+    if (m_ipMode == IpMode::IPV4 || m_ipMode == IpMode::ALL) m_addr.addr = bestIps.addr;
+    if (m_ipMode == IpMode::IPV6 || m_ipMode == IpMode::ALL) m_addr.addr6 = bestIps.addr6;
   } else {
-    bool ipv6 = inet_pton(AF_INET6, m_addr.c_str(), &(sockAddr6.sin6_addr.s6_addr)) == 1;
-    bool ipv4 = inet_pton(AF_INET, m_addr.c_str(), &(sockAddr4.sin_addr.s_addr)) == 1;
-    if (ipv6){
-       m_ipMode = IpMode::ALL;
-    } else if (!ipv4) {
-      std::cerr << "Ip address " << m_addr << "  is not a valid ip address" << std::endl;
+    const std::string addr = !m_addr.addr.empty() ? m_addr.addr : m_addr.addr6;
+
+    if (m_ipMode != kiwix::IpMode::AUTO) {
+      std::cerr << "ERROR: When an IP address is provided the IP mode must not be set" << std::endl;
       return false;
     }
+
+    bool validV4 = inet_pton(AF_INET, m_addr.addr.c_str(), &(sockAddr4.sin_addr.s_addr)) == 1;
+    bool validV6 = inet_pton(AF_INET6, m_addr.addr6.c_str(), &(sockAddr6.sin6_addr.s6_addr)) == 1;
+
+    if (!validV4 && !validV6) {
+      std::cerr << "ERROR: invalid IP address: " << addr << std::endl;
+      return false;
+    } 
+
+    m_ipMode = !m_addr.addr.empty() ? IpMode::IPV4 : IpMode::IPV6; 
   }
 
   if (m_ipMode == IpMode::ALL) {

src/server/internalServer.h

@@ -27,6 +27,7 @@ extern "C" {
 
 #include "library.h"
 #include "name_mapper.h"
+#include "tools.h"
 
 #include <zim/search.h>
 #include <zim/suggestion.h>
@@ -94,7 +95,7 @@ class InternalServer {
   public:
     InternalServer(LibraryPtr library,
                    std::shared_ptr<NameMapper> nameMapper,
-                   std::string addr,
+                   IpAddress addr,
                    int port,
                    std::string root,
                    int nbThreads,
@@ -117,7 +118,7 @@ class InternalServer {
                                void** cont_cls);
     bool start();
     void stop();
-    std::string getAddress() const { return m_addr; }
+    IpAddress getAddress() const { return m_addr; }
     int getPort() const { return m_port; }
     IpMode getIpMode() const { return m_ipMode; }
 
@@ -166,7 +167,7 @@ class InternalServer {
     typedef ConcurrentCache<std::string, std::shared_ptr<LockableSuggestionSearcher>> SuggestionSearcherCache;
 
   private: // data
-    std::string m_addr;
+    IpAddress m_addr;
     int m_port;
     std::string m_root;                   // URI-encoded
     std::string m_rootPrefixOfDecodedURL; // URI-decoded

src/tools/networkTools.cpp

@@ -19,6 +19,7 @@
  */
 
 #include "tools.h"
+#include "stringTools.h"
 #include <tools/networkTools.h>
 
 #include <stdio.h>
@@ -62,6 +63,12 @@ size_t write_callback_to_iss(char* ptr, size_t size, size_t nmemb, void* userdat
   return nmemb;
 }
 
+void updatePublicIpAddress(IpAddress& publicIpAddr, const IpAddress& interfaceIpAddr) 
+{
+  if (publicIpAddr.addr.empty())  publicIpAddr.addr  = interfaceIpAddr.addr;
+  if (publicIpAddr.addr6.empty()) publicIpAddr.addr6 = interfaceIpAddr.addr6;
+}
+
 } // unnamed namespace
 
 std::string download(const std::string& url) {
@@ -85,7 +92,6 @@ std::string download(const std::string& url) {
   return ss.str();
 }
 
-
 namespace
 {
 
@@ -211,39 +217,35 @@ std::map<std::string, std::string> getNetworkInterfaces() {
   return result;
 }
 
-
-std::string getBestPublicIp(bool ipv6) {
-  IpAddress bestPublicIp = IpAddress{"127.0.0.1","::1"};
+IpAddress getBestPublicIps() {
+  IpAddress bestPublicIps;
   std::map<std::string, IpAddress> interfaces = getNetworkInterfacesIPv4Or6();
-
 #ifndef _WIN32
   const char* const prioritizedNames[] = { "eth0", "eth1", "wlan0", "wlan1", "en0", "en1" };
-  for(auto name: prioritizedNames) {
-    auto it=interfaces.find(name);
-    if(it != interfaces.end() && !(ipv6 && (*it).second.addr6.empty())) {
-      bestPublicIp = (*it).second;
-      break;
+  for (const auto& name : prioritizedNames) {
+    const auto it = interfaces.find(name);
+    if (it != interfaces.end()) {
+      updatePublicIpAddress(bestPublicIps, it->second);
     }
   }
 #endif
-
-  const char* const prefixes[] = { "192.168", "172.16.", "10.0" };
-  for(auto prefix : prefixes){
-    for(auto& itr : interfaces) {
-      std::string interfaceIp(itr.second.addr);
-      if (interfaceIp.find(prefix) == 0 && !(ipv6 && itr.second.addr6.empty())) {
-        bestPublicIp = itr.second;
-        break;
+  const char* const v4prefixes[] = { "192.168", "172.16", "10.0", "169.254" };
+  for (const auto& prefix : v4prefixes) {
+    for (const auto& [_, interfaceIps] : interfaces) {
+      if (kiwix::startsWith(interfaceIps.addr, prefix)) {
+        updatePublicIpAddress(bestPublicIps, interfaceIps);
       }
     }
   }
-  return ipv6 ? bestPublicIp.addr6 : bestPublicIp.addr;
-}
 
+  updatePublicIpAddress(bestPublicIps, {"127.0.0.1", "::1"});
+
+  return bestPublicIps;
+}
 
 std::string getBestPublicIp()
 {
-  return getBestPublicIp(false);
+  return getBestPublicIps().addr;
 }
 
 } // namespace kiwix

test/otherTools.cpp

@@ -18,6 +18,7 @@
  */
 
 #include "gtest/gtest.h"
+#include "../include/tools.h"
 #include "../src/tools/otherTools.h"
 #include "zim/suggestion_iterator.h"
 #include "../src/server/i18n_utils.h"
@@ -252,11 +253,8 @@ TEST(networkTools, getNetworkInterfaces)
   }
 }
 
-TEST(networkTools, getBestPublicIp)
+TEST(networkTools, getBestPublicIps)
 {
-  using kiwix::getBestPublicIp;
-
-  std::cout << "getBestPublicIp(true)  " << getBestPublicIp(true)  << std::endl;
-  std::cout << "getBestPublicIp(false) " << getBestPublicIp(false) << std::endl;
-  std::cout << "getBestPublicIp()      " << getBestPublicIp()      << std::endl;
+  std::cout << "getBestPublicIps(): " << "[" << kiwix::getBestPublicIps().addr << ", " << kiwix::getBestPublicIps().addr6 << "]" << std::endl;
+  std::cout << "getBestPublicIp(): " << kiwix::getBestPublicIp() << std::endl;
 }