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 8e29d38cc2097968f725cb2a51a88acf7e39d99e..aa9fd777c4e77f6ae009bc2b5dc6f16cc8df8f83 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 85691de87e8ea940a2e00722e28204e91c9c82aa..29a86b7dd37cc5a37b4fbb139dce312b87cd54df 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 6f951cd506fe6a1933543c1a33669eddb99e17c5..357704209fc9e03951d87c8131355aeab648e83f 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 88746898098f2893b60a8a207d4a9c8b0de871f3..f8f0c8edfcf9758fd7c2aee42f6ba30513976b9d 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",