From d2b36e466afd69b6e2b202aca21db3bd2e48b190 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 8 Aug 2013 13:01:16 +0200 Subject: [PATCH] global dns cache: didn't work [regression] CURLOPT_DNS_USE_GLOBAL_CACHE broke in commit c43127414d89ccb (been broken since the libcurl 7.29.0 release). While this option has been documented as deprecated for almost a decade and nobody even reported this bug, it should remain functional. Added test case 1512 to verify --- lib/hostip.c | 27 +++++++----- lib/hostip.h | 4 +- lib/multi.c | 56 ++++++++++++++---------- tests/data/Makefile.am | 2 +- tests/data/test1512 | 80 ++++++++++++++++++++++++++++++++++ tests/libtest/Makefile.inc | 6 ++- tests/libtest/lib1512.c | 89 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 224 insertions(+), 40 deletions(-) create mode 100644 tests/data/test1512 create mode 100644 tests/libtest/lib1512.c diff --git a/lib/hostip.c b/lib/hostip.c index 3765ca1cd..c137a19c3 100644 --- a/lib/hostip.c +++ b/lib/hostip.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2012, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2013, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -681,12 +681,14 @@ clean_up: * Curl_resolv_unlock() unlocks the given cached DNS entry. When this has been * made, the struct may be destroyed due to pruning. It is important that only * one unlock is made for each Curl_resolv() call. + * + * May be called with 'data' == NULL for global cache. */ void Curl_resolv_unlock(struct SessionHandle *data, struct Curl_dns_entry *dns) { DEBUGASSERT(dns && (dns->inuse>0)); - if(data->share) + if(data && data->share) Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); dns->inuse--; @@ -697,7 +699,7 @@ void Curl_resolv_unlock(struct SessionHandle *data, struct Curl_dns_entry *dns) free(dns); } - if(data->share) + if(data && data->share) Curl_share_unlock(data, CURL_LOCK_DATA_DNS); } @@ -734,22 +736,23 @@ static int hostcache_inuse(void *data, void *hc) return 1; /* free all entries */ } -void Curl_hostcache_clean(struct SessionHandle *data) +/* + * Curl_hostcache_clean() + * + * This _can_ be called with 'data' == NULL but then of course no locking + * can be done! + */ + +void Curl_hostcache_clean(struct SessionHandle *data, + struct curl_hash *hash) { /* Entries added to the hostcache with the CURLOPT_RESOLVE function are * still present in the cache with the inuse counter set to 1. Detect them * and cleanup! */ - Curl_hash_clean_with_criterium(data->dns.hostcache, data, hostcache_inuse); + Curl_hash_clean_with_criterium(hash, data, hostcache_inuse); } -void Curl_hostcache_destroy(struct SessionHandle *data) -{ - Curl_hostcache_clean(data); - Curl_hash_destroy(data->dns.hostcache); - data->dns.hostcachetype = HCACHE_NONE; - data->dns.hostcache = NULL; -} CURLcode Curl_loadhostpairs(struct SessionHandle *data) { diff --git a/lib/hostip.h b/lib/hostip.h index 9dd47baa6..a38f732a4 100644 --- a/lib/hostip.h +++ b/lib/hostip.h @@ -7,7 +7,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2012, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2013, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -203,7 +203,7 @@ CURLcode Curl_set_dns_servers(struct SessionHandle *data, char *servers); /* * Clean off entries from the cache */ -void Curl_hostcache_clean(struct SessionHandle *data); +void Curl_hostcache_clean(struct SessionHandle *data, struct curl_hash *hash); /* * Destroy the hostcache of this handle. diff --git a/lib/multi.c b/lib/multi.c index 9653554f8..b64092a5d 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -334,7 +334,6 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle, CURL *easy_handle) { struct curl_llist *timeoutlist; - struct SessionHandle *easy; struct Curl_multi *multi = (struct Curl_multi *)multi_handle; struct SessionHandle *data = (struct SessionHandle *)easy_handle; struct SessionHandle *new_closure = NULL; @@ -348,8 +347,8 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle, if(!GOOD_EASY_HANDLE(easy_handle)) return CURLM_BAD_EASY_HANDLE; - /* Prevent users from adding same easy handle more than - once and prevent adding to more than one multi stack */ + /* Prevent users from adding same easy handle more than once and prevent + adding to more than one multi stack */ if(data->multi) /* possibly we should create a new unique error code for this condition */ return CURLM_BAD_EASY_HANDLE; @@ -359,13 +358,11 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle, if(!timeoutlist) return CURLM_OUT_OF_MEMORY; - easy = data; - /* In case multi handle has no hostcache yet, allocate one */ if(!multi->hostcache) { hostcache = Curl_mk_dnscache(); if(!hostcache) { - free(easy); + free(data); Curl_llist_destroy(timeoutlist, NULL); return CURLM_OUT_OF_MEMORY; } @@ -377,7 +374,7 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle, new_closure = (struct SessionHandle *)curl_easy_init(); if(!new_closure) { Curl_hash_destroy(hostcache); - free(easy); + free(data); Curl_llist_destroy(timeoutlist, NULL); return CURLM_OUT_OF_MEMORY; } @@ -408,18 +405,28 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle, timeoutlist = NULL; /* set the easy handle */ - multistate(easy, CURLM_STATE_INIT); + multistate(data, CURLM_STATE_INIT); + if((data->set.global_dns_cache) && + (data->dns.hostcachetype != HCACHE_GLOBAL)) { + /* global dns cache was requested but still isn't */ + struct curl_hash *global = Curl_global_host_cache_init(); + if(global) { + /* only do this if the global cache init works */ + data->dns.hostcache = global; + data->dns.hostcachetype = HCACHE_GLOBAL; + } + } /* for multi interface connections, we share DNS cache automatically if the easy handle's one is currently not set. */ - if(!easy->dns.hostcache || - (easy->dns.hostcachetype == HCACHE_NONE)) { - easy->dns.hostcache = multi->hostcache; - easy->dns.hostcachetype = HCACHE_MULTI; + else if(!data->dns.hostcache || + (data->dns.hostcachetype == HCACHE_NONE)) { + data->dns.hostcache = multi->hostcache; + data->dns.hostcachetype = HCACHE_MULTI; } /* Point to the multi's connection cache */ - easy->state.conn_cache = multi->conn_cache; + data->state.conn_cache = multi->conn_cache; /* This adds the new entry at the 'end' of the doubly-linked circular list of SessionHandle structs to try and maintain a FIFO queue so @@ -427,22 +434,22 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle, /* We add this new entry last in the list. */ - easy->next = NULL; /* end of the line */ + data->next = NULL; /* end of the line */ if(multi->easyp) { struct SessionHandle *last = multi->easylp; - last->next = easy; - easy->prev = last; - multi->easylp = easy; /* the new last node */ + last->next = data; + data->prev = last; + multi->easylp = data; /* the new last node */ } else { /* first node, make both prev and next be NULL! */ - easy->next = NULL; - easy->prev = NULL; - multi->easylp = multi->easyp = easy; /* both first and last */ + data->next = NULL; + data->prev = NULL; + multi->easylp = multi->easyp = data; /* both first and last */ } /* make the SessionHandle refer back to this multi handle */ - Curl_easy_addmulti(easy_handle, multi_handle); + Curl_easy_addmulti(data, multi_handle); /* Set the timeout for this handle to expire really soon so that it will be taken care of even when this handle is added in the midst of operation @@ -450,7 +457,7 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle, sockets that time-out or have actions will be dealt with. Since this handle has no action yet, we make sure it times out to get things to happen. */ - Curl_expire(easy, 1); + Curl_expire(data, 1); /* increase the node-counter */ multi->num_easy++; @@ -1811,7 +1818,8 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle) if(multi->closure_handle) { multi->closure_handle->dns.hostcache = multi->hostcache; - Curl_hostcache_clean(multi->closure_handle); + Curl_hostcache_clean(multi->closure_handle, + multi->closure_handle->dns.hostcache); Curl_close(multi->closure_handle); multi->closure_handle = NULL; @@ -1833,7 +1841,7 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle) nexteasy=easy->next; if(easy->dns.hostcachetype == HCACHE_MULTI) { /* clear out the usage of the shared DNS cache */ - Curl_hostcache_clean(easy); + Curl_hostcache_clean(easy, easy->dns.hostcache); easy->dns.hostcache = NULL; easy->dns.hostcachetype = HCACHE_NONE; } diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index eaaf7eca2..0d5c29d88 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -113,7 +113,7 @@ test1400 test1401 test1402 test1403 test1404 test1405 test1406 test1407 \ test1408 test1409 test1410 test1412 test1413 test1414 \ \ test1500 test1501 test1502 test1503 test1504 test1505 test1506 test1507 \ -test1508 test1509 test1510 test1511 \ +test1508 test1509 test1510 test1511 test1512 \ \ test1900 test1901 test1902 test1903 \ \ diff --git a/tests/data/test1512 b/tests/data/test1512 new file mode 100644 index 000000000..9d2134b2a --- /dev/null +++ b/tests/data/test1512 @@ -0,0 +1,80 @@ + + + +HTTP +GLOBAL DNS CACHE + + + +# Server-side + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 47 + +file contents should appear once for each file + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 47 + +file contents should appear once for each file + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 47 + +file contents should appear once for each file + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 47 + +file contents should appear once for each file + + + +# Client-side + + +http + + +lib1512 + + +GLOBAL CACHE test over two easy performs + + +http://%HOSTIP:%HTTPPORT/path/1512 %HOSTIP %HTTPPORT + + + +# Verify data after the test has been "shot" + + +GET /path/15120001 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + +GET /path/15120002 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + + + +^Host:.* + + +$_ = '' if (($_ !~ /left intact/) && ($_ !~ /Closing connection/)) + + + diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc index 664e00063..cf6210591 100644 --- a/tests/libtest/Makefile.inc +++ b/tests/libtest/Makefile.inc @@ -21,7 +21,7 @@ noinst_PROGRAMS = chkhostname libauthretry libntlmconnect \ lib571 lib572 lib573 lib574 lib575 lib576 lib578 lib579 lib582 \ lib583 lib585 lib586 lib587 lib590 lib591 lib597 lib598 lib599 \ lib1500 lib1501 lib1502 lib1503 lib1504 lib1505 lib1506 lib1507 lib1508 \ - lib1509 lib1510 lib1511 \ + lib1509 lib1510 lib1511 lib1512 \ lib1900 \ lib2033 @@ -339,6 +339,10 @@ lib1511_SOURCES = lib1511.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) lib1511_LDADD = $(TESTUTIL_LIBS) lib1511_CPPFLAGS = $(AM_CPPFLAGS) -DLIB1511 +lib1512_SOURCES = lib1512.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) +lib1512_LDADD = $(TESTUTIL_LIBS) +lib1512_CPPFLAGS = $(AM_CPPFLAGS) -DLIB1512 + lib1900_SOURCES = lib1900.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) lib1900_LDADD = $(TESTUTIL_LIBS) lib1900_CPPFLAGS = $(AM_CPPFLAGS) diff --git a/tests/libtest/lib1512.c b/tests/libtest/lib1512.c new file mode 100644 index 000000000..93d5ea908 --- /dev/null +++ b/tests/libtest/lib1512.c @@ -0,0 +1,89 @@ +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * + * Copyright (C) 2013, Linus Nielsen Feltzing + * + * This software is licensed as described in the file COPYING, which + * you should have received as part of this distribution. The terms + * are also available at http://curl.haxx.se/docs/copyright.html. + * + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. + * + ***************************************************************************/ + +/* + * Use global DNS cache (while deprecated it should still work), populate it + * with CURLOPT_RESOLVE in the first request and then make sure a subsequent + * easy transfer finds and uses the populated stuff. + */ + +#include "test.h" + +#include "memdebug.h" + +#define NUM_HANDLES 2 + +int test(char *URL) +{ + CURLcode res; + CURL *curl[NUM_HANDLES] = {NULL, NULL}; + char *port = libtest_arg3; + char *address = libtest_arg2; + char dnsentry[256]; + struct curl_slist *slist = NULL; + int i; + char target_url[256]; + (void)URL; /* URL is setup in the code */ + + if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) { + fprintf(stderr, "curl_global_init() failed\n"); + return TEST_ERR_MAJOR_BAD; + } + + sprintf(dnsentry, "server.example.curl:%s:%s", port, address); + printf("%s\n", dnsentry); + slist = curl_slist_append(slist, dnsentry); + + /* get NUM_HANDLES easy handles */ + for(i=0; i < NUM_HANDLES; i++) { + /* get an easy handle */ + easy_init(curl[i]); + /* specify target */ + sprintf(target_url, "http://server.example.curl:%s/path/1512%04i", + port, i + 1); + target_url[sizeof(target_url) - 1] = '\0'; + easy_setopt(curl[i], CURLOPT_URL, target_url); + /* go verbose */ + easy_setopt(curl[i], CURLOPT_VERBOSE, 1L); + /* include headers */ + easy_setopt(curl[i], CURLOPT_HEADER, 1L); + + easy_setopt(curl[i], CURLOPT_DNS_USE_GLOBAL_CACHE, 1L); + } + + /* make the first one populate the GLOBAL cache */ + easy_setopt(curl[0], CURLOPT_RESOLVE, slist); + + /* run NUM_HANDLES transfers */ + for(i=0; (i < NUM_HANDLES) && !res; i++) + res = curl_easy_perform(curl[i]); + +test_cleanup: + + curl_easy_cleanup(curl[0]); + curl_easy_cleanup(curl[1]); + curl_slist_free_all(slist); + curl_global_cleanup(); + + return (int)res; +} +