ares_init_options with ARES_OPT_UDP_PORT wrong byte order

Regression from c-ares 1.19.1, ARES_OPT_UDP_PORT and ARES_OPT_TCP_PORT are
specified from the user in host-byte order, but there was a regression that
caused it to be read as if it was network byte order.

Fixes Bug: #640
Reported By: @Flow86
Fix By: Brad House (@bradh352)

include/ares.h

@@ -290,8 +290,8 @@ struct ares_options {
   int            timeout; /* in seconds or milliseconds, depending on options */
   int            tries;
   int            ndots;
-  unsigned short udp_port;
-  unsigned short tcp_port;
+  unsigned short udp_port; /* host byte order */
+  unsigned short tcp_port; /* host byte order */
   int            socket_send_buffer_size;
   int            socket_receive_buffer_size;
   struct in_addr    *servers;

src/lib/ares_options.c

@@ -131,10 +131,10 @@ int ares_save_options(ares_channel_t *channel, struct ares_options *options,
   }
 
   if (channel->optmask & ARES_OPT_UDP_PORT) {
-    options->udp_port = ntohs(channel->udp_port);
+    options->udp_port = channel->udp_port;
   }
   if (channel->optmask & ARES_OPT_TCP_PORT) {
-    options->tcp_port = ntohs(channel->tcp_port);
+    options->tcp_port = channel->tcp_port;
   }
 
   if (channel->optmask & ARES_OPT_SOCK_STATE_CB) {
@@ -301,11 +301,11 @@ ares_status_t ares__init_by_options(ares_channel_t            *channel,
   }
 
   if (optmask & ARES_OPT_UDP_PORT) {
-    channel->udp_port = htons(options->udp_port);
+    channel->udp_port = options->udp_port;
   }
 
   if (optmask & ARES_OPT_TCP_PORT) {
-    channel->tcp_port = htons(options->tcp_port);
+    channel->tcp_port = options->tcp_port;
   }
 
   if (optmask & ARES_OPT_SOCK_STATE_CB) {

src/lib/ares_private.h

@@ -172,8 +172,8 @@ struct server_state {
                                               * can be hard errors or timeouts
                                               */
   struct ares_addr          addr;
-  unsigned short            udp_port;
-  unsigned short            tcp_port;
+  unsigned short            udp_port; /* host byte order */
+  unsigned short            tcp_port; /* host byte order */
 
   ares__llist_t            *connections;
   struct server_connection *tcp_conn;

test/ares-test-main.cc

@@ -27,7 +27,7 @@ int main(int argc, char* argv[]) {
       ares::test::verbose = true;
     } else if ((strcmp(argv[ii], "-p") == 0) && (ii + 1 < argc)) {
       ii++;
-      ares::test::mock_port = atoi(argv[ii]);
+      ares::test::mock_port = (unsigned short)atoi(argv[ii]);
     } else if (strcmp(argv[ii], "-4") == 0) {
       ares::test::families = ares::test::ipv4_family;
       ares::test::families_modes = ares::test::ipv4_family_both_modes;

test/ares-test.cc

@@ -49,8 +49,8 @@ namespace ares {
 namespace test {
 
 bool verbose = false;
-static constexpr int dynamic_port = 0;
-int mock_port = dynamic_port;
+static constexpr unsigned short dynamic_port = 0;
+unsigned short mock_port = dynamic_port;
 
 const std::vector<int> both_families = {AF_INET, AF_INET6};
 const std::vector<int> ipv4_family = {AF_INET};
@@ -195,7 +195,7 @@ void DefaultChannelModeTest::Process() {
   ProcessWork(channel_, NoExtraFDs, nullptr);
 }
 
-MockServer::MockServer(int family, int port)
+MockServer::MockServer(int family, unsigned short port)
   : udpport_(port), tcpport_(port), qid_(-1) {
   // Create a TCP socket to receive data on.
   tcp_data_ = NULL;
@@ -456,11 +456,11 @@ void MockServer::ProcessRequest(ares_socket_t fd, struct sockaddr_storage* addr,
 }
 
 // static
-MockChannelOptsTest::NiceMockServers MockChannelOptsTest::BuildServers(int count, int family, int base_port) {
+MockChannelOptsTest::NiceMockServers MockChannelOptsTest::BuildServers(int count, int family, unsigned short base_port) {
   NiceMockServers servers;
   assert(count > 0);
-  for (int ii = 0; ii < count; ii++) {
-    int port = base_port == dynamic_port ? dynamic_port : base_port + ii;
+  for (unsigned short ii = 0; ii < count; ii++) {
+    unsigned short port = base_port == dynamic_port ? dynamic_port : base_port + ii;
     std::unique_ptr<NiceMockServer> server(new NiceMockServer(family, port));
     servers.push_back(std::move(server));
   }
@@ -483,9 +483,9 @@ MockChannelOptsTest::MockChannelOptsTest(int count,
   }
 
   // Point the library at the first mock server by default (overridden below).
-  opts.udp_port = (unsigned short)server_.udpport();
+  opts.udp_port = server_.udpport();
   optmask |= ARES_OPT_UDP_PORT;
-  opts.tcp_port = (unsigned short)server_.tcpport();
+  opts.tcp_port = server_.tcpport();
   optmask |= ARES_OPT_TCP_PORT;
 
   // If not already overridden, set short-ish timeouts.

test/ares-test.h

@@ -52,7 +52,7 @@ typedef unsigned char byte;
 namespace test {
 
 extern bool                                    verbose;
-extern int                                     mock_port;
+extern unsigned short                          mock_port;
 extern const std::vector<int>                  both_families;
 extern const std::vector<int>                  ipv4_family;
 extern const std::vector<int>                  ipv6_family;
@@ -193,7 +193,7 @@ protected:
 // Mock DNS server to allow responses to be scripted by tests.
 class MockServer {
 public:
-  MockServer(int family, int port);
+  MockServer(int family, unsigned short port);
   ~MockServer();
 
   // Mock method indicating the processing of a particular <name, RRtype>
@@ -235,12 +235,12 @@ public:
   void          ProcessFD(ares_socket_t fd);
 
   // Ports the server is responding to
-  int           udpport() const
+  unsigned short udpport() const
   {
     return udpport_;
   }
 
-  int tcpport() const
+  unsigned short tcpport() const
   {
     return tcpport_;
   }
@@ -250,8 +250,8 @@ private:
                       int qid, const std::string &name, int rrtype);
   void ProcessPacket(ares_socket_t fd, struct sockaddr_storage *addr, ares_socklen_t addrlen,
                      byte *data, int len);
-  int  udpport_;
-  int  tcpport_;
+  unsigned short udpport_;
+  unsigned short tcpport_;
   ares_socket_t  udpfd_;
   ares_socket_t  tcpfd_;
   std::set<ares_socket_t> connfds_;
@@ -280,7 +280,7 @@ protected:
   std::set<ares_socket_t>                              fds() const;
   void                                                 ProcessFD(ares_socket_t fd);
 
-  static NiceMockServers BuildServers(int count, int family, int base_port);
+  static NiceMockServers BuildServers(int count, int family, unsigned short base_port);
 
   NiceMockServers        servers_;
   // Convenience reference to first server.