MEDIUM: regex: modify regex_comp() to atomically allocate/free the my_regex struct
Now we atomically allocate the my_regex struct within function
regex_comp() and compile the regex or free both in case of failure. The
pointer to the allocated my_regex struct is returned directly. The
my_regex* argument to regex_comp() is removed.
Function regex_free() was modified so that it systematically frees the
my_regex entry. The function does nothing when called with a NULL as
argument (like free()). It will avoid existing risk of not properly
freeing the initialized area.
Other structures are also updated in order to be compatible (the ones
related to Lua and action rules).
diff --git a/include/common/regex.h b/include/common/regex.h
index c3b921b..0814f8f 100644
--- a/include/common/regex.h
+++ b/include/common/regex.h
@@ -88,7 +88,7 @@
*
* The function return 1 is succes case, else return 0 and err is filled.
*/
-int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **err);
+struct my_regex *regex_comp(const char *str, int cs, int cap, char **err);
int exp_replace(char *dst, unsigned int dst_size, char *src, const char *str, const regmatch_t *matches);
const char *check_replace_string(const char *str);
const char *chain_regex(struct hdr_exp **head, struct my_regex *preg,
@@ -161,6 +161,8 @@
size_t nmatch, regmatch_t pmatch[], int flags);
static inline void regex_free(struct my_regex *preg) {
+ if (!preg)
+ return;
#if defined(USE_PCRE) || defined(USE_PCRE_JIT)
pcre_free(preg->reg);
/* PCRE < 8.20 requires pcre_free() while >= 8.20 requires pcre_study_free(),
@@ -176,6 +178,7 @@
#else
regfree(&preg->regex);
#endif
+ free(preg);
}
#endif /* _COMMON_REGEX_H */
diff --git a/include/types/action.h b/include/types/action.h
index 6c5fccf..948e281 100644
--- a/include/types/action.h
+++ b/include/types/action.h
@@ -121,7 +121,7 @@
char *name; /* header name */
int name_len; /* header name's length */
struct list fmt; /* log-format compatible expression */
- struct my_regex re; /* used by replace-header and replace-value */
+ struct my_regex *re; /* used by replace-header and replace-value */
} hdr_add; /* args used by "add-header" and "set-header" */
struct {
char *name; /* header name */
diff --git a/include/types/hlua.h b/include/types/hlua.h
index 36554e2..70c7685 100644
--- a/include/types/hlua.h
+++ b/include/types/hlua.h
@@ -6,6 +6,7 @@
#include <lua.h>
#include <lauxlib.h>
+#include <common/regex.h>
#include <common/xref.h>
#include <types/proto_http.h>
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index b46e119..7760d9c 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -275,17 +275,10 @@
((px->cap & PR_CAP_BE) ? SMP_VAL_BE_HRS_HDR : SMP_VAL_FE_HRS_HDR),
file, line);
- preg = calloc(1, sizeof(*preg));
- if (!preg) {
- ha_alert("parsing [%s:%d] : '%s' : not enough memory to build regex.\n", file, line, cmd);
- ret_code = ERR_ALERT | ERR_FATAL;
- goto err;
- }
-
cs = !(flags & REG_ICASE);
cap = !(flags & REG_NOSUB);
error = NULL;
- if (!regex_comp(reg, preg, cs, cap, &error)) {
+ if (!(preg = regex_comp(reg, cs, cap, &error))) {
ha_alert("parsing [%s:%d] : '%s' : regular expression '%s' : %s\n", file, line, cmd, reg, error);
free(error);
ret_code = ERR_ALERT | ERR_FATAL;
@@ -309,7 +302,6 @@
err_free:
regex_free(preg);
err:
- free(preg);
free(errmsg);
return ret_code;
}
@@ -444,8 +436,14 @@
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(*curproxy->expect_regex));
- regex_comp(defproxy.expect_str, curproxy->expect_regex, 1, 1, NULL);
+ error = NULL;
+ if (!(curproxy->expect_regex = regex_comp(defproxy.expect_str, 1, 1, &error))) {
+ ha_alert("parsing [%s:%d] : regular expression '%s' : %s\n", file, linenum,
+ defproxy.expect_str, error);
+ free(error);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto out;
+ }
}
}
@@ -631,11 +629,8 @@
free(defproxy.server_id_hdr_name);
defproxy.server_id_hdr_len = 0;
free(defproxy.expect_str);
- if (defproxy.expect_regex) {
- regex_free(defproxy.expect_regex);
- free(defproxy.expect_regex);
- defproxy.expect_regex = NULL;
- }
+ regex_free(defproxy.expect_regex);
+ defproxy.expect_regex = NULL;
if (defproxy.conf.logformat_string != default_http_log_format &&
defproxy.conf.logformat_string != default_tcp_log_format &&
@@ -2927,16 +2922,11 @@
}
curproxy->options2 |= PR_O2_EXP_RSTS;
free(curproxy->expect_str);
- if (curproxy->expect_regex) {
- regex_free(curproxy->expect_regex);
- free(curproxy->expect_regex);
- curproxy->expect_regex = NULL;
- }
+ regex_free(curproxy->expect_regex);
curproxy->expect_str = strdup(args[cur_arg + 1]);
- curproxy->expect_regex = calloc(1, sizeof(*curproxy->expect_regex));
error = NULL;
- if (!regex_comp(args[cur_arg + 1], curproxy->expect_regex, 1, 1, &error)) {
- ha_alert("parsing [%s:%d] : '%s %s %s' : bad regular expression '%s': %s.\n",
+ if (!(curproxy->expect_regex = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
+ ha_alert("parsing [%s:%d] : '%s %s %s' : regular expression '%s': %s.\n",
file, linenum, args[0], args[1], ptr_arg, args[cur_arg + 1], error);
free(error);
err_code |= ERR_ALERT | ERR_FATAL;
@@ -2952,16 +2942,11 @@
}
curproxy->options2 |= PR_O2_EXP_RSTR;
free(curproxy->expect_str);
- if (curproxy->expect_regex) {
- regex_free(curproxy->expect_regex);
- free(curproxy->expect_regex);
- curproxy->expect_regex = NULL;
- }
+ regex_free(curproxy->expect_regex);
curproxy->expect_str = strdup(args[cur_arg + 1]);
- curproxy->expect_regex = calloc(1, sizeof(*curproxy->expect_regex));
error = NULL;
- if (!regex_comp(args[cur_arg + 1], curproxy->expect_regex, 1, 1, &error)) {
- ha_alert("parsing [%s:%d] : '%s %s %s' : bad regular expression '%s': %s.\n",
+ if (!(curproxy->expect_regex = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
+ ha_alert("parsing [%s:%d] : '%s %s %s' : regular expression '%s': %s.\n",
file, linenum, args[0], args[1], ptr_arg, args[cur_arg + 1], error);
free(error);
err_code |= ERR_ALERT | ERR_FATAL;
@@ -3258,10 +3243,9 @@
tcpcheck->action = TCPCHK_ACT_EXPECT;
tcpcheck->string_len = 0;
tcpcheck->string = NULL;
- tcpcheck->expect_regex = calloc(1, sizeof(*tcpcheck->expect_regex));
error = NULL;
- if (!regex_comp(args[cur_arg + 1], tcpcheck->expect_regex, 1, 1, &error)) {
- ha_alert("parsing [%s:%d] : '%s %s %s' : bad regular expression '%s': %s.\n",
+ if (!(tcpcheck->expect_regex = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
+ ha_alert("parsing [%s:%d] : '%s %s %s' : regular expression '%s': %s.\n",
file, linenum, args[0], args[1], ptr_arg, args[cur_arg + 1], error);
free(error);
err_code |= ERR_ALERT | ERR_FATAL;
diff --git a/src/checks.c b/src/checks.c
index a7b4b2f..e6b0bd1 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -3193,8 +3193,7 @@
LIST_DEL(&rule->list);
free(rule->comment);
free(rule->string);
- if (rule->expect_regex)
- regex_free(rule->expect_regex);
+ regex_free(rule->expect_regex);
pool_free(pool_head_tcpcheck_rule, rule);
}
pool_free(pool_head_email_alert, alert);
diff --git a/src/haproxy.c b/src/haproxy.c
index bf91c0b..318ebf2 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2168,11 +2168,7 @@
}
for (exp = p->req_exp; exp != NULL; ) {
- if (exp->preg) {
- regex_free(exp->preg);
- free(exp->preg);
- }
-
+ regex_free(exp->preg);
free((char *)exp->replace);
expb = exp;
exp = exp->next;
@@ -2180,11 +2176,7 @@
}
for (exp = p->rsp_exp; exp != NULL; ) {
- if (exp->preg) {
- regex_free(exp->preg);
- free(exp->preg);
- }
-
+ regex_free(exp->preg);
free((char *)exp->replace);
expb = exp;
exp = exp->next;
diff --git a/src/hlua.c b/src/hlua.c
index ff5ffba..a7fd52b 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -5366,13 +5366,13 @@
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len));
const char *reg = MAY_LJMP(luaL_checkstring(L, 3));
const char *value = MAY_LJMP(luaL_checkstring(L, 4));
- struct my_regex re;
+ struct my_regex *re;
- if (!regex_comp(reg, &re, 1, 1, NULL))
+ if (!(re = regex_comp(reg, 1, 1, NULL)))
WILL_LJMP(luaL_argerror(L, 3, "invalid regex"));
- http_transform_header_str(htxn->s, msg, name, name_len, value, &re, action);
- regex_free(&re);
+ http_transform_header_str(htxn->s, msg, name, name_len, value, re, action);
+ regex_free(re);
return 0;
}
diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index 60882f3..3b71ce1 100644
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -1537,14 +1537,14 @@
return 1;
}
-static struct my_regex *hlua_check_regex(lua_State *L, int ud)
+static struct my_regex **hlua_check_regex(lua_State *L, int ud)
{
return (hlua_checkudata(L, ud, class_regex_ref));
}
static int hlua_regex_comp(struct lua_State *L)
{
- struct my_regex *regex;
+ struct my_regex **regex;
const char *str;
int cs;
char *err;
@@ -1556,7 +1556,7 @@
regex = lua_newuserdata(L, sizeof(*regex));
err = NULL;
- if (!regex_comp(str, regex, cs, 1, &err)) {
+ if (!(*regex = regex_comp(str, cs, 1, &err))) {
lua_pushboolean(L, 0); /* status error */
lua_pushstring(L, err); /* Reason */
free(err);
@@ -1576,7 +1576,7 @@
static int hlua_regex_exec(struct lua_State *L)
{
- struct my_regex *regex;
+ struct my_regex **regex;
const char *str;
size_t len;
struct buffer *tmp;
@@ -1584,6 +1584,11 @@
regex = hlua_check_regex(L, 1);
str = luaL_checklstring(L, 2, &len);
+ if (!*regex) {
+ lua_pushboolean(L, 0);
+ return 1;
+ }
+
/* Copy the string because regex_exec2 require a 'char *'
* and not a 'const char *'.
*/
@@ -1594,14 +1599,14 @@
}
memcpy(tmp->area, str, len);
- lua_pushboolean(L, regex_exec2(regex, tmp->area, len));
+ lua_pushboolean(L, regex_exec2(*regex, tmp->area, len));
return 1;
}
static int hlua_regex_match(struct lua_State *L)
{
- struct my_regex *regex;
+ struct my_regex **regex;
const char *str;
size_t len;
regmatch_t pmatch[20];
@@ -1612,6 +1617,11 @@
regex = hlua_check_regex(L, 1);
str = luaL_checklstring(L, 2, &len);
+ if (!*regex) {
+ lua_pushboolean(L, 0);
+ return 1;
+ }
+
/* Copy the string because regex_exec2 require a 'char *'
* and not a 'const char *'.
*/
@@ -1622,7 +1632,7 @@
}
memcpy(tmp->area, str, len);
- ret = regex_exec_match2(regex, tmp->area, len, 20, pmatch, 0);
+ ret = regex_exec_match2(*regex, tmp->area, len, 20, pmatch, 0);
lua_pushboolean(L, ret);
lua_newtable(L);
if (ret) {
@@ -1636,10 +1646,11 @@
static int hlua_regex_free(struct lua_State *L)
{
- struct my_regex *regex;
+ struct my_regex **regex;
regex = hlua_check_regex(L, 1);
- regex_free(regex);
+ regex_free(*regex);
+ *regex = NULL;
return 0;
}
diff --git a/src/http_rules.c b/src/http_rules.c
index c7b5a71..ab01f26 100644
--- a/src/http_rules.c
+++ b/src/http_rules.c
@@ -263,7 +263,7 @@
LIST_INIT(&rule->arg.hdr_add.fmt);
error = NULL;
- if (!regex_comp(args[cur_arg + 1], &rule->arg.hdr_add.re, 1, 1, &error)) {
+ if (!(rule->arg.hdr_add.re = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
ha_alert("parsing [%s:%d] : '%s' : %s.\n", file, linenum,
args[cur_arg + 1], error);
free(error);
@@ -713,7 +713,7 @@
LIST_INIT(&rule->arg.hdr_add.fmt);
error = NULL;
- if (!regex_comp(args[cur_arg + 1], &rule->arg.hdr_add.re, 1, 1, &error)) {
+ if (!(rule->arg.hdr_add.re = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
ha_alert("parsing [%s:%d] : '%s' : %s.\n", file, linenum,
args[cur_arg + 1], error);
free(error);
@@ -1191,7 +1191,7 @@
list_for_each_entry_safe(pr, tr, r, list) {
LIST_DEL(&pr->list);
- regex_free(&pr->arg.hdr_add.re);
+ regex_free(pr->arg.hdr_add.re);
free(pr);
}
}
@@ -1205,7 +1205,7 @@
if (pr->action == ACT_HTTP_REQ_AUTH)
free(pr->arg.auth.realm);
- regex_free(&pr->arg.hdr_add.re);
+ regex_free(pr->arg.hdr_add.re);
free(pr);
}
}
diff --git a/src/pattern.c b/src/pattern.c
index 49d05a1..c93be9b 100644
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -1142,7 +1142,6 @@
list_for_each_entry_safe(pat, tmp, &expr->patterns, list) {
regex_free(pat->pat.ptr.ptr);
- free(pat->pat.ptr.ptr);
free(pat->pat.data);
free(pat);
}
@@ -1253,18 +1252,9 @@
/* duplicate pattern */
memcpy(&patl->pat, pat, sizeof(*pat));
- /* allocate regex */
- patl->pat.ptr.reg = calloc(1, sizeof(*patl->pat.ptr.reg));
- if (!patl->pat.ptr.reg) {
- free(patl);
- memprintf(err, "out of memory while indexing pattern");
- return 0;
- }
-
/* compile regex */
- if (!regex_comp(pat->ptr.str, patl->pat.ptr.reg,
- !(expr->mflags & PAT_MF_IGNORE_CASE), cap, err)) {
- free(patl->pat.ptr.reg);
+ if (!(patl->pat.ptr.reg = regex_comp(pat->ptr.str, !(expr->mflags & PAT_MF_IGNORE_CASE),
+ cap, err))) {
free(patl);
return 0;
}
@@ -1562,7 +1552,6 @@
/* Delete and free entry. */
LIST_DEL(&pat->list);
regex_free(pat->pat.ptr.ptr);
- free(pat->pat.ptr.ptr);
free(pat->pat.data);
free(pat);
}
diff --git a/src/proto_http.c b/src/proto_http.c
index 2ab3315..b47fbe6 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -1581,7 +1581,7 @@
if (http_transform_header(s, &txn->req, rule->arg.hdr_add.name,
rule->arg.hdr_add.name_len,
&rule->arg.hdr_add.fmt,
- &rule->arg.hdr_add.re, rule->action)) {
+ rule->arg.hdr_add.re, rule->action)) {
rule_ret = HTTP_RULE_RES_BADREQ;
goto end;
}
@@ -1939,7 +1939,7 @@
if (http_transform_header(s, &txn->rsp, rule->arg.hdr_add.name,
rule->arg.hdr_add.name_len,
&rule->arg.hdr_add.fmt,
- &rule->arg.hdr_add.re, rule->action)) {
+ rule->arg.hdr_add.re, rule->action)) {
rule_ret = HTTP_RULE_RES_BADREQ;
goto end;
}
diff --git a/src/proto_htx.c b/src/proto_htx.c
index 2d1d8fb..b99b9cf 100644
--- a/src/proto_htx.c
+++ b/src/proto_htx.c
@@ -2901,7 +2901,7 @@
if (htx_transform_header(s, &s->req, htx,
ist2(rule->arg.hdr_add.name, rule->arg.hdr_add.name_len),
&rule->arg.hdr_add.fmt,
- &rule->arg.hdr_add.re, rule->action)) {
+ rule->arg.hdr_add.re, rule->action)) {
rule_ret = HTTP_RULE_RES_BADREQ;
goto end;
}
@@ -3243,7 +3243,7 @@
if (htx_transform_header(s, &s->res, htx,
ist2(rule->arg.hdr_add.name, rule->arg.hdr_add.name_len),
&rule->arg.hdr_add.fmt,
- &rule->arg.hdr_add.re, rule->action)) {
+ rule->arg.hdr_add.re, rule->action)) {
rule_ret = HTTP_RULE_RES_BADREQ;
goto end;
}
diff --git a/src/regex.c b/src/regex.c
index 713f0c9..6b86b03 100644
--- a/src/regex.c
+++ b/src/regex.c
@@ -337,13 +337,32 @@
#endif
}
-int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **err)
+struct my_regex *regex_comp(const char *str, int cs, int cap, char **err)
{
+ struct my_regex *regex = NULL;
#if defined(USE_PCRE) || defined(USE_PCRE_JIT)
int flags = 0;
const char *error;
int erroffset;
+#elif defined(USE_PCRE2) || defined(USE_PCRE2_JIT)
+ int flags = 0;
+ int errn;
+#if defined(USE_PCRE2_JIT)
+ int jit;
+#endif
+ PCRE2_UCHAR error[256];
+ PCRE2_SIZE erroffset;
+#else
+ int flags = REG_EXTENDED;
+#endif
+
+ regex = calloc(1, sizeof(*regex));
+ if (!regex) {
+ memprintf(err, "not enough memory to build regex");
+ goto out_fail_alloc;
+ }
+#if defined(USE_PCRE) || defined(USE_PCRE_JIT)
if (!cs)
flags |= PCRE_CASELESS;
if (!cap)
@@ -352,24 +371,16 @@
regex->reg = pcre_compile(str, flags, &error, &erroffset, NULL);
if (!regex->reg) {
memprintf(err, "regex '%s' is invalid (error=%s, erroffset=%d)", str, error, erroffset);
- return 0;
+ goto out_fail_alloc;
}
regex->extra = pcre_study(regex->reg, PCRE_STUDY_JIT_COMPILE, &error);
if (!regex->extra && error != NULL) {
pcre_free(regex->reg);
memprintf(err, "failed to compile regex '%s' (error=%s)", str, error);
- return 0;
+ goto out_fail_alloc;
}
#elif defined(USE_PCRE2) || defined(USE_PCRE2_JIT)
- int flags = 0;
- int errn;
-#if defined(USE_PCRE2_JIT)
- int jit;
-#endif
- PCRE2_UCHAR error[256];
- PCRE2_SIZE erroffset;
-
if (!cs)
flags |= PCRE2_CASELESS;
if (!cap)
@@ -379,7 +390,7 @@
if (!regex->reg) {
pcre2_get_error_message(errn, error, sizeof(error));
memprintf(err, "regex '%s' is invalid (error=%s, erroffset=%zu)", str, error, erroffset);
- return 0;
+ goto out_fail_alloc;
}
#if defined(USE_PCRE2_JIT)
@@ -391,13 +402,11 @@
if (jit < 0 && jit != PCRE2_ERROR_JIT_BADOPTION) {
pcre2_code_free(regex->reg);
memprintf(err, "regex '%s' jit compilation failed", str);
- return 0;
+ goto out_fail_alloc;
}
#endif
#else
- int flags = REG_EXTENDED;
-
if (!cs)
flags |= REG_ICASE;
if (!cap)
@@ -405,10 +414,14 @@
if (regcomp(®ex->regex, str, flags) != 0) {
memprintf(err, "regex '%s' is invalid", str);
- return 0;
+ goto out_fail_alloc;
}
#endif
- return 1;
+ return regex;
+
+ out_fail_alloc:
+ free(regex);
+ return NULL;
}
static void regex_register_build_options(void)
diff --git a/src/sample.c b/src/sample.c
index 2a1ce8d..67f59e8 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -1328,20 +1328,11 @@
continue;
}
- reg = calloc(1, sizeof(*reg));
- if (!reg) {
- ha_alert("parsing [%s:%d] : not enough memory to build regex in arg %d of %s%s%s%s '%s' %s proxy '%s'.\n",
- cur->file, cur->line,
- cur->arg_pos + 1, conv_pre, conv_ctx, conv_pos, ctx, cur->kw, where, p->id);
- cfgerr++;
- continue;
- }
-
rflags = 0;
rflags |= (arg->type_flags & ARGF_REG_ICASE) ? REG_ICASE : 0;
err = NULL;
- if (!regex_comp(arg->data.str.area, reg, !(rflags & REG_ICASE), 1 /* capture substr */, &err)) {
+ if (!(reg = regex_comp(arg->data.str.area, !(rflags & REG_ICASE), 1 /* capture substr */, &err))) {
ha_alert("parsing [%s:%d] : error in regex '%s' in arg %d of %s%s%s%s '%s' %s proxy '%s' : %s.\n",
cur->file, cur->line,
arg->data.str.area,
@@ -1425,11 +1416,8 @@
p->unresolved = 0;
}
else if (p->type == ARGT_REG) {
- if (p->data.reg) {
- regex_free(p->data.reg);
- free(p->data.reg);
- p->data.reg = NULL;
- }
+ regex_free(p->data.reg);
+ p->data.reg = NULL;
}
p++;
}