]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: sample: add missing ADDR=>? compatibility matrix entries
authorAurelien DARRAGON <adarragon@haproxy.com>
Tue, 6 Jun 2023 13:58:49 +0000 (15:58 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 3 Jul 2023 14:32:01 +0000 (16:32 +0200)
SMP_T_ADDR support was added in b805f71 ("MEDIUM: sample: let the cast
functions set their output type").

According to the above commit, it is made clear that the ADDR type is
a pseudo/generic type that may be used for compatibility checks but
that cannot be emitted from a fetch or converter.

With that in mind, all conversions from ADDR to other types were
explicitly disabled in the compatibility matrix.

But later, when map_*_ip functions were updated in b2f8f08 ("MINOR: map:
The map can return IPv4 and IPv6"), we started using ADDR as "expected"
output type for converters. This still complies with the original
description from b805f71, because it is used as the theoric output
type, and is never emitted from the converters themselves (only "real"
types such as IPV4 or IPV6 are actually being emitted at runtime).

But this introduced an ambiguity as well as a few bugs, because some
compatibility checks are being performed at config parse time, and thus
rely on the expected output type to check if the conversion from current
element to the next element in the chain is theorically supported.

However, because the compatibility matrix doesn't support ADDR to other
types it is currently impossible to use map_*_ip converters in the middle
of a chain (the only supported usage is when map_*_ip converters are
at the very end of the chain).

To illustrate this, consider the following examples:

  acl test str(ok),map_str_ip(test.map) -m found                          # this will work
  acl test str(ok),map_str_ip(test.map),ipmask(24) -m found               # this will raise an error

Likewise, stktable_compatible_sample() check for stick tables also relies
on out_type[table_type] compatibility check, so map_*_ip cannot be used
with sticktables at the moment:

  backend st_test
    stick-table type string size 1m expire 10m store http_req_rate(10m)
  frontend fe
    bind localhost:8080
    mode http
    http-request track-sc0 str(test),map_str_ip(test.map) table st_test     # raises an error

To fix this, and prevent future usage of ADDR as expected output type
(for either fetches or converters) from introducing new bugs, the
ADDR=>? part of the matrix should follow the ANY type logic.

That is, ADDR, which is a pseudo-type, must be compatible with itself,
and where IPV4 and IPV6 both support a backward conversion to a given
type, ADDR must support it as well. It is done by setting the relevant
ADDR entries to c_pseudo() in the compatibility matrix to indicate that
the operation is theorically supported (c_pseudo() will never be executed
because ADDR should not be emitted: this only serves as a hint for
compatibility checks at parse time).

This is what's being done in this commit, thanks to this the broken
examples documented above should work as expected now, and future
usage of ADDR as out_type should not cause any issue.

include/haproxy/sample-t.h
src/sample.c

index 9969a18d6cea3bcb195696888a39e50c58147095..b2da0772b2417a5ca550f7724f14b758f2875bcb 100644 (file)
@@ -36,7 +36,7 @@ enum {
        SMP_T_ANY = 0,   /* pseudo type: any type */
        SMP_T_BOOL,      /* boolean */
        SMP_T_SINT,      /* signed 64bits integer type */
-       SMP_T_ADDR,      /* ipv4 or ipv6, only used for input type compatibility */
+       SMP_T_ADDR,      /* pseudo type: could be ipv4 or ipv6 */
        SMP_T_IPV4,      /* ipv4 type */
        SMP_T_IPV6,      /* ipv6 type */
        SMP_T_STR,       /* char string type */
index a8eb50713d180bcf3029d7a87cc7019053f443df..48f108de53df81daa172a49f4cb09a111870df5f 100644 (file)
@@ -997,7 +997,7 @@ sample_cast_fct sample_casts[SMP_TYPES][SMP_TYPES] = {
 /* from:  ANY */ { c_none, c_pseudo,  c_pseudo,  c_pseudo,   c_pseudo, c_pseudo,   c_pseudo,   c_pseudo,   c_pseudo,   },
 /*       BOOL */ { c_none, c_none,    c_none,    NULL,       NULL,     NULL,       c_int2str,  c_bool2bin, NULL,       },
 /*       SINT */ { c_none, c_none,    c_none,    c_int2ip,   c_int2ip, c_int2ipv6, c_int2str,  c_int2bin,  NULL,       },
-/*       ADDR */ { c_none, NULL,      NULL,      NULL,       NULL,     NULL,       NULL,       NULL,       NULL,       },
+/*       ADDR */ { c_none, NULL,      NULL,      c_pseudo,   c_pseudo, c_pseudo,   c_pseudo,   c_pseudo,   NULL,       },
 /*       IPV4 */ { c_none, NULL,      c_ip2int,  c_none,     c_none,   c_ip2ipv6,  c_ip2str,   c_addr2bin, NULL,       },
 /*       IPV6 */ { c_none, NULL,      NULL,      c_none,     c_ipv62ip,c_none,     c_ipv62str, c_addr2bin, NULL,       },
 /*        STR */ { c_none, c_str2int, c_str2int, c_str2addr, c_str2ip, c_str2ipv6, c_none,     c_none,     c_str2meth, },