Merge pull request #13 from measurement-kit/release/v0.21.0

Release/v0.21.0

.gitignore

@@ -1,5 +1,6 @@
 /.ninja_deps
 /.ninja_log
+/.vs
 /CMakeCache.txt
 /CMakeFiles/
 /CTestTestfile.cmake

CMakeLists.txt

@@ -92,6 +92,10 @@ if("${UNIX}" OR "${MINGW}")
     # for GCC, -Wmissing-prototypes only works for C/ObjC.
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wmissing-prototypes")
   endif()
+elseif("${MSVC}")
+  # TODO(bassosimone): extend this also to linker warnings.
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /WX")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /WX")
 endif()
 
 if(${WIN32})

libndt.cpp

@@ -719,41 +719,49 @@ bool Client::connect_tcp(const std::string &hostname, const std::string &port,
     EMIT_WARNING("socket already connected");
     return false;
   }
-  addrinfo hints{};
-  hints.ai_socktype = SOCK_STREAM;
-  hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
-  addrinfo *rp = nullptr;
-  int rv = this->getaddrinfo(hostname.data(), port.data(), &hints, &rp);
-  if (rv != 0) {
-    hints.ai_flags &= ~AI_NUMERICHOST;
-    rv = this->getaddrinfo(hostname.data(), port.data(), &hints, &rp);
+  // Implementation note: we could perform getaddrinfo() in one pass but having
+  // a virtual API that resolves a hostname to a vector of IP addresses makes
+  // life easier when you want to override hostname resolution, because you have
+  // to reimplement a simpler method, compared to reimplementing getaddrinfo().
+  std::vector<std::string> addresses;
+  if (!resolve(hostname, &addresses)) {
+    return false;
+  }
+  for (auto &addr : addresses) {
+    addrinfo hints{};
+    hints.ai_socktype = SOCK_STREAM;
+    hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
+    addrinfo *rp = nullptr;
+    int rv = this->getaddrinfo(addr.data(), port.data(), &hints, &rp);
     if (rv != 0) {
-      EMIT_WARNING("getaddrinfo() failed: " << gai_strerror(rv));
+      EMIT_WARNING("unexpected getaddrinfo() failure");
       return false;
     }
-    // FALLTHROUGH
-  }
-  EMIT_DEBUG("getaddrinfo(): okay");
-  for (auto aip = rp; (aip); aip = aip->ai_next) {
-    *sock = this->socket(aip->ai_family, aip->ai_socktype, 0);
-    if (*sock == -1) {
-      EMIT_WARNING("socket() failed: " << get_last_error());
-      continue;
+    assert(rp);
+    for (auto aip = rp; (aip); aip = aip->ai_next) {
+      *sock = this->socket(aip->ai_family, aip->ai_socktype, 0);
+      if (*sock == -1) {
+        EMIT_WARNING("socket() failed: " << get_last_error());
+        continue;
+      }
+      // The following two lines ensure that casting `size_t` to
+      // SockLen is safe because SockLen is `int` and the value of
+      // the ai_addrlen field is always small enough.
+      static_assert(sizeof(SockLen) == sizeof(int), "Wrong SockLen size");
+      assert(aip->ai_addrlen <= INT_MAX);
+      if (this->connect(*sock, aip->ai_addr, (SockLen)aip->ai_addrlen) == 0) {
+        EMIT_DEBUG("connect(): okay");
+        break;
+      }
+      EMIT_WARNING("connect() failed: " << get_last_error());
+      this->closesocket(*sock);
+      *sock = -1;
     }
-    // The following two lines ensure that casting `size_t` to
-    // SockLen is safe because SockLen is `int` and the value of
-    // the ai_addrlen field is always small enough.
-    static_assert(sizeof(SockLen) == sizeof(int), "Wrong SockLen size");
-    assert(aip->ai_addrlen <= INT_MAX);
-    if (this->connect(*sock, aip->ai_addr, (SockLen)aip->ai_addrlen) == 0) {
-      EMIT_DEBUG("connect(): okay");
-      break;
+    this->freeaddrinfo(rp);
+    if (*sock != -1) {
+      break; // we have a connection!
     }
-    EMIT_WARNING("connect() failed: " << get_last_error());
-    this->closesocket(*sock);
-    *sock = -1;
   }
-  this->freeaddrinfo(rp);
   return *sock != -1;
 }
 
@@ -1030,6 +1038,51 @@ bool Client::msg_read_legacy(uint8_t *code, std::string *msg) noexcept {
   return true;
 }
 
+// Utilities for low-level
+
+bool Client::resolve(const std::string &hostname,
+                     std::vector<std::string> *addrs) noexcept {
+  assert(addrs != nullptr);
+  EMIT_DEBUG("resolve: " << hostname);
+  addrinfo hints{};
+  hints.ai_socktype = SOCK_STREAM;
+  hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
+  addrinfo *rp = nullptr;
+  constexpr const char *portno = "80";  // any port would do
+  int rv = this->getaddrinfo(hostname.data(), portno, &hints, &rp);
+  if (rv != 0) {
+    hints.ai_flags &= ~AI_NUMERICHOST;
+    rv = this->getaddrinfo(hostname.data(), portno, &hints, &rp);
+    if (rv != 0) {
+      EMIT_WARNING("getaddrinfo() failed: " << gai_strerror(rv));
+      return false;
+    }
+    // FALLTHROUGH
+  }
+  assert(rp);
+  EMIT_DEBUG("getaddrinfo(): okay");
+  auto result = true;
+  for (auto aip = rp; (aip); aip = aip->ai_next) {
+    char address[NI_MAXHOST], port[NI_MAXSERV];
+    // The following two lines ensure that casting `size_t` to
+    // SockLen is safe because SockLen is `int` and the value of
+    // the ai_addrlen field is always small enough.
+    static_assert(sizeof(SockLen) == sizeof(int), "Wrong SockLen size");
+    assert(sizeof(address) <= INT_MAX && sizeof(port) <= INT_MAX);
+    if (this->getnameinfo(aip->ai_addr, (SockLen)aip->ai_addrlen, address,
+                          (SockLen)sizeof(address), port, (SockLen)sizeof(port),
+                          NI_NUMERICHOST | NI_NUMERICSERV) != 0) {
+      EMIT_WARNING("unexpected getnameinfo() failure");
+      result = false;
+      break;
+    }
+    addrs->push_back(address); // we only care about address
+    EMIT_DEBUG("- " << address);
+  }
+  this->freeaddrinfo(rp);
+  return result;
+}
+
 // Dependencies (curl)
 
 bool Client::query_mlabns_curl(const std::string &url, long timeout,
@@ -1085,6 +1138,12 @@ int Client::getaddrinfo(const char *domain, const char *port,
   return ::getaddrinfo(domain, port, hints, res);
 }
 
+int Client::getnameinfo(const sockaddr *sa, SockLen salen, char *host,
+                        SockLen hostlen, char *serv, SockLen servlen,
+                        int flags) noexcept {
+  return ::getnameinfo(sa, salen, host, hostlen, serv, servlen, flags);
+}
+
 void Client::freeaddrinfo(addrinfo *aip) noexcept { ::freeaddrinfo(aip); }
 
 Socket Client::socket(int domain, int type, int protocol) noexcept {

libndt.hpp

@@ -17,6 +17,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <vector>
 
 struct addrinfo;
 struct sockaddr;
@@ -26,8 +27,8 @@ namespace measurement_kit {
 namespace libndt {
 
 constexpr uint64_t api_major = 0;
-constexpr uint64_t api_minor = 20;
-constexpr uint64_t api_patch = 2;
+constexpr uint64_t api_minor = 21;
+constexpr uint64_t api_patch = 0;
 
 constexpr uint8_t nettest_middlebox = 1 << 0;
 constexpr uint8_t nettest_upload = 1 << 1;
@@ -157,6 +158,11 @@ class Client {
 
   virtual bool msg_read_legacy(uint8_t *code, std::string *msg) noexcept;
 
+  // Utilities for low-level
+
+  virtual bool resolve(const std::string &hostname,
+                       std::vector<std::string> *addrs) noexcept;
+
   // Dependencies (cURL)
 
   virtual bool query_mlabns_curl(const std::string &url, long timeout,
@@ -169,6 +175,9 @@ class Client {
 
   virtual int getaddrinfo(const char *domain, const char *port,
                           const addrinfo *hints, addrinfo **res) noexcept;
+  virtual int getnameinfo(const sockaddr *sa, SockLen salen, char *host,
+                          SockLen hostlen, char *serv, SockLen servlen,
+                          int flags) noexcept;
   virtual void freeaddrinfo(addrinfo *aip) noexcept;
 
   virtual Socket socket(int domain, int type, int protocol) noexcept;

libndt_test.cpp

@@ -1167,17 +1167,39 @@ TEST_CASE("Client::connect_tcp() requires initial socket to be -1") {
   REQUIRE(client.connect_tcp("1.2.3.4", "33", &sock) == false);
 }
 
-class FailGetaddrinfo : public libndt::Client {
+class FailResolve : public libndt::Client {
  public:
   using libndt::Client::Client;
+  bool resolve(const std::string &,
+               std::vector<std::string> *) noexcept override {
+    return false;
+  }
+};
+
+TEST_CASE("Client::connect_tcp() deals with Client::resolve() failure") {
+  FailResolve client;
+  libndt::Socket sock = -1;
+  client.settings.verbosity = libndt::verbosity_quiet;
+  REQUIRE(client.connect_tcp("1.2.3.4", "33", &sock) == false);
+}
+
+class FailGetaddrinfoInConnectTcp : public libndt::Client {
+ public:
+  using libndt::Client::Client;
+  bool resolve(const std::string &str,
+               std::vector<std::string> *addrs) noexcept override {
+    REQUIRE(str == "1.2.3.4"); // make sure it did not change
+    addrs->push_back(str);
+    return true;
+  }
   int getaddrinfo(const char *, const char *, const addrinfo *,
                   addrinfo **) noexcept override {
     return EAI_AGAIN;
   }
 };
 
 TEST_CASE("Client::connect_tcp() deals with Client::getaddrinfo() failure") {
-  FailGetaddrinfo client;
+  FailGetaddrinfoInConnectTcp client;
   libndt::Socket sock = -1;
   client.settings.verbosity = libndt::verbosity_quiet;
   REQUIRE(client.connect_tcp("1.2.3.4", "33", &sock) == false);
@@ -1587,6 +1609,41 @@ TEST_CASE(
   REQUIRE(client.msg_read_legacy(&code, &s) == false);
 }
 
+// Client::resolve() tests
+// -----------------------
+
+class FailGetaddrinfo : public libndt::Client {
+ public:
+  using libndt::Client::Client;
+  int getaddrinfo(const char *, const char *, const addrinfo *,
+                  addrinfo **) noexcept override {
+    return EAI_AGAIN;
+  }
+};
+
+TEST_CASE("Client::resolve() deals with Client::getaddrinfo() failure") {
+  FailGetaddrinfo client;
+  std::vector<std::string> addrs;
+  client.settings.verbosity = libndt::verbosity_quiet;
+  REQUIRE(client.resolve("x.org", &addrs) == false);
+}
+
+class FailGetnameinfo : public libndt::Client {
+ public:
+  using libndt::Client::Client;
+  int getnameinfo(const sockaddr *, libndt::SockLen, char *, libndt::SockLen,
+                  char *, libndt::SockLen, int) noexcept override {
+    return EAI_AGAIN;
+  }
+};
+
+TEST_CASE("Client::resolve() deals with Client::getnameinfo() failure") {
+  FailGetnameinfo client;
+  std::vector<std::string> addrs;
+  client.settings.verbosity = libndt::verbosity_quiet;
+  REQUIRE(client.resolve("x.org", &addrs) == false);
+}
+
 // Client::query_mlabns_curl() tests
 // ---------------------------------