diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c index d611445d60a..bbc5715f48b 100644 --- a/bin/varnishd/cache/cache_ban.c +++ b/bin/varnishd/cache/cache_ban.c @@ -39,6 +39,7 @@ #include "cache_ban.h" #include "cache_objhead.h" +#include "vbm.h" #include "vcli_serve.h" #include "vend.h" #include "vmb.h" @@ -66,6 +67,10 @@ struct ban_test { const void *arg2_spec; }; +// mark invalid arg1/oper so we report only once +static struct vbitmap *inval_oper_logged; +static struct vbitmap *inval_arg1_logged; + static const char * const arg_name[BAN_ARGARRSZ + 1] = { #define PVAR(a, b, c) [BAN_ARGIDX(c)] = (a), #include "tbl/ban_vars.h" @@ -215,6 +220,8 @@ ban_get_lump(const uint8_t **bs) /*-------------------------------------------------------------------- * Pick a test apart from a spec string + * + * NOTICE: This code must be invariant to unknown arg1 and oper */ static void @@ -486,6 +493,30 @@ BAN_Time(const struct ban *b) * Evaluate ban-spec */ +static void +ban_inval_arg1(uint8_t arg1) +{ + Lck_Lock(&ban_mtx); + VSC_C_main->bans_inval_arg1++; + Lck_Unlock(&ban_mtx); + if (vbit_test(inval_arg1_logged, arg1)) + return; + vbit_set(inval_arg1_logged, arg1); + VSL(SLT_Error, NO_VXID, "Unsupported ban argument 0x%02x", arg1); +} + +static void +ban_inval_oper(uint8_t oper) +{ + Lck_Lock(&ban_mtx); + VSC_C_main->bans_inval_oper++; + Lck_Unlock(&ban_mtx); + if (vbit_test(inval_oper_logged, oper)) + return; + vbit_set(inval_oper_logged, oper); + VSL(SLT_Error, NO_VXID, "Unsupported ban operator 0x%02x", oper); +} + int ban_evaluate(struct worker *wrk, const uint8_t *bsarg, struct objcore *oc, const struct http *reqhttp, unsigned *tests) @@ -547,7 +578,8 @@ ban_evaluate(struct worker *wrk, const uint8_t *bsarg, struct objcore *oc, darg2 = bt.arg2_double; break; default: - WRONG("Wrong BAN_ARG code"); + ban_inval_arg1(bt.arg1); + continue; } switch (bt.oper) { @@ -608,7 +640,7 @@ ban_evaluate(struct worker *wrk, const uint8_t *bsarg, struct objcore *oc, return (0); break; default: - WRONG("Wrong BAN_OPER code"); + ban_inval_oper(bt.oper); } } return (1); @@ -794,6 +826,27 @@ ccf_ban(struct cli *cli, const char * const *av, void *priv) bprintf((buf), "%jus", dec); \ } while (0) +static void +ban_render_inval(struct cli *cli, uint8_t arg1, uint8_t oper) +{ + if (! IS_BAN_ARG(arg1)) + ban_inval_arg1(arg1); + if (! IS_BAN_OPER(oper)) + ban_inval_oper(oper); + + if (IS_BAN_ARG(arg1)) + VCLI_Out(cli, "%s", arg_name[BAN_ARGIDX(arg1)]); + else + VCLI_Out(cli, "(0x%02x)", arg1); + + if (IS_BAN_OPER(oper)) + VCLI_Out(cli, " %s ", ban_oper[BAN_OPERIDX(oper)]); + else + VCLI_Out(cli, " (0x%02x) ", oper); + + VCLI_Out(cli, "UNSUPPORTED"); +} + static void ban_render(struct cli *cli, const uint8_t *bs, int quote) { @@ -805,6 +858,13 @@ ban_render(struct cli *cli, const uint8_t *bs, int quote) bs += BANS_HEAD_LEN; while (bs < be) { ban_iter(&bs, &bt); + if (UNLIKELY(! IS_BAN_ARG(bt.arg1) || ! IS_BAN_OPER(bt.oper))) { + ban_render_inval(cli, bt.arg1, bt.oper); + if (bs < be) + VCLI_Out(cli, " && "); + continue; + } + ASSERT_BAN_ARG(bt.arg1); ASSERT_BAN_OPER(bt.oper); @@ -985,6 +1045,12 @@ BAN_Init(void) { struct ban_proto *bp; + // uses malloc where static could be used, but only once + inval_oper_logged = vbit_new(256); + inval_arg1_logged = vbit_new(256); + AN(inval_oper_logged); + AN(inval_arg1_logged); + BAN_Build_Init(); Lck_New(&ban_mtx, lck_ban); CLI_AddFuncs(ban_cmds); @@ -1028,4 +1094,9 @@ BAN_Shutdown(void) Lck_Unlock(&ban_mtx); BAN_Build_Fini(); + + vbit_destroy(inval_oper_logged); + vbit_destroy(inval_arg1_logged); + inval_oper_logged = NULL; + inval_arg1_logged = NULL; } diff --git a/bin/varnishd/cache/cache_ban.h b/bin/varnishd/cache/cache_ban.h index d3339eef02d..48c86131333 100644 --- a/bin/varnishd/cache/cache_ban.h +++ b/bin/varnishd/cache/cache_ban.h @@ -86,7 +86,8 @@ #define BAN_OPERIDX(x) ((x) - BANS_OPER_OFF_) #define BAN_OPERARRSZ (BANS_OPER_LIM_ - BANS_OPER_OFF_) -#define ASSERT_BAN_OPER(x) assert((x) >= BANS_OPER_OFF_ && (x) < BANS_OPER_LIM_) +#define IS_BAN_OPER(x) ((x) >= BANS_OPER_OFF_ && (x) < BANS_OPER_LIM_) +#define ASSERT_BAN_OPER(x) assert(IS_BAN_OPER(x)) #define BANS_ARG_URL 0x18 #define BANS_ARG_OFF_ BANS_ARG_URL @@ -101,7 +102,8 @@ #define BAN_ARGIDX(x) ((x) - BANS_ARG_OFF_) #define BAN_ARGARRSZ (BANS_ARG_LIM - BANS_ARG_OFF_) -#define ASSERT_BAN_ARG(x) assert((x) >= BANS_ARG_OFF_ && (x) < BANS_ARG_LIM) +#define IS_BAN_ARG(x) ((x) >= BANS_ARG_OFF_ && (x) < BANS_ARG_LIM) +#define ASSERT_BAN_ARG(x) assert(IS_BAN_ARG(x)) // has an arg1_spec (BANS_FLAG_HTTP at build time) #define BANS_HAS_ARG1_SPEC(arg) \ diff --git a/lib/libvsc/VSC_main.vsc b/lib/libvsc/VSC_main.vsc index 22097f2d8a0..739d641b8a2 100644 --- a/lib/libvsc/VSC_main.vsc +++ b/lib/libvsc/VSC_main.vsc @@ -925,6 +925,22 @@ Number of extra bytes accumulated through dropped and completed bans in the persistent ban lists. +.. varnish_vsc:: bans_inval_arg1 + :group: ban_mtx + :oneliner: Unsupported ban argument encountered + + Number of times an unsupported ban argument has been encountered. + + Unsupported ban arguments always evaluate true. + +.. varnish_vsc:: bans_inval_oper + :group: ban_mtx + :oneliner: Unsupported ban operator encountered + + Number of times an unsupported ban operator has been encountered. + + Unsupported ban operators always evaluate true. + .. varnish_vsc:: n_purges :oneliner: Number of purge operations executed