Skip to content

Commit

Permalink
BUG/MEDIUM: actions: always apply a longest match on prefix lookup
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 1e3422e)
Signed-off-by: Willy Tarreau <[email protected]>
  • Loading branch information
wtarreau committed Oct 6, 2023
1 parent 217b0f4 commit 106008b
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions include/haproxy/action.h
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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,
Expand Down

0 comments on commit 106008b

Please sign in to comment.