MEDIUM: acl: extend the pattern parsers to report meaningful errors

By passing the error pointer to all ACL parsers, we can make them report
useful errors and not simply fail.

diff --git a/include/proto/acl.h b/include/proto/acl.h
index 2c74e2db331f5c4faa4162af20306c81e6fed12a..c60b9140d3cc57cd17964c17b99b1754af614293 100644 (file)
--- a/include/proto/acl.h
+++ b/include/proto/acl.h
@@ -133,7 +133,7 @@ void acl_unregister_keywords(struct acl_kw_list *kwl);
 
 
 /* ignore the current line */
-int acl_parse_nothing(const char **text, struct acl_pattern *pattern, int *opaque);
+int acl_parse_nothing(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
 
 /* NB: For two strings to be identical, it is required that their lengths match */
 int acl_match_str(struct sample *smp, struct acl_pattern *pattern);
@@ -145,31 +145,31 @@ int acl_match_len(struct sample *smp, struct acl_pattern *pattern);
 int acl_match_int(struct sample *smp, struct acl_pattern *pattern);
 
 /* Parse an integer. It is put both in min and max. */
-int acl_parse_int(const char **text, struct acl_pattern *pattern, int *opaque);
+int acl_parse_int(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
 
 /* Parse an version. It is put both in min and max. */
-int acl_parse_dotted_ver(const char **text, struct acl_pattern *pattern, int *opaque);
+int acl_parse_dotted_ver(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
 
 /* Parse a range of integers delimited by either ':' or '-'. If only one
  * integer is read, it is set as both min and max.
  */
-int acl_parse_range(const char **text, struct acl_pattern *pattern, int *opaque);
+int acl_parse_range(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
 
 /* Parse a string. It is allocated and duplicated. */
-int acl_parse_str(const char **text, struct acl_pattern *pattern, int *opaque);
+int acl_parse_str(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
 
 /* Parse and concatenate strings into one. It is allocated and duplicated. */
-int acl_parse_strcat(const char **text, struct acl_pattern *pattern, int *opaque);
+int acl_parse_strcat(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
 
 /* Parse a regex. It is allocated. */
-int acl_parse_reg(const char **text, struct acl_pattern *pattern, int *opaque);
+int acl_parse_reg(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
 
 /* Parse an IP address and an optional mask in the form addr[/mask].
  * The addr may either be an IPv4 address or a hostname. The mask
  * may either be a dotted mask or a number of bits. Returns 1 if OK,
  * otherwise 0.
  */
-int acl_parse_ip(const char **text, struct acl_pattern *pattern, int *opaque);
+int acl_parse_ip(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
 
 /* always fake a data retrieval */
 int acl_fetch_nothing(struct proxy *px, struct session *l4, void *l7, unsigned int opt,

diff --git a/include/types/acl.h b/include/types/acl.h
index 00dfa0c6cc971cd68092f1ed10efcca9df69a7e9..9e560cf6453c3a3488673757a7cb8c794f2665fa 100644 (file)
--- a/include/types/acl.h
+++ b/include/types/acl.h
@@ -229,7 +229,7 @@ struct session;
 struct acl_expr;
 struct acl_keyword {
        const char *kw;
-       int (*parse)(const char **text, struct acl_pattern *pattern, int *opaque);
+       int (*parse)(const char **text, struct acl_pattern *pattern, int *opaque, char **err);
        int (*fetch)(struct proxy *px, struct session *l4, void *l7, unsigned int opt,
                     const struct arg *args, struct sample *smp);
        int (*match)(struct sample *smp, struct acl_pattern *pattern);

diff --git a/src/acl.c b/src/acl.c
index 0298353d01f260b0f2b09131e2af7246accb0ed7..a1346b8c804c2d908dda3613dc6ed963a33c1e75 100644 (file)
--- a/src/acl.c
+++ b/src/acl.c
@@ -458,7 +458,7 @@ acl_fetch_ssl_hello_sni(struct proxy *px, struct session *l4, void *l7, unsigned
  */
 
 /* ignore the current line */
-int acl_parse_nothing(const char **text, struct acl_pattern *pattern, int *opaque)
+int acl_parse_nothing(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
        return 1;
 }
@@ -733,7 +733,7 @@ static void *acl_lookup_ip(struct sample *smp, struct acl_expr *expr)
 }
 
 /* Parse a string. It is allocated and duplicated. */
-int acl_parse_str(const char **text, struct acl_pattern *pattern, int *opaque)
+int acl_parse_str(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
        int len;
 
@@ -746,8 +746,11 @@ int acl_parse_str(const char **text, struct acl_pattern *pattern, int *opaque)
                struct ebmb_node *node;
 
                node = calloc(1, sizeof(*node) + len + 1);
-               if (!node)
+               if (!node) {
+                       if (err)
+                               memprintf(err, "out of memory while loading string pattern");
                        return 0;
+               }
                memcpy(node->key, *text, len + 1);
                if (ebst_insert(pattern->val.tree, node) != node)
                        free(node); /* was a duplicate */
@@ -756,15 +759,18 @@ int acl_parse_str(const char **text, struct acl_pattern *pattern, int *opaque)
        }
 
        pattern->ptr.str = strdup(*text);
-       if (!pattern->ptr.str)
+       if (!pattern->ptr.str) {
+               if (err)
+                       memprintf(err, "out of memory while loading string pattern");
                return 0;
+       }
        pattern->len = len;
        return 1;
 }
 
 /* Parse and concatenate all further strings into one. */
 int
-acl_parse_strcat(const char **text, struct acl_pattern *pattern, int *opaque)
+acl_parse_strcat(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
 
        int len = 0, i;
@@ -774,8 +780,11 @@ acl_parse_strcat(const char **text, struct acl_pattern *pattern, int *opaque)
                len += strlen(text[i])+1;
 
        pattern->ptr.str = s = calloc(1, len);
-       if (!pattern->ptr.str)
+       if (!pattern->ptr.str) {
+               if (err)
+                       memprintf(err, "out of memory while loading pattern");
                return 0;
+       }
 
        for (i = 0; *text[i]; i++)
                s += sprintf(s, i?" %s":"%s", text[i]);
@@ -792,19 +801,24 @@ static void acl_free_reg(void *ptr)
 }
 
 /* Parse a regex. It is allocated. */
-int acl_parse_reg(const char **text, struct acl_pattern *pattern, int *opaque)
+int acl_parse_reg(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
        regex_t *preg;
        int icase;
 
        preg = calloc(1, sizeof(regex_t));
 
-       if (!preg)
+       if (!preg) {
+               if (err)
+                       memprintf(err, "out of memory while loading pattern");
                return 0;
+       }
 
        icase = (pattern->flags & ACL_PAT_F_IGNORE_CASE) ? REG_ICASE : 0;
        if (regcomp(preg, *text, REG_EXTENDED | REG_NOSUB | icase) != 0) {
                free(preg);
+               if (err)
+                       memprintf(err, "regex '%s' is invalid", *text);
                return 0;
        }
 
@@ -823,8 +837,11 @@ int acl_parse_reg(const char **text, struct acl_pattern *pattern, int *opaque)
  * rejected for other operators. The operator may be changed at any time.
  * The operator is stored in the 'opaque' argument.
  *
+ * If err is non-NULL, an error message will be returned there on errors and
+ * the caller will have to free it.
+ *
  */
-int acl_parse_int(const char **text, struct acl_pattern *pattern, int *opaque)
+int acl_parse_int(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
        signed long long i;
        unsigned int j, last, skip = 0;
@@ -839,6 +856,8 @@ int acl_parse_int(const char **text, struct acl_pattern *pattern, int *opaque)
                case STD_OP_LT: *opaque = 3; break;
                case STD_OP_LE: *opaque = 4; break;
                default:
+                       if (err)
+                               memprintf(err, "'%s' is neither a number nor a supported operator", ptr);
                        return 0;
                }
 
@@ -863,9 +882,12 @@ int acl_parse_int(const char **text, struct acl_pattern *pattern, int *opaque)
                 i += j;
         }
 
-       if (last && *opaque >= 1 && *opaque <= 4)
+       if (last && *opaque >= 1 && *opaque <= 4) {
                /* having a range with a min or a max is absurd */
+               if (err)
+                       memprintf(err, "integer range '%s' specified with a comparison operator", text[skip]);
                return 0;
+       }
 
        if (!last)
                pattern->val.range.min = i;
@@ -912,7 +934,7 @@ int acl_parse_int(const char **text, struct acl_pattern *pattern, int *opaque)
  *    acl valid_ssl       ssl_req_proto 3.0-3.1
  *
  */
-int acl_parse_dotted_ver(const char **text, struct acl_pattern *pattern, int *opaque)
+int acl_parse_dotted_ver(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
        signed long long i;
        unsigned int j, last, skip = 0;
@@ -927,6 +949,8 @@ int acl_parse_dotted_ver(const char **text, struct acl_pattern *pattern, int *op
                case STD_OP_LT: *opaque = 3; break;
                case STD_OP_LE: *opaque = 4; break;
                default:
+                       if (err)
+                               memprintf(err, "'%s' is neither a number nor a supported operator", ptr);
                        return 0;
                }
 
@@ -964,9 +988,12 @@ int acl_parse_dotted_ver(const char **text, struct acl_pattern *pattern, int *op
        if (i < 65536)
                i <<= 16;
 
-       if (last && *opaque >= 1 && *opaque <= 4)
+       if (last && *opaque >= 1 && *opaque <= 4) {
                /* having a range with a min or a max is absurd */
+               if (err)
+                       memprintf(err, "version range '%s' specified with a comparison operator", text[skip]);
                return 0;
+       }
 
        if (!last)
                pattern->val.range.min = i;
@@ -998,7 +1025,7 @@ int acl_parse_dotted_ver(const char **text, struct acl_pattern *pattern, int *op
  * may either be a dotted mask or a number of bits. Returns 1 if OK,
  * otherwise 0.
  */
-int acl_parse_ip(const char **text, struct acl_pattern *pattern, int *opaque)
+int acl_parse_ip(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
        struct eb_root *tree = NULL;
        if (pattern->flags & ACL_PAT_F_TREE_OK)
@@ -1016,8 +1043,11 @@ int acl_parse_ip(const char **text, struct acl_pattern *pattern, int *opaque)
                        mask = mask ? 33 - flsnz(mask & -mask) : 0; /* equals cidr value */
                        /* FIXME: insert <addr>/<mask> into the tree here */
                        node = calloc(1, sizeof(*node) + 4); /* reserve 4 bytes for IPv4 address */
-                       if (!node)
+                       if (!node) {
+                               if (err)
+                                       memprintf(err, "out of memory while loading IPv4 pattern");
                                return 0;
+                       }
                        memcpy(node->key, &pattern->val.ipv4.addr, 4); /* network byte order */
                        node->node.pfx = mask;
                        if (ebmb_insert_prefix(tree, node, 4) != node)
@@ -1027,8 +1057,11 @@ int acl_parse_ip(const char **text, struct acl_pattern *pattern, int *opaque)
                }
                return 1;
        }
-       else
+       else {
+               if (err)
+                       memprintf(err, "'%s' is not a valid IPv4 address", *text);
                return 0;
+       }
 }
 
 /*
@@ -1215,11 +1248,8 @@ static int acl_read_patterns_from_file(  struct acl_keyword *aclkw,
                        pattern->val.tree = &expr->pattern_tree;
                }
 
-               if (!aclkw->parse(args, pattern, &opaque)) {
-                       memprintf(err, "failed to parse pattern '%s' at line %d of file <%s>",
-                                 *args, line, filename);
+               if (!aclkw->parse(args, pattern, &opaque, err))
                        goto out_free_pattern;
-               }
 
                /* if the parser did not feed the tree, let's chain the pattern to the list */
                if (!(pattern->flags & ACL_PAT_F_TREE)) {
@@ -1386,12 +1416,10 @@ struct acl_expr *parse_acl_expr(const char **args, char **err)
                }
                pattern->flags = patflags;
 
-               ret = aclkw->parse(args, pattern, &opaque);
-               if (!ret) {
-                       if (err)
-                               memprintf(err, "failed to parse some ACL patterns");
+               ret = aclkw->parse(args, pattern, &opaque, err);
+               if (!ret)
                        goto out_free_pattern;
-               }
+
                LIST_ADDQ(&expr->patterns, &pattern->list);
                args += ret;
        }

diff --git a/src/proto_http.c b/src/proto_http.c
index 07e0eb59737523870eddba5fa4eab16fa6914b52..60526f85097e64a60ee6d34ad631ead484d97818 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -7624,7 +7624,7 @@ acl_prefetch_http(struct proxy *px, struct session *s, void *l7, unsigned int op
  * We use the pre-parsed method if it is known, and store its number as an
  * integer. If it is unknown, we use the pointer and the length.
  */
-static int acl_parse_meth(const char **text, struct acl_pattern *pattern, int *opaque)
+static int acl_parse_meth(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
        int len, meth;
 
@@ -7634,8 +7634,11 @@ static int acl_parse_meth(const char **text, struct acl_pattern *pattern, int *o
        pattern->val.i = meth;
        if (meth == HTTP_METH_OTHER) {
                pattern->ptr.str = strdup(*text);
-               if (!pattern->ptr.str)
+               if (!pattern->ptr.str) {
+                       if (err)
+                               memprintf(err, "out of memory while loading pattern");
                        return 0;
+               }
                pattern->len = len;
        }
        return 1;
@@ -7704,11 +7707,14 @@ static int acl_match_meth(struct sample *smp, struct acl_pattern *pattern)
 /* 2. Check on Request/Status Version
  * We simply compare strings here.
  */
-static int acl_parse_ver(const char **text, struct acl_pattern *pattern, int *opaque)
+static int acl_parse_ver(const char **text, struct acl_pattern *pattern, int *opaque, char **err)
 {
        pattern->ptr.str = strdup(*text);
-       if (!pattern->ptr.str)
+       if (!pattern->ptr.str) {
+               if (err)
+                       memprintf(err, "out of memory while loading pattern");
                return 0;
+       }
        pattern->len = strlen(*text);
        return 1;
 }