BUG/MINOR: resolvers: reject a null or negative resolve_retries value

The resolve_retries value is parsed with atoi() and never checked. When
a resolution is started, its try counter is initialized to this value and
immediately decremented for the first query, and the resolution is only
considered as failed once the counter reaches zero on a timeout. With a
value of zero (or a negative one, or a non-numeric one which atoi() turns
into zero), the counter starts negative and never reaches zero, so the
resolution never fails: requesters are never notified of timeouts or
errors (servers keep their stale address forever), and queries are sent
every "timeout retry" forever instead of waiting "timeout resolve"
between resolutions, which with a small "timeout retry" floods the
nameserver.

This can be reproduced with "resolve_retries 0", "timeout retry 200ms",
"timeout resolve 1s" and a nameserver which never responds: 20 queries
are sent in 4s, versus 4 with "resolve_retries 1".

Let's reject values lower than 1 at parsing time, since a resolution
always needs at least one query anyway. Configurations with such values
were not working as expected and will now be rejected with an explicit
error.

This patch should be backported to all stable branches.

diff --git a/src/resolvers.c b/src/resolvers.c
index 910267bbcf429ad889fa8ea238ec9ed043ae39a0..a1cb92c5c1c27884c022bac34ece89fc8d5018ca 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -4118,6 +4118,12 @@ int cfg_parse_resolvers(const char *file, int linenum, char **args, int kwm)
                        goto out;
                }
                curr_resolvers->resolve_retries = atoi(args[1]);
+               if (curr_resolvers->resolve_retries < 1) {
+                       ha_alert("parsing [%s:%d] : '%s' expects a strictly positive integer as argument (was '%s').\n",
+                                file, linenum, args[0], args[1]);
+                       err_code |= ERR_ALERT | ERR_FATAL;
+                       goto out;
+               }
        }
        else if (strcmp(args[0], "timeout") == 0) {
                if (!*args[1]) {