BUG/MEDIUM: actions: always apply a longest match on prefix lookup

Many actions take arguments after a parenthesis. When this happens, they
have to be tagged in the parser with KWF_MATCH_PREFIX so that a sub-word
is sufficient (since by default the whole block including the parenthesis
is taken).

The problem with this is that the parser stops on the first match. This
was OK years ago when there were very few actions, but over time new ones
were added and many actions are the prefix of another one (e.g. "set-var"
is the prefix of "set-var-fmt"). And what happens in this case is that the
first word is picked. Most often that doesn't cause trouble because such
similar-looking actions involve the same custom parser so actually the
wrong selection of the first entry results in the correct parser to be
used anyway and the error to be silently hidden.

But it's getting worse when accidentally declaring prefixes in multiple
files, because in this case it will solely depend on the object file link
order: if the longest name appears first, it will be properly detected,
but if it appears last, its other prefix will be detected and might very
well not be related at all and use a distinct parser. And this is random
enough to make some actions succeed or fail depending on the build options
that affect the linkage order. Worse: what if a keyword is the prefix of
another one, with a different parser but a compatible syntax ? It could
seem to work by accident but not do the expected operations.

The correct solution is to always look for the longest matching name.
This way the correct keyword will always be matched and used and there
will be no risk to randomly pick the wrong anymore.

This fix must be backported to the relevant stable releases.

diff --git a/include/haproxy/action.h b/include/haproxy/action.h
index 73d672e47ec07302c1884be90741ba81c96ebfae..dba1408b95f53ff9bf915a53a00c78a9d24a2bba 100644 (file)
--- a/include/haproxy/action.h
+++ b/include/haproxy/action.h
@@ -39,6 +39,8 @@ void free_act_rule(struct act_rule *rule);
 static inline struct action_kw *action_lookup(struct list *keywords, const char *kw)
 {
        struct action_kw_list *kw_list;
+       struct action_kw *best = NULL;
+       int len, bestlen = 0;
        int i;
 
        if (LIST_ISEMPTY(keywords))
@@ -47,13 +49,18 @@ static inline struct action_kw *action_lookup(struct list *keywords, const char
        list_for_each_entry(kw_list, keywords, list) {
                for (i = 0; kw_list->kw[i].kw != NULL; i++) {
                        if ((kw_list->kw[i].flags & KWF_MATCH_PREFIX) &&
-                           strncmp(kw, kw_list->kw[i].kw, strlen(kw_list->kw[i].kw)) == 0)
-                               return &kw_list->kw[i];
+                           (len = strlen(kw_list->kw[i].kw)) > bestlen &&
+                           strncmp(kw, kw_list->kw[i].kw, len) == 0) {
+                               if (len > bestlen) {
+                                       bestlen = len;
+                                       best = &kw_list->kw[i];
+                               }
+                       }
                        if (strcmp(kw, kw_list->kw[i].kw) == 0)
                                return &kw_list->kw[i];
                }
        }
-       return NULL;
+       return best;
 }
 
 static inline void action_build_list(struct list *keywords,