Empty TXT records were not being preserved (#922)

When parsing, preserve empty TXT records.

Fixes #921
Fix-By: Brad House (@bradh352)

src/lib/record/ares_dns_multistring.c

@@ -146,6 +146,18 @@ ares_status_t ares_dns_multistring_add_own(ares_dns_multistring_t *strs,
     return status;
   }
 
+  /* Issue #921, ares_dns_multistring_get() doesn't have a way to indicate
+   * success or fail on a zero-length string which is actually valid.  So we
+   * are going to allocate a 1-byte buffer to use as a placeholder in this
+   * case */
+  if (str == NULL) {
+    str = ares_malloc_zero(1);
+    if (str == NULL) {
+      ares_array_remove_last(strs->strs);
+      return ARES_ENOMEM;
+    }
+  }
+
   data->data = str;
   data->len  = len;
 
@@ -252,36 +264,38 @@ ares_status_t ares_dns_multistring_parse_buf(ares_buf_t *buf,
       break; /* LCOV_EXCL_LINE: DefensiveCoding */
     }
 
-    if (len) {
+
-      /* When used by the _str() parser, it really needs to be validated to
+    /* When used by the _str() parser, it really needs to be validated to
-       * be a valid printable ascii string.  Do that here */
+     * be a valid printable ascii string.  Do that here */
-      if (validate_printable && ares_buf_len(buf) >= len) {
+    if (len && validate_printable && ares_buf_len(buf) >= len) {
-        size_t      mylen;
+      size_t      mylen;
-        const char *data = (const char *)ares_buf_peek(buf, &mylen);
+      const char *data = (const char *)ares_buf_peek(buf, &mylen);
-        if (!ares_str_isprint(data, len)) {
+      if (!ares_str_isprint(data, len)) {
-          status = ARES_EBADSTR;
+        status = ARES_EBADSTR;
-          break;
+        break;
-        }
       }
+    }
 
-      if (strs != NULL) {
+    if (strs != NULL) {
-        unsigned char *data = NULL;
+      unsigned char *data = NULL;
+      if (len) {
         status = ares_buf_fetch_bytes_dup(buf, len, ARES_TRUE, &data);
         if (status != ARES_SUCCESS) {
           break;
         }
-        status = ares_dns_multistring_add_own(*strs, data, len);
+      }
-        if (status != ARES_SUCCESS) {
+      status = ares_dns_multistring_add_own(*strs, data, len);
-          ares_free(data);
+      if (status != ARES_SUCCESS) {
-          break;
+        ares_free(data);
-        }
+        break;
-      } else {
+      }
-        status = ares_buf_consume(buf, len);
+    } else {
-        if (status != ARES_SUCCESS) {
+      status = ares_buf_consume(buf, len);
-          break;
+      if (status != ARES_SUCCESS) {
-        }
+        break;
       }
     }
+
   }
 
   if (status != ARES_SUCCESS && strs != NULL) {

test/ares-test-parse-txt.cc

@@ -38,7 +38,7 @@ TEST_F(LibraryTest, ParseTxtReplyOK) {
   std::string expected2a = "txt2a";
   std::string expected2b("ABC\0ABC", 7);
   pkt.set_qid(0x1234).set_response().set_aa()
-    .add_question(new DNSQuestion("example.com", T_MX))
+    .add_question(new DNSQuestion("example.com", T_TXT))
     .add_answer(new DNSTxtRR("example.com", 100, {expected1}))
     .add_answer(new DNSTxtRR("example.com", 100, {expected2a, expected2b}));
   std::vector<byte> data = pkt.data();
@@ -68,7 +68,7 @@ TEST_F(LibraryTest, ParseTxtExtReplyOK) {
   std::string expected2a = "txt2a";
   std::string expected2b("ABC\0ABC", 7);
   pkt.set_qid(0x1234).set_response().set_aa()
-    .add_question(new DNSQuestion("example.com", T_MX))
+    .add_question(new DNSQuestion("example.com", T_TXT))
     .add_answer(new DNSTxtRR("example.com", 100, {expected1}))
     .add_answer(new DNSTxtRR("example.com", 100, {expected2a, expected2b}));
   std::vector<byte> data = pkt.data();
@@ -95,6 +95,39 @@ TEST_F(LibraryTest, ParseTxtExtReplyOK) {
   ares_free_data(txt);
 }
 
+TEST_F(LibraryTest, ParseTxtEmpty) {
+  DNSPacket pkt;
+  std::string expected1 = "";
+  pkt.set_qid(0x1234).set_response().set_aa()
+    .add_question(new DNSQuestion("example.com", T_TXT))
+    .add_answer(new DNSTxtRR("example.com", 100, {expected1}));
+  std::vector<byte> data = pkt.data();
+
+  ares_dns_record_t   *dnsrec = NULL;
+  ares_dns_rr_t       *rr     = NULL;
+  EXPECT_EQ(ARES_SUCCESS, ares_dns_parse(data.data(), data.size(), 0, &dnsrec));
+  EXPECT_EQ(1, ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ANSWER));
+  rr = ares_dns_record_rr_get(dnsrec, ARES_SECTION_ANSWER, 0);
+  ASSERT_NE(nullptr, rr);
+  EXPECT_EQ(ARES_REC_TYPE_TXT, ares_dns_rr_get_type(rr));
+
+  size_t txtdata_len;
+  const unsigned char *txtdata;
+
+  /* Using array methodology */
+  EXPECT_EQ(1, ares_dns_rr_get_abin_cnt(rr, ARES_RR_TXT_DATA));
+  txtdata = ares_dns_rr_get_abin(rr, ARES_RR_TXT_DATA, 0, &txtdata_len);
+  EXPECT_EQ(txtdata_len, 0);
+  EXPECT_NE(nullptr, txtdata);
+
+  /* Using combined methodology */
+  txtdata = ares_dns_rr_get_bin(rr, ARES_RR_TXT_DATA, &txtdata_len);
+  EXPECT_EQ(txtdata_len, 0);
+  EXPECT_NE(nullptr, txtdata);
+
+  ares_dns_record_destroy(dnsrec); dnsrec = NULL;
+}
+
 TEST_F(LibraryTest, ParseTxtMalformedReply1) {
   std::vector<byte> data = {
     0x12, 0x34,  // qid
@@ -213,7 +246,7 @@ TEST_F(LibraryTest, ParseTxtReplyErrors) {
   std::string expected2a = "txt2a";
   std::string expected2b = "txt2b";
   pkt.set_qid(0x1234).set_response().set_aa()
-    .add_question(new DNSQuestion("example.com", T_MX))
+    .add_question(new DNSQuestion("example.com", T_TXT))
     .add_answer(new DNSTxtRR("example.com", 100, {expected1}))
     .add_answer(new DNSTxtRR("example.com", 100, {expected1}))
     .add_answer(new DNSTxtRR("example.com", 100, {expected2a, expected2b}));
@@ -227,7 +260,7 @@ TEST_F(LibraryTest, ParseTxtReplyErrors) {
   txt = nullptr;
   EXPECT_EQ(ARES_EBADRESP, ares_parse_txt_reply(data.data(), (int)data.size(), &txt));
   EXPECT_EQ(nullptr, txt);
-  pkt.add_question(new DNSQuestion("example.com", T_MX));
+  pkt.add_question(new DNSQuestion("example.com", T_TXT));
 
 #ifdef DISABLED
   // Question != answer
@@ -240,13 +273,13 @@ TEST_F(LibraryTest, ParseTxtReplyErrors) {
 #endif
 
   // Two questions.
-  pkt.add_question(new DNSQuestion("example.com", T_MX));
+  pkt.add_question(new DNSQuestion("example.com", T_TXT));
   data = pkt.data();
   txt = nullptr;
   EXPECT_EQ(ARES_EBADRESP, ares_parse_txt_reply(data.data(), (int)data.size(), &txt));
   EXPECT_EQ(nullptr, txt);
   pkt.questions_.clear();
-  pkt.add_question(new DNSQuestion("example.com", T_MX));
+  pkt.add_question(new DNSQuestion("example.com", T_TXT));
 
   // No answer.
   pkt.answers_.clear();
@@ -274,7 +307,7 @@ TEST_F(LibraryTest, ParseTxtReplyAllocFail) {
   std::string expected2a = "txt2a";
   std::string expected2b = "txt2b";
   pkt.set_qid(0x1234).set_response().set_aa()
-    .add_question(new DNSQuestion("example.com", T_MX))
+    .add_question(new DNSQuestion("example.com", T_TXT))
     .add_answer(new DNSCnameRR("example.com", 300, "c.example.com"))
     .add_answer(new DNSTxtRR("c.example.com", 100, {expected1}))
     .add_answer(new DNSTxtRR("c.example.com", 100, {expected1}))