MEDIUM: config: warn on unitless timeouts < 100 ms

From time to time we face a configuration with very small timeouts which
look accidental because there could be expectations that they're expressed
in seconds and not milliseconds.

This commit adds a check for non-nul unitless values smaller than 100
and emits a warning suggesting to append an explicit unit if that was
the intent.

Only the common timeouts, the server check intervals and the resolvers
hold and timeout values were covered for now. All the code needs to be
manually reviewed to verify if it supports emitting warnings.

This may break some configs using "zero-warning", but greps in existing
configs indicate that these are extremely rare and solely intentionally
done during tests. At least even if a user leaves that after a test, it
will be more obvious when reading 10ms that something's probably not
correct.

diff --git a/include/haproxy/tools.h b/include/haproxy/tools.h
index 8e29d38..aa9fd77 100644 (file)
--- a/include/haproxy/tools.h
+++ b/include/haproxy/tools.h
@@ -1256,6 +1256,23 @@ static inline void update_char_fingerprint(uint8_t *fp, char prev, char curr)
        fp[32 * from + to]++;
 }
 
+/* checks that the numerical argument, if passed without units and is non-zero,
+ * is at least as large as value <min>. It returns 1 if the value is too small,
+ * otherwise zero. This is used to warn about the use of small values without
+ * units.
+ */
+static inline int warn_if_lower(const char *text, long min)
+{
+       int digits;
+       long value;
+
+       digits = strspn(text, "0123456789");
+       if (digits < strlen(text))
+               return 0; // there are non-digits here.
+
+       value = atol(text);
+       return value && value < min;
+}
 
 /* compare the current OpenSSL version to a string */
 int openssl_compare_current_version(const char *version);

diff --git a/src/check.c b/src/check.c
index 85691de..29a86b7 100644 (file)
--- a/src/check.c
+++ b/src/check.c
@@ -2190,6 +2190,12 @@ static int srv_parse_agent_inter(char **args, int *cur_arg, struct proxy *curpx,
        }
        srv->agent.inter = delay;
 
+       if (warn_if_lower(args[*cur_arg+1], 100)) {
+               memprintf(errmsg, "'%s %u' in server '%s' is suspiciously small for a value in milliseconds. Please use an explicit unit ('%ums') if that was the intent",
+                         args[*cur_arg], delay, srv->id, delay);
+               err_code |= ERR_WARN;
+       }
+
   out:
        return err_code;
 
@@ -2459,6 +2465,12 @@ static int srv_parse_check_inter(char **args, int *cur_arg, struct proxy *curpx,
        }
        srv->check.inter = delay;
 
+       if (warn_if_lower(args[*cur_arg+1], 100)) {
+               memprintf(errmsg, "'%s %u' in server '%s' is suspiciously small for a value in milliseconds. Please use an explicit unit ('%ums') if that was the intent",
+                         args[*cur_arg], delay, srv->id, delay);
+               err_code |= ERR_WARN;
+       }
+
   out:
        return err_code;
 
@@ -2504,6 +2516,12 @@ static int srv_parse_check_fastinter(char **args, int *cur_arg, struct proxy *cu
        }
        srv->check.fastinter = delay;
 
+       if (warn_if_lower(args[*cur_arg+1], 100)) {
+               memprintf(errmsg, "'%s %u' in server '%s' is suspiciously small for a value in milliseconds. Please use an explicit unit ('%ums') if that was the intent",
+                         args[*cur_arg], delay, srv->id, delay);
+               err_code |= ERR_WARN;
+       }
+
   out:
        return err_code;
 
@@ -2549,6 +2567,12 @@ static int srv_parse_check_downinter(char **args, int *cur_arg, struct proxy *cu
        }
        srv->check.downinter = delay;
 
+       if (warn_if_lower(args[*cur_arg+1], 100)) {
+               memprintf(errmsg, "'%s %u' in server '%s' is suspiciously small for a value in milliseconds. Please use an explicit unit ('%ums') if that was the intent",
+                         args[*cur_arg], delay, srv->id, delay);
+               err_code |= ERR_WARN;
+       }
+
   out:
        return err_code;
 

diff --git a/src/proxy.c b/src/proxy.c
index 6f951cd..3577042 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -620,6 +620,12 @@ static int proxy_parse_timeout(char **args, int section, struct proxy *proxy,
                return -1;
        }
 
+       if (warn_if_lower(args[1], 100)) {
+               memprintf(err, "'timeout %s %u' in %s '%s' is suspiciously small for a value in milliseconds. Please use an explicit unit ('%ums') if that was the intent.",
+                         name, timeout, proxy_type_str(proxy), proxy->id, timeout);
+               retval = 1;
+       }
+
        if (!(proxy->cap & cap)) {
                memprintf(err, "'timeout %s' will be ignored because %s '%s' has no %s capability",
                          name, proxy_type_str(proxy), proxy->id,

diff --git a/src/resolvers.c b/src/resolvers.c
index 8874689..f8f0c8e 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -3733,6 +3733,12 @@ int cfg_parse_resolvers(const char *file, int linenum, char **args, int kwm)
                        goto out;
                }
 
+               if (warn_if_lower(args[2], 100)) {
+                       ha_alert("parsing [%s:%d] : '%s %s %u' looks suspiciously small for a value in milliseconds."
+                                " Please use an explicit unit ('%ums') if that was the intent.\n",
+                                file, linenum, args[0], args[1], time, time);
+                       err_code |= ERR_WARN;
+               }
        }
        else if (strcmp(args[0], "accepted_payload_size") == 0) {
                int i = 0;
@@ -3813,6 +3819,12 @@ int cfg_parse_resolvers(const char *file, int linenum, char **args, int kwm)
                                curr_resolvers->px->timeout.connect = tout;
                        }
 
+                       if (warn_if_lower(args[2], 100)) {
+                               ha_alert("parsing [%s:%d] : '%s %s %u' looks suspiciously small for a value in milliseconds."
+                                        " Please use an explicit unit ('%ums') if that was the intent.\n",
+                                        file, linenum, args[0], args[1], tout, tout);
+                               err_code |= ERR_WARN;
+                       }
                }
                else {
                        ha_alert("parsing [%s:%d] : '%s' expects 'retry' or 'resolve' and <time> as arguments got '%s'.\n",