hostip: cache negative name resolves

Hold them for half the normal lifetime. Helps when told to transfer N
URLs in quick succession that all use the same non-resolving hostname.

Done by storing a DNS entry with a NULL pointer for 'addr'.

Previously an attempt was made in #12406 by Björn Stenberg that was
ultimately never merged.

Closes #18157

docs/TODO

@@ -24,7 +24,6 @@
  1.5 get rid of PATH_MAX
  1.6 thread-safe sharing
  1.8 CURLOPT_RESOLVE for any port number
- 1.9 Cache negative name resolves
  1.10 auto-detect proxy
  1.11 minimize dependencies with dynamically loaded modules
  1.12 updated DNS server while running
@@ -263,11 +262,6 @@
 
  See https://github.com/curl/curl/issues/1264
 
-1.9 Cache negative name resolves
-
- A name resolve that has failed is likely to fail when made again within a
- short period of time. Currently we only cache positive responses.
-
 1.10 auto-detect proxy
 
  libcurl could be made to detect the system proxy setup automatically and use

docs/libcurl/opts/CURLOPT_DNS_CACHE_TIMEOUT.md

@@ -10,6 +10,7 @@ See-also:
   - CURLOPT_DNS_USE_GLOBAL_CACHE (3)
   - CURLOPT_MAXAGE_CONN (3)
   - CURLOPT_RESOLVE (3)
+  - CURLMOPT_NETWORK_CHANGED (3)
 Protocol:
   - All
 Added-in: 7.9.3
@@ -48,8 +49,11 @@ DNS entries have a "TTL" property but libcurl does not use that. This DNS
 cache timeout is entirely speculative that a name resolves to the same address
 for a small amount of time into the future.
 
-Since version 8.1.0, libcurl prunes entries from the DNS cache if it exceeds
-30,000 entries no matter which timeout value is used.
+libcurl prunes entries from the DNS cache if it exceeds 30,000 entries no
+matter which timeout value is used. (Added in version 8.1.0)
+
+Since curl 8.16.0, failed name resolves are stored in the DNS cache for half
+the set timeout period.
 
 # DEFAULT
 

docs/tests/FILEFORMAT.md

@@ -747,3 +747,13 @@ should be cut off from the upload data before comparing it.
 
 ### `<valgrind>`
 disable - disables the valgrind log check for this test
+
+### `<dns [host="name"]>`
+
+This specify the input the DNS server is expected to get from curl. Because of
+differences in implementations, this section is sorted automatically before
+compared.
+
+Because of local configurations in machines running tests, there may be
+additional requests sent to `[host].[custom suffix]`. To prevent such requests
+to mess up comparisons, we can set the hostname to check in the `<dns>` tag.

lib/asyn-thrdd.c

@@ -368,6 +368,14 @@ static CURLcode async_rr_start(struct Curl_easy *data)
     thrdd->rr.channel = NULL;
     return CURLE_FAILED_INIT;
   }
+#ifdef CURLDEBUG
+  if(getenv("CURL_DNS_SERVER")) {
+    const char *servers = getenv("CURL_DNS_SERVER");
+    status = ares_set_servers_ports_csv(thrdd->rr.channel, servers);
+    if(status)
+      return CURLE_FAILED_INIT;
+  }
+#endif
 
   memset(&thrdd->rr.hinfo, 0, sizeof(thrdd->rr.hinfo));
   thrdd->rr.hinfo.port = -1;
@@ -375,6 +383,7 @@ static CURLcode async_rr_start(struct Curl_easy *data)
                     data->conn->host.name, ARES_CLASS_IN,
                     ARES_REC_TYPE_HTTPS,
                     async_thrdd_rr_done, data, NULL);
+  CURL_TRC_DNS(data, "Issued HTTPS-RR request for %s", data->conn->host.name);
   return CURLE_OK;
 }
 #endif

lib/hostip.c

@@ -202,6 +202,8 @@ dnscache_entry_is_stale(void *datap, void *hc)
   if(dns->timestamp.tv_sec || dns->timestamp.tv_usec) {
     /* get age in milliseconds */
     timediff_t age = curlx_timediff(prune->now, dns->timestamp);
+    if(!dns->addr)
+      age *= 2; /* negative entries age twice as fast */
     if(age >= prune->max_age_ms)
       return TRUE;
     if(age > prune->oldest_ms)
@@ -798,6 +800,28 @@ static bool can_resolve_ip_version(struct Curl_easy *data, int ip_version)
   return TRUE;
 }
 
+static CURLcode store_negative_resolve(struct Curl_easy *data,
+                                       const char *host,
+                                       int port)
+{
+  struct Curl_dnscache *dnscache = dnscache_get(data);
+  struct Curl_dns_entry *dns;
+  DEBUGASSERT(dnscache);
+  if(!dnscache)
+    return CURLE_FAILED_INIT;
+
+  /* put this new host in the cache */
+  dns = dnscache_add_addr(data, dnscache, NULL, host, 0, port, FALSE);
+  if(dns) {
+    /* release the returned reference; the cache itself will keep the
+     * entry alive: */
+    dns->refcount--;
+    infof(data, "Store negative name resolve for %s:%d", host, port);
+    return CURLE_OK;
+  }
+  return CURLE_OUT_OF_MEMORY;
+}
+
 /*
  * Curl_resolv() is the main name resolve function within libcurl. It resolves
  * a name and returns a pointer to the entry in the 'entry' argument (if one
@@ -917,6 +941,11 @@ out:
    * or `respwait` is set for an async operation.
    * Everything else is a failure to resolve. */
   if(dns) {
+    if(!dns->addr) {
+      infof(data, "Negative DNS entry");
+      dns->refcount--;
+      return CURLE_COULDNT_RESOLVE_HOST;
+    }
     *entry = dns;
     return CURLE_OK;
   }
@@ -942,6 +971,7 @@ error:
     Curl_resolv_unlink(data, &dns);
   *entry = NULL;
   Curl_async_shutdown(data);
+  store_negative_resolve(data, hostname, port);
   return CURLE_COULDNT_RESOLVE_HOST;
 }
 
@@ -1523,6 +1553,9 @@ CURLcode Curl_resolv_check(struct Curl_easy *data,
   result = Curl_async_is_resolved(data, dns);
   if(*dns)
     show_resolve_info(data, *dns);
+  if(result)
+    store_negative_resolve(data, data->state.async.hostname,
+                           data->state.async.port);
   return result;
 }
 #endif

tests/data/Makefile.am

@@ -252,7 +252,7 @@ test2064 test2065 test2066 test2067 test2068 test2069 test2070 test2071 \
 test2072 test2073 test2074 test2075 test2076 test2077 test2078 test2079 \
 test2080 test2081 test2082 test2083 test2084 test2085 test2086 test2087 \
 test2088 test2089 \
-test2100 test2101 test2102 test2103 \
+test2100 test2101 test2102 test2103 test2104 \
 \
 test2200 test2201 test2202 test2203 test2204 test2205 \
 \

tests/data/test2104

@@ -0,0 +1,49 @@
+<testcase>
+<info>
+<keywords>
+DNS cache
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+<dns>
+</dns>
+</reply>
+
+#
+# Client-side
+<client>
+<server>
+dns
+</server>
+<features>
+override-dns
+</features>
+<name>
+Get three URLs with bad host name - cache
+</name>
+<setenv>
+CURL_DNS_SERVER=127.0.0.1:%DNSPORT
+</setenv>
+<command>
+http://examplehost.example/%TESTNUMBER http://examplehost.example/%TESTNUMBER http://examplehost.example/%TESTNUMBER
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+# curl: (6) Could not resolve host: examplehost.example
+<errorcode>
+6
+</errorcode>
+
+# Ignore HTTPS requests here
+<dns host="examplehost.example QTYPE A">
+QNAME examplehost.example QTYPE A
+QNAME examplehost.example QTYPE AAAA
+</dns>
+</verify>
+</testcase>

tests/data/test655

@@ -36,7 +36,7 @@ lib%TESTNUMBER
 resolver start callback
 </name>
 <command>
-http://%HOSTIP:%HTTPPORT/%TESTNUMBER
+http://failthis/%TESTNUMBER http://%HOSTIP:%HTTPPORT/%TESTNUMBER
 </command>
 </client>
 

tests/libtest/lib655.c

@@ -74,7 +74,7 @@ static CURLcode test_lib655(const char *URL)
     goto test_cleanup;
   }
 
-  /* First set the URL that is about to receive our request. */
+  /* Set the URL that is about to receive our first request. */
   test_setopt(curl, CURLOPT_URL, URL);
 
   test_setopt(curl, CURLOPT_RESOLVER_START_DATA, TEST_DATA_STRING);
@@ -91,6 +91,9 @@ static CURLcode test_lib655(const char *URL)
     goto test_cleanup;
   }
 
+  /* Set the URL that receives our second request. */
+  test_setopt(curl, CURLOPT_URL, libtest_arg2);
+
   test_setopt(curl, CURLOPT_RESOLVER_START_FUNCTION, resolver_alloc_cb_pass);
 
   /* this should succeed */

tests/runtests.pl

@@ -1674,6 +1674,29 @@ sub singletest_check {
         }
     }
 
+    my @dnsd = getpart("verify", "dns");
+    if(@dnsd) {
+        # we're supposed to verify a dynamically generated file!
+        my %hash = getpartattr("verify", "dns");
+        my $hostname=$hash{'host'};
+
+        # Verify the sent DNS requests
+        my @out = loadarray("$logdir/dnsd.input");
+        my @sverify = sort @dnsd;
+        my @sout = sort @out;
+
+        if($hostname) {
+            # when a hostname is set, we filter out requests to just this
+            # pattern
+            @sout = grep {/$hostname/} @sout;
+        }
+
+        $res = compare($runnerid, $testnum, $testname, "DNS", \@sout, \@sverify);
+        if($res) {
+            return -1;
+        }
+    }
+
     # accept multiple comma-separated error codes
     my @splerr = split(/ *, */, $errorcode);
     my $errok;

tests/server/dnsd.c

@@ -64,6 +64,19 @@ static int qname(const unsigned char **pkt, size_t *size)
 #define QTYPE_AAAA 28
 #define QTYPE_HTTPS 0x41
 
+static const char *type2string(unsigned short qtype)
+{
+  switch(qtype) {
+  case QTYPE_A:
+    return "A";
+  case QTYPE_AAAA:
+    return "AAAA";
+  case QTYPE_HTTPS:
+    return "HTTPS";
+  }
+  return "<unknown>";
+}
+
 /*
  * Handle initial connection protocol.
  *
@@ -125,8 +138,7 @@ static int store_incoming(const unsigned char *data, size_t size,
   fprintf(server, "Z: %x\n", (id & 0x70) >> 4);
   fprintf(server, "RCODE: %x\n", (id & 0x0f));
 #endif
-  qd = get16bit(&data, &size);
-  fprintf(server, "QDCOUNT: %04x\n", qd);
+  (void) get16bit(&data, &size);
 
   data += 6; /* skip ANCOUNT, NSCOUNT and ARCOUNT */
   size -= 6;
@@ -136,14 +148,13 @@ static int store_incoming(const unsigned char *data, size_t size,
   qptr = data;
 
   if(!qname(&data, &size)) {
-    fprintf(server, "QNAME: %s\n", name);
     qd = get16bit(&data, &size);
-    fprintf(server, "QTYPE: %04x\n", qd);
+    fprintf(server, "QNAME %s QTYPE %s\n", name, type2string(qd));
     *qtype = qd;
-    logmsg("Question for '%s' type %x", name, qd);
+    logmsg("Question for '%s' type %x / %s", name, qd,
+           type2string(qd));
 
-    qd = get16bit(&data, &size);
-    logmsg("QCLASS: %04x\n", qd);
+    (void) get16bit(&data, &size);
 
     *qlen = qsize - size; /* total size of the query */
     memcpy(qbuf, qptr, *qlen);
@@ -618,7 +629,7 @@ static int test_dnsd(int argc, char **argv)
       clear_advisor_read_lock(loglockfile);
     }
 
-    logmsg("end of one transfer");
+    /* logmsg("end of one transfer"); */
   }
 
 dnsd_cleanup: