Propagate record duplication error code (#820)

In c-ares 1.30.0 we started validating strings parsed are printable.
This caused a regression in a pycares test case due to a wrong response
code being returned as the error was being propagated from a different
section of code that was assuming the only possible failure condition
was out-of-memory.

This PR adds a fix for this and also a test case to validate it.

Ref: https://github.com/saghul/pycares/issues/200
Fix By: Brad House (@bradh352)

src/lib/ares_dns_private.h

@@ -26,6 +26,8 @@
 #ifndef __ARES_DNS_PRIVATE_H
 #define __ARES_DNS_PRIVATE_H
 
+ares_status_t ares_dns_record_duplicate_ex(ares_dns_record_t **dest,
+                                           const ares_dns_record_t *src);
 ares_bool_t ares_dns_rec_type_allow_name_compression(ares_dns_rec_type_t type);
 ares_bool_t ares_dns_opcode_isvalid(ares_dns_opcode_t opcode);
 ares_bool_t ares_dns_rcode_isvalid(ares_dns_rcode_t rcode);

src/lib/ares_dns_record.c

@@ -296,6 +296,7 @@ ares_status_t ares_dns_record_query_set_name(ares_dns_record_t *dnsrec,
   if (dnsrec == NULL || idx >= dnsrec->qdcount || name == NULL) {
     return ARES_EFORMERR;
   }
+
   orig_name            = dnsrec->qd[idx].name;
   dnsrec->qd[idx].name = ares_strdup(name);
   if (dnsrec->qd[idx].name == NULL) {
@@ -1622,26 +1623,34 @@ done:
   return status;
 }
 
-ares_dns_record_t *ares_dns_record_duplicate(const ares_dns_record_t *dnsrec)
+ares_status_t ares_dns_record_duplicate_ex(ares_dns_record_t **dest,
+                                           const ares_dns_record_t *src)
 {
   unsigned char     *data     = NULL;
   size_t             data_len = 0;
-  ares_dns_record_t *out      = NULL;
   ares_status_t      status;
 
-  if (dnsrec == NULL) {
-    return NULL;
+  if (dest == NULL || src == NULL) {
+    return ARES_EFORMERR;
   }
 
-  status = ares_dns_write(dnsrec, &data, &data_len);
+  *dest = NULL;
+
+  status = ares_dns_write(src, &data, &data_len);
   if (status != ARES_SUCCESS) {
-    return NULL;
+    return status;
   }
 
-  status = ares_dns_parse(data, data_len, 0, &out);
+  status = ares_dns_parse(data, data_len, 0, dest);
   ares_free(data);
-  if (status != ARES_SUCCESS) {
-    return NULL;
-  }
-  return out;
+
+  return status;
+}
+
+ares_dns_record_t *ares_dns_record_duplicate(const ares_dns_record_t *dnsrec)
+{
+  ares_dns_record_t *dest = NULL;
+
+  ares_dns_record_duplicate_ex(&dest, dnsrec);
+  return dest;
 }

src/lib/ares_send.c

@@ -145,11 +145,11 @@ ares_status_t ares_send_nolock(ares_channel_t          *channel,
   query->using_tcp    = (channel->flags & ARES_FLAG_USEVC)?ARES_TRUE:ARES_FALSE;
 
   /* Duplicate Query */
-  query->query = ares_dns_record_duplicate(dnsrec);
-  if (query->query == NULL) {
+  status = ares_dns_record_duplicate_ex(&query->query, dnsrec);
+  if (status != ARES_SUCCESS) {
     ares_free(query);
-    callback(arg, ARES_ENOMEM, 0, NULL);
-    return ARES_ENOMEM;
+    callback(arg, status, 0, NULL);
+    return status;
   }
 
   ares_dns_record_set_id(query->query, id);

test/ares-test-mock.cc

@@ -197,6 +197,21 @@ TEST_P(MockUDPChannelTest, TruncationRetry) {
   EXPECT_EQ("{'www.google.com' aliases=[] addrs=[1.2.3.4]}", ss.str());
 }
 
+TEST_P(MockUDPChannelTest, UTF8BadName) {
+  DNSPacket reply;
+  reply.set_response().set_aa()
+    .add_question(new DNSQuestion("españa.icom.museum", T_A))
+    .add_answer(new DNSARR("españa.icom.museum", 100, {2, 3, 4, 5}));
+  ON_CALL(server_, OnRequest("españa.icom.museum", T_A))
+    .WillByDefault(SetReply(&server_, &reply));
+
+  HostResult result;
+  ares_gethostbyname(channel_, "españa.icom.museum", AF_INET, HostCallback, &result);
+  Process();
+  EXPECT_TRUE(result.done_);
+  EXPECT_EQ(ARES_EBADNAME, result.status_);
+}
+
 static int sock_cb_count = 0;
 static int SocketConnectCallback(ares_socket_t fd, int type, void *data) {
   int rc = *(int*)data;