Merge pull request #1132 from kiwix/feature/best-public-ip

Refactor getBestPublicIp for all valid ips
kiwix · Oct 2, 2024 · 8bbda99 · 8bbda99
2 parents bef3ec7 + 95529d2
commit 8bbda99
Show file tree

Hide file tree

Showing 8 changed files with 101 additions and 60 deletions.
diff --git a/include/common.h b/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;
 

diff --git a/include/server.h b/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; }
@@ -64,15 +64,15 @@ namespace kiwix
        void setBlockExternalLinks(bool blockExternalLinks)
         { m_blockExternalLinks = blockExternalLinks; }
        void setIpMode(IpMode mode) { m_ipMode = mode; }
-       int getPort();
-       std::string getAddress();
+       int getPort() 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;
   };

diff --git a/include/tools.h b/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

diff --git a/src/server.cpp b/src/server.cpp
@@ -75,12 +75,24 @@ void Server::setRoot(const std::string& root)
   }
 }
 
-int Server::getPort()
+// 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()
+IpAddress Server::getAddress() const
 {
   return mp_server->getAddress();
 }

diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp
@@ -85,6 +85,19 @@ namespace kiwix {
 namespace
 {
 
+bool ipAvailable(const std::string addr)
+{
+  auto interfaces = kiwix::getNetworkInterfacesIPv4Or6();
+
+  for (const auto& [_, interfaceIps] : interfaces) {
+    if ((interfaceIps.addr == addr) || (interfaceIps.addr6 == addr)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 inline std::string normalizeRootUrl(std::string rootUrl)
 {
   while ( !rootUrl.empty() && rootUrl.back() == '/' )
@@ -407,7 +420,7 @@ class InternalServer::CustomizedResources : public std::map<std::string, Customi
 
 InternalServer::InternalServer(LibraryPtr library,
                                std::shared_ptr<NameMapper> nameMapper,
-                               std::string addr,
+                               IpAddress addr,
                                int port,
                                std::string root,
                                int nbThreads,
@@ -461,30 +474,44 @@ 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;
+    } 
+
+    if (!ipAvailable(addr)) {
+      std::cerr << "ERROR: IP address is not available on this system: " << addr << std::endl;
       return false;
     }
+
+    m_ipMode = !m_addr.addr.empty() ? IpMode::IPV4 : IpMode::IPV6; 
   }
 
-  if (m_ipMode == IpMode::all) {
+  if (m_ipMode == IpMode::ALL) {
     flags|=MHD_USE_DUAL_STACK;
-  } else if (m_ipMode == IpMode::ipv6) {
+  } else if (m_ipMode == IpMode::IPV6) {
     flags|=MHD_USE_IPv6;
   }
 
-  struct sockaddr* sockaddr = (m_ipMode==IpMode::all || m_ipMode==IpMode::ipv6)
+  struct sockaddr* sockaddr = (m_ipMode==IpMode::ALL || m_ipMode==IpMode::IPV6)
                               ? (struct sockaddr*)&sockAddr6
                               : (struct sockaddr*)&sockAddr4;
 

diff --git a/src/server/internalServer.h b/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,8 +118,8 @@ class InternalServer {
                                void** cont_cls);
     bool start();
     void stop();
-    std::string getAddress() { return m_addr; }
-    int getPort() { return m_port; }
+    IpAddress getAddress() const { return m_addr; }
+    int getPort() const { return m_port; }
     IpMode getIpMode() const { return m_ipMode; }
 
   private: // functions
@@ -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

diff --git a/src/tools/networkTools.cpp b/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,40 +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;
+  const char* const prioritizedNames[] = { "eth0", "eth1", "wlan0", "wlan1", "en0", "en1" };
+  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
diff --git a/test/otherTools.cpp b/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;
 }