From a7fffb5c0f95bffb37ff1854841c3fcf4d1a5f9c Mon Sep 17 00:00:00 2001 From: Daniel Kempenich Date: Tue, 17 Jan 2023 23:02:56 -0600 Subject: [PATCH 1/2] Replace magic numbers with consts for evdns_base_resolv_conf_parse() errors --- evdns.c | 23 ++++++++++++----------- include/event2/dns.h | 25 ++++++++++++++++++++++--- test/regress_dns.c | 6 ++++-- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/evdns.c b/evdns.c index cd8e038a..52e7ee62 100644 --- a/evdns.c +++ b/evdns.c @@ -4471,12 +4471,13 @@ resolv_conf_parse_line(struct evdns_base *base, char *const start, int flags) { /* exported function */ /* returns: */ -/* 0 no errors */ -/* 1 failed to open file */ -/* 2 failed to stat file */ -/* 3 file too large */ -/* 4 out of memory */ -/* 5 short read from file */ +/* EVDNS_ERROR_NONE (0) no errors */ +/* EVDNS_ERROR_FAILED_TO_OPEN_FILE (1) failed to open file */ +/* EVDNS_ERROR_FAILED_TO_STAT_FILE (2) failed to stat file */ +/* EVDNS_ERROR_FILE_TOO_LARGE (3) file too large */ +/* EVDNS_ERROR_OUT_OF_MEMORY (4) out of memory */ +/* EVDNS_ERROR_SHORT_READ_FROM_FILE (5) short read from file */ +/* EVDNS_ERROR_NO_NAMESERVERS_CONFIGURED (6) no nameservers configured */ int evdns_base_resolv_conf_parse(struct evdns_base *base, int flags, const char *const filename) { int res; @@ -4516,7 +4517,7 @@ evdns_base_resolv_conf_parse_impl(struct evdns_base *base, int flags, const char size_t n; char *resolv; char *start; - int err = 0; + int err = EVDNS_ERROR_NONE; int add_default; log(EVDNS_LOG_DEBUG, "Parsing resolv.conf file %s", filename); @@ -4534,16 +4535,16 @@ evdns_base_resolv_conf_parse_impl(struct evdns_base *base, int flags, const char if (!filename) { evdns_resolv_set_defaults(base, flags); - return 1; + return EVDNS_ERROR_FAILED_TO_OPEN_FILE; } if ((err = evutil_read_file_(filename, &resolv, &n, 0)) < 0) { if (err == -1) { /* No file. */ evdns_resolv_set_defaults(base, flags); - return 1; + return EVDNS_ERROR_FAILED_TO_OPEN_FILE; } else { - return 2; + return EVDNS_ERROR_FAILED_TO_STAT_FILE; } } @@ -4563,7 +4564,7 @@ evdns_base_resolv_conf_parse_impl(struct evdns_base *base, int flags, const char if (!base->server_head && add_default) { /* no nameservers were configured. */ evdns_base_nameserver_ip_add(base, "127.0.0.1"); - err = 6; + err = EVDNS_ERROR_NO_NAMESERVERS_CONFIGURED; } if (flags & DNS_OPTION_SEARCH && (!base->global_search_state || base->global_search_state->num_domains == 0)) { search_set_from_hostname(base); diff --git a/include/event2/dns.h b/include/event2/dns.h index be61e783..17a00cff 100644 --- a/include/event2/dns.h +++ b/include/event2/dns.h @@ -261,6 +261,21 @@ struct event_base; * @see DNS_OPTION_NAMESERVERS_NO_DEFAULT */ #define EVDNS_BASE_NAMESERVERS_NO_DEFAULT 0x10000 +/* No errors */ +#define EVDNS_ERROR_NONE 0 +/* Failed to open file */ +#define EVDNS_ERROR_FAILED_TO_OPEN_FILE 1 +/* Failed to stat file */ +#define EVDNS_ERROR_FAILED_TO_STAT_FILE 2 +/* File too large */ +#define EVDNS_ERROR_FILE_TOO_LARGE 3 +/* Out of memory */ +#define EVDNS_ERROR_OUT_OF_MEMORY 4 +/* Short read from file */ +#define EVDNS_ERROR_SHORT_READ_FROM_FILE 5 +/* No nameservers configured */ +#define EVDNS_ERROR_NO_NAMESERVERS_CONFIGURED 6 + /** Initialize the asynchronous DNS library. @@ -511,9 +526,13 @@ int evdns_base_set_option(struct evdns_base *base, const char *option, const cha The following directives are not parsed from the file: sortlist, rotate, no-check-names, inet6, debug. - If this function encounters an error, the possible return values are: 1 = - failed to open file, 2 = failed to stat file, 3 = file too large, 4 = out of - memory, 5 = short read from file, 6 = no nameservers listed in the file + If this function encounters an error, the possible return values are: + EVDNS_ERROR_FAILED_TO_OPEN_FILE (1) - failed to open file + EVDNS_ERROR_FAILED_TO_STAT_FILE (2) - failed to stat file + EVDNS_ERROR_FILE_TOO_LARGE (3) - file too large + EVDNS_ERROR_OUT_OF_MEMORY (4) - out of memory + EVDNS_ERROR_SHORT_READ_FROM_FILE (5) - short read from file + EVDNS_ERROR_NO_NAMESERVERS_CONFIGURED (6) - no nameservers configured. @param base the evdns_base to which to apply this operation @param flags any of DNS_OPTION_NAMESERVERS|DNS_OPTION_SEARCH|DNS_OPTION_MISC| diff --git a/test/regress_dns.c b/test/regress_dns.c index 2f251c72..4e247b34 100644 --- a/test/regress_dns.c +++ b/test/regress_dns.c @@ -1203,12 +1203,14 @@ dns_nameservers_no_default_test(void *arg) * EVDNS_BASE_INITIALIZE_NAMESERVERS|EVDNS_BASE_NAMESERVERS_NO_DEFAULT * because we cannot mock "/etc/resolv.conf" (yet). */ - evdns_base_resolv_conf_parse(dns, + ok = evdns_base_resolv_conf_parse(dns, DNS_OPTIONS_ALL|DNS_OPTION_NAMESERVERS_NO_DEFAULT, RESOLV_FILE); + tt_int_op(ok, ==, EVDNS_ERROR_FAILED_TO_OPEN_FILE); tt_int_op(evdns_base_get_nameserver_addr(dns, 0, NULL, 0), ==, -1); tt_int_op(evdns_base_get_nameserver_fd(dns, 0), ==, -1); - evdns_base_resolv_conf_parse(dns, DNS_OPTIONS_ALL, RESOLV_FILE); + ok = evdns_base_resolv_conf_parse(dns, DNS_OPTIONS_ALL, RESOLV_FILE); + tt_int_op(ok, ==, EVDNS_ERROR_FAILED_TO_OPEN_FILE); tt_int_op(evdns_base_get_nameserver_addr(dns, 0, NULL, 0), ==, sizeof(struct sockaddr)); tt_int_op(evdns_base_get_nameserver_fd(dns, 0), !=, -1); From ebd7e8d7930df0ef9f11a23309c59cded735e7c4 Mon Sep 17 00:00:00 2001 From: Daniel Kempenich Date: Fri, 27 Jan 2023 08:44:41 +0100 Subject: [PATCH 2/2] Allow evdns_base_new to succeed with no nameservers configured If resolv.conf has no nameservers, evdns_base_new can still succeed with the default of using the name server from localhost matching the man page documentation for resolv.conf. --- evdns.c | 2 +- test/regress_dns.c | 38 ++++++++++++++++++++++++++++++++++++++ test/regress_main.c | 4 ++-- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/evdns.c b/evdns.c index 52e7ee62..05e515c0 100644 --- a/evdns.c +++ b/evdns.c @@ -4892,7 +4892,7 @@ evdns_base_new(struct event_base *event_base, int flags) #else r = evdns_base_resolv_conf_parse(base, opts, "/etc/resolv.conf"); #endif - if (r) { + if (r && (EVDNS_ERROR_NO_NAMESERVERS_CONFIGURED != r)) { evdns_base_free_and_unlock(base, 0); return NULL; } diff --git a/test/regress_dns.c b/test/regress_dns.c index 4e247b34..e28ad52b 100644 --- a/test/regress_dns.c +++ b/test/regress_dns.c @@ -42,6 +42,9 @@ #include #ifndef _WIN32 #include +#include +#include +#include #include #include #include @@ -1218,6 +1221,39 @@ end: if (dns) evdns_base_free(dns, 0); } + +static void +dns_nameservers_no_nameservers_configured_test(void *arg) +{ + struct basic_test_data *data = arg; + struct event_base *base = data->base; + struct evdns_base *dns = NULL; + int fd = -1; + char *tmpfilename = NULL; + const char filecontents[] = "# tmp empty resolv.conf\n"; + const size_t filecontentssize = sizeof(filecontents); + int ok; + + fd = regress_make_tmpfile(filecontents, filecontentssize, &tmpfilename); + if (fd < 0) + tt_skip(); + + dns = evdns_base_new(base, 0); + tt_assert(dns); + + ok = evdns_base_resolv_conf_parse(dns, DNS_OPTIONS_ALL, tmpfilename); + tt_int_op(ok, ==, EVDNS_ERROR_NO_NAMESERVERS_CONFIGURED); + +end: + if (fd != -1) + close(fd); + if (dns) + evdns_base_free(dns, 0); + if (tmpfilename) { + unlink(tmpfilename); + free(tmpfilename); + } +} #endif /* === Test for bufferevent_socket_connect_hostname */ @@ -3007,6 +3043,8 @@ struct testcase_t dns_testcases[] = { #ifndef _WIN32 { "nameservers_no_default", dns_nameservers_no_default_test, TT_FORK|TT_NEED_BASE, &basic_setup, NULL }, + { "no_nameservers_configured", dns_nameservers_no_nameservers_configured_test, + TT_FORK|TT_NEED_BASE, &basic_setup, NULL }, #endif { "getaddrinfo_async", test_getaddrinfo_async, diff --git a/test/regress_main.c b/test/regress_main.c index 489c74b7..a504d6ce 100644 --- a/test/regress_main.c +++ b/test/regress_main.c @@ -144,11 +144,11 @@ regress_make_tmpfile(const void *data, size_t datalen, char **filename_out) return (-1); if (write(fd, data, datalen) != (int)datalen) { close(fd); + unlink(tmpfilename); return (-1); } lseek(fd, 0, SEEK_SET); - /* remove it from the file system */ - unlink(tmpfilename); + *filename_out = strdup(tmpfilename); return (fd); #else /* XXXX actually delete the file later */