On Wed, Aug 14, 2024 at 01:51:21PM +0200, Pablo Neira Ayuso wrote:
> Bail out if rate are used:
>
> ruleset.nft:5:77-106: Error: Wrong rate format, expecting bytes or kbytes or mbytes
> add rule netdev firewall PROTECTED_IPS update @quota_temp_before { ip daddr quota over 45000 mbytes/second } add @quota_trigger { ip daddr }
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> improve error reporting while at this.
>
> Fixes: 6615676d825e ("src: add per-bytes limit")
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> v2: - change patch subject
> - use strndup() to fetch units in rate_parse() so limit rate does not break.
>
> src/datatype.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/src/datatype.c b/src/datatype.c
> index d398a9c8c618..297c5d0409d5 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -1485,14 +1485,14 @@ static struct error_record *time_unit_parse(const struct location *loc,
> struct error_record *data_unit_parse(const struct location *loc,
> const char *str, uint64_t *rate)
> {
> - if (strncmp(str, "bytes", strlen("bytes")) == 0)
> + if (strcmp(str, "bytes") == 0)
> *rate = 1ULL;
> - else if (strncmp(str, "kbytes", strlen("kbytes")) == 0)
> + else if (strcmp(str, "kbytes") == 0)
> *rate = 1024;
> - else if (strncmp(str, "mbytes", strlen("mbytes")) == 0)
> + else if (strcmp(str, "mbytes") == 0)
> *rate = 1024 * 1024;
> else
> - return error(loc, "Wrong rate format");
> + return error(loc, "Wrong unit format, expecting bytes, kbytes or mbytes");
>
> return NULL;
> }
I have local commits which introduce KBYTES and MBYTES keywords and
thereby kill the need for quota_unit and limit_bytes cases in
parser_bison.y. It still needs testing and is surely not solving all
issues there are, but I find it nicer than the partially redundant code
we have right now.
My motivation for this was to maybe improve parser's ability to handle
lack of spaces in input. I still see the scanner fall into the generic
"string" token case which requires manual dissection in the parser.
What is your motivation for the above changes? Maybe we could collect
parser limitations around these units and see what helps "the most"?
Cheers, Phil
