[nft PATCH v2] evaluate: Accept ranges of N-N

[nft PATCH v2] evaluate: Accept ranges of N-N

[Phil Sutter]

Trying to add a range of size 1 was previously not allowed:

| # nft add element ip t s '{ 40-40 }'
| Error: Range has zero or negative size
| add element ip t s { 40-40 }
|                      ^^^^^

The error message is not correct: If a range 40-41 is of size 2 (it
contains elements 40 and 41), a range 40-40 must be of size 1.

Handling this is even supported already: If a single element is added to
an interval set, it is converted into just this range. The implication
is that on output, previous input of '40-40' is indistinguishable from
single element input '40'.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1312
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Simplify fix (actual merging neither necessary nor beneficial), adjust
  commit message accordingly.
- Add test case.
---
 src/evaluate.c                                |  4 +--
 .../sets/0036single_item_intervals_0          | 35 +++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100755 tests/shell/testcases/sets/0036single_item_intervals_0

diff --git a/src/evaluate.c b/src/evaluate.c
index 8086f750417a7..d2a4914f2ec39 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -951,9 +951,9 @@ static int expr_evaluate_range(struct eval_ctx *ctx, struct expr **expr)
 		return -1;
 	right = range->right;
 
-	if (mpz_cmp(left->value, right->value) >= 0)
+	if (mpz_cmp(left->value, right->value) > 0)
 		return expr_error(ctx->msgs, range,
-				  "Range has zero or negative size");
+				  "Range has negative size");
 
 	datatype_set(range, left->dtype);
 	range->flags |= EXPR_F_CONSTANT;
diff --git a/tests/shell/testcases/sets/0036single_item_intervals_0 b/tests/shell/testcases/sets/0036single_item_intervals_0
new file mode 100755
index 0000000000000..237ad6dd98e5a
--- /dev/null
+++ b/tests/shell/testcases/sets/0036single_item_intervals_0
@@ -0,0 +1,35 @@
+#!/bin/bash
+
+set -e
+
+RULESET="table inet t {
+	set s {
+		type ipv4_addr
+		flags interval
+	}
+}"
+
+$NFT -f - <<< "$RULESET"
+$NFT add element inet t s '{ 1.1.1.1-1.1.1.1 }'
+$NFT add element inet t s '{ 2.2.2.2-2.2.2.2 }'
+
+$NFT create element inet t s '{ 1.1.1.1 }' && exit 1
+$NFT create element inet t s '{ 1.1.1.1-1.1.1.1 }' && exit 1
+
+EXPECT="table inet t {
+	set s {
+		type ipv4_addr
+		flags interval
+		elements = { 1.1.1.1, 2.2.2.2 }
+	}
+}"
+GET=$($NFT list set inet t s)
+
+if [ "$EXPECT" != "$GET" ]; then
+	DIFF="$(which diff)"
+	[ -x "$DIFF" ] && $DIFF -u <(echo "$EXPECT") <(echo "$GET")
+	exit 1
+fi
+
+$NFT delete element inet t s '{ 2.2.2.2 }'
+$NFT delete element inet t s '{ 1.1.1.1-1.1.1.1 }'
-- 
2.21.0

Re: [nft PATCH v2] evaluate: Accept ranges of N-N

[Pablo Neira Ayuso]

On Fri, Jul 05, 2019 at 02:15:05PM +0200, Phil Sutter wrote:
> Trying to add a range of size 1 was previously not allowed:
> 
> | # nft add element ip t s '{ 40-40 }'
> | Error: Range has zero or negative size
> | add element ip t s { 40-40 }
> |                      ^^^^^
> 
> The error message is not correct: If a range 40-41 is of size 2 (it
> contains elements 40 and 41), a range 40-40 must be of size 1.
> 
> Handling this is even supported already: If a single element is added to
> an interval set, it is converted into just this range. The implication
> is that on output, previous input of '40-40' is indistinguishable from
> single element input '40'.

What kind of human is going to generate such range? :-)

I think we can place this item in the "nft ruleset optimization"
discussion during the NFWS.

Re: [nft PATCH v2] evaluate: Accept ranges of N-N

[Phil Sutter]

On Fri, Jul 05, 2019 at 02:58:47PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Jul 05, 2019 at 02:15:05PM +0200, Phil Sutter wrote:
> > Trying to add a range of size 1 was previously not allowed:
> > 
> > | # nft add element ip t s '{ 40-40 }'
> > | Error: Range has zero or negative size
> > | add element ip t s { 40-40 }
> > |                      ^^^^^
> > 
> > The error message is not correct: If a range 40-41 is of size 2 (it
> > contains elements 40 and 41), a range 40-40 must be of size 1.
> > 
> > Handling this is even supported already: If a single element is added to
> > an interval set, it is converted into just this range. The implication
> > is that on output, previous input of '40-40' is indistinguishable from
> > single element input '40'.
> 
> What kind of human is going to generate such range? :-)

According to the ticket, some scripts do. :)

> I think we can place this item in the "nft ruleset optimization"
> discussion during the NFWS.

Sure, I'll add a note to the proposal.

Thanks, Phil