Empty TXT records were not being preserved (#922)

When parsing, preserve empty TXT records.

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

src/lib/ares__buf.c

@@ -971,10 +971,10 @@ ares_status_t ares__buf_parse_dns_abinstr(ares__buf_t *buf,
       break; /* LCOV_EXCL_LINE: DefensiveCoding */
     }
 
-    if (len) {
+
     /* When used by the _str() parser, it really needs to be validated to
      * 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;
       const char *data = (const char *)ares__buf_peek(buf, &mylen);
       if (!ares__str_isprint(data, len)) {
@@ -985,10 +985,12 @@ ares_status_t ares__buf_parse_dns_abinstr(ares__buf_t *buf,
 
     if (strs != 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) {
         ares_free(data);
@@ -1000,7 +1002,7 @@ ares_status_t ares__buf_parse_dns_abinstr(ares__buf_t *buf,
         break;
       }
     }
-    }
+
   }
 
   if (status != ARES_SUCCESS && strs != NULL) {

src/lib/ares_dns_multistring.c

@@ -138,6 +138,17 @@ ares_status_t ares__dns_multistring_add_own(ares__dns_multistring_t *strs,
     strs->alloc = newalloc;
   }
 
+  /* 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) {
+      return ARES_ENOMEM;
+    }
+  }
+
   strs->strs[strs->cnt].data = str;
   strs->strs[strs->cnt].len  = len;
   strs->cnt++;

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}))