From 1ee51a658186b354bd1713f8102ca9bd446105db Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 19 Aug 2011 20:04:17 +0200 Subject: [PATCH] [BUG] check: http-check expect + regex would crash in defaults section Manoj Kumar reported a case where haproxy would crash upon start-up. The cause was an "http-check expect" statement declared in the defaults section, which caused a NULL regex to be used during the check. This statement is not allowed in defaults sections precisely because this requires saving a copy of the regex in the default proxy. But the check was not made to prevent it from being declared there, hence the issue. Instead of adding code to detect its abnormal use, we decided to implement it. It was not that much complex because the expect_str part was not used with regexes, so it could hold the string form of the regex in order to compile it again for every backend (there's no way to clone regexes). This patch has been tested and works. So it's both a bugfix and a minor feature enhancement. It should be backported to 1.4 though it's not critical since the config was not supposed to be supported. --- doc/configuration.txt | 2 +- include/types/proxy.h | 2 +- src/cfgparse.c | 21 ++++++++++++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index afffc63eb..291cb013f 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -2330,7 +2330,7 @@ http-check disable-on-404 http-check expect [!] Make HTTP health checks consider reponse contents or specific status codes May be used in sections : defaults | frontend | listen | backend - no | no | yes | yes + yes | no | yes | yes Arguments : is a keyword indicating how to look for a specific pattern in the response. The keyword may be one of "status", "rstatus", diff --git a/include/types/proxy.h b/include/types/proxy.h index e4fcc7750..da64bb1d7 100644 --- a/include/types/proxy.h +++ b/include/types/proxy.h @@ -316,7 +316,7 @@ struct proxy { int grace; /* grace time after stop request */ char *check_req; /* HTTP or SSL request to use for PR_O_HTTP_CHK|PR_O_SSL3_CHK */ int check_len; /* Length of the HTTP or SSL3 request */ - char *expect_str; /* http-check expected content */ + char *expect_str; /* http-check expected content : string or text version of the regex */ regex_t *expect_regex; /* http-check expected content */ struct chunk errmsg[HTTP_ERR_SIZE]; /* default or customized error messages for known errors */ int uuid; /* universally unique proxy ID, used for SNMP */ diff --git a/src/cfgparse.c b/src/cfgparse.c index 8c61f3d53..f6a0ef2fe 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -1425,6 +1425,15 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) } curproxy->check_len = defproxy.check_len; + if (defproxy.expect_str) { + curproxy->expect_str = strdup(defproxy.expect_str); + if (defproxy.expect_regex) { + /* note: this regex is known to be valid */ + curproxy->expect_regex = calloc(1, sizeof(regex_t)); + regcomp(curproxy->expect_regex, defproxy.expect_str, REG_EXTENDED); + } + } + if (defproxy.cookie_name) curproxy->cookie_name = strdup(defproxy.cookie_name); curproxy->cookie_len = defproxy.cookie_len; @@ -1522,6 +1531,8 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) defproxy.fwdfor_hdr_len = 0; free(defproxy.orgto_hdr_name); defproxy.orgto_hdr_len = 0; + free(defproxy.expect_str); + if (defproxy.expect_regex) regfree(defproxy.expect_regex); for (rc = 0; rc < HTTP_ERR_SIZE; rc++) chunk_destroy(&defproxy.errmsg[rc]); @@ -3635,6 +3646,7 @@ stats_error_parsing: goto out; } curproxy->options2 |= PR_O2_EXP_STS; + free(curproxy->expect_str); curproxy->expect_str = strdup(args[cur_arg + 1]); } else if (strcmp(ptr_arg, "string") == 0) { @@ -3645,6 +3657,7 @@ stats_error_parsing: goto out; } curproxy->options2 |= PR_O2_EXP_STR; + free(curproxy->expect_str); curproxy->expect_str = strdup(args[cur_arg + 1]); } else if (strcmp(ptr_arg, "rstatus") == 0) { @@ -3655,6 +3668,9 @@ stats_error_parsing: goto out; } curproxy->options2 |= PR_O2_EXP_RSTS; + free(curproxy->expect_str); + if (curproxy->expect_regex) regfree(curproxy->expect_regex); + curproxy->expect_str = strdup(args[cur_arg + 1]); curproxy->expect_regex = calloc(1, sizeof(regex_t)); if (regcomp(curproxy->expect_regex, args[cur_arg + 1], REG_EXTENDED) != 0) { Alert("parsing [%s:%d] : '%s %s %s' : bad regular expression '%s'.\n", @@ -3671,6 +3687,9 @@ stats_error_parsing: goto out; } curproxy->options2 |= PR_O2_EXP_RSTR; + free(curproxy->expect_str); + if (curproxy->expect_regex) regfree(curproxy->expect_regex); + curproxy->expect_str = strdup(args[cur_arg + 1]); curproxy->expect_regex = calloc(1, sizeof(regex_t)); if (regcomp(curproxy->expect_regex, args[cur_arg + 1], REG_EXTENDED) != 0) { Alert("parsing [%s:%d] : '%s %s %s' : bad regular expression '%s'.\n", @@ -3687,7 +3706,7 @@ stats_error_parsing: } } else { - Alert("parsing [%s:%d] : '%s' only supports 'disable-on-404', 'expect' .\n", file, linenum, args[0]); + Alert("parsing [%s:%d] : '%s' only supports 'disable-on-404', 'send-state', 'expect'.\n", file, linenum, args[0]); err_code |= ERR_ALERT | ERR_FATAL; goto out; }