From: Phil Sutter Date: Wed, 12 Nov 2025 23:03:37 +0000 (+0100) Subject: mergesort: Fix sorting of string values X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=340d904a974ed206fcc4d8ca32540762fd8e59e0;p=thirdparty%2Fnftables.git mergesort: Fix sorting of string values Sorting order was obviously wrong, e.g. "ppp0" ordered before "eth1". Moreover, this happened on Little Endian only so sorting order actually depended on host's byteorder. By reimporting string values as Big Endian, both issues are fixed: On one hand, GMP-internal byteorder no longer depends on host's byteorder, on the other comparing strings really starts with the first character, not the last. Fixes: 14ee0a979b622 ("src: sort set elements in netlink_get_setelems()") Signed-off-by: Phil Sutter --- diff --git a/src/mergesort.c b/src/mergesort.c index a9cba614..97e36917 100644 --- a/src/mergesort.c +++ b/src/mergesort.c @@ -37,6 +37,13 @@ static mpz_srcptr expr_msort_value(const struct expr *expr, mpz_t value) case EXPR_RANGE: return expr_msort_value(expr->left, value); case EXPR_VALUE: + if (expr_basetype(expr)->type == TYPE_STRING) { + char buf[expr->len]; + + mpz_export_data(buf, expr->value, BYTEORDER_HOST_ENDIAN, expr->len); + mpz_import_data(value, buf, BYTEORDER_BIG_ENDIAN, expr->len); + return value; + } return expr->value; case EXPR_RANGE_VALUE: return expr->range.low; diff --git a/tests/py/any/meta.t.json.output b/tests/py/any/meta.t.json.output index 8f4d597a..4454bb96 100644 --- a/tests/py/any/meta.t.json.output +++ b/tests/py/any/meta.t.json.output @@ -233,60 +233,6 @@ } ] -# meta iifname {"dummy0", "lo"} -[ - { - "match": { - "left": { - "meta": { "key": "iifname" } - }, - "op": "==", - "right": { - "set": [ - "lo", - "dummy0" - ] - } - } - } -] - -# meta iifname != {"dummy0", "lo"} -[ - { - "match": { - "left": { - "meta": { "key": "iifname" } - }, - "op": "!=", - "right": { - "set": [ - "lo", - "dummy0" - ] - } - } - } -] - -# meta oifname { "dummy0", "lo"} -[ - { - "match": { - "left": { - "meta": { "key": "oifname" } - }, - "op": "==", - "right": { - "set": [ - "lo", - "dummy0" - ] - } - } - } -] - # meta skuid {"bin", "root", "daemon"} accept [ { diff --git a/tests/py/any/queue.t.json.output b/tests/py/any/queue.t.json.output index ea372238..90670cc9 100644 --- a/tests/py/any/queue.t.json.output +++ b/tests/py/any/queue.t.json.output @@ -104,11 +104,11 @@ 0 ], [ - "ppp0", + "eth1", 2 ], [ - "eth1", + "ppp0", 2 ] ] diff --git a/tests/py/inet/osf.t.json.output b/tests/py/inet/osf.t.json.output index 922e395f..77ca7e30 100644 --- a/tests/py/inet/osf.t.json.output +++ b/tests/py/inet/osf.t.json.output @@ -18,6 +18,26 @@ } ] +# osf version { "Windows:XP", "MacOs:Sierra" } +[ + { + "match": { + "left": { + "osf": { + "key": "version" + } + }, + "op": "==", + "right": { + "set": [ + "MacOs:Sierra", + "Windows:XP" + ] + } + } + } +] + # ct mark set osf name map { "Windows" : 0x00000001, "MacOs" : 0x00000002 } [ { @@ -51,3 +71,37 @@ } } ] + +# ct mark set osf version map { "Windows:XP" : 0x00000003, "MacOs:Sierra" : 0x00000004 } +[ + { + "mangle": { + "key": { + "ct": { + "key": "mark" + } + }, + "value": { + "map": { + "data": { + "set": [ + [ + "MacOs:Sierra", + 4 + ], + [ + "Windows:XP", + 3 + ] + ] + }, + "key": { + "osf": { + "key": "version" + } + } + } + } + } + } +] diff --git a/tests/shell/testcases/maps/dumps/0012map_0.json-nft b/tests/shell/testcases/maps/dumps/0012map_0.json-nft index 2892e11d..6c885703 100644 --- a/tests/shell/testcases/maps/dumps/0012map_0.json-nft +++ b/tests/shell/testcases/maps/dumps/0012map_0.json-nft @@ -32,21 +32,21 @@ "map": "verdict", "elem": [ [ - "lo", + "eth0", { - "accept": null + "drop": null } ], [ - "eth0", + "eth1", { "drop": null } ], [ - "eth1", + "lo", { - "drop": null + "accept": null } ] ] @@ -69,21 +69,21 @@ "data": { "set": [ [ - "lo", + "eth0", { - "accept": null + "drop": null } ], [ - "eth0", + "eth1", { "drop": null } ], [ - "eth1", + "lo", { - "drop": null + "accept": null } ] ] diff --git a/tests/shell/testcases/maps/dumps/0012map_0.nft b/tests/shell/testcases/maps/dumps/0012map_0.nft index e734fc1c..0df329a5 100644 --- a/tests/shell/testcases/maps/dumps/0012map_0.nft +++ b/tests/shell/testcases/maps/dumps/0012map_0.nft @@ -1,12 +1,12 @@ table ip x { map z { type ifname : verdict - elements = { "lo" : accept, - "eth0" : drop, - "eth1" : drop } + elements = { "eth0" : drop, + "eth1" : drop, + "lo" : accept } } chain y { - iifname vmap { "lo" : accept, "eth0" : drop, "eth1" : drop } + iifname vmap { "eth0" : drop, "eth1" : drop, "lo" : accept } } } diff --git a/tests/shell/testcases/maps/dumps/named_ct_objects.json-nft b/tests/shell/testcases/maps/dumps/named_ct_objects.json-nft index c0f270e3..34c8798d 100644 --- a/tests/shell/testcases/maps/dumps/named_ct_objects.json-nft +++ b/tests/shell/testcases/maps/dumps/named_ct_objects.json-nft @@ -195,8 +195,8 @@ }, "handle": 0, "elem": [ - "sip", - "ftp" + "ftp", + "sip" ] } }, diff --git a/tests/shell/testcases/maps/dumps/named_ct_objects.nft b/tests/shell/testcases/maps/dumps/named_ct_objects.nft index 59f18932..dab683bf 100644 --- a/tests/shell/testcases/maps/dumps/named_ct_objects.nft +++ b/tests/shell/testcases/maps/dumps/named_ct_objects.nft @@ -50,8 +50,8 @@ table inet t { set helpname { typeof ct helper - elements = { "sip", - "ftp" } + elements = { "ftp", + "sip" } } chain y { diff --git a/tests/shell/testcases/sets/dumps/sets_with_ifnames.json-nft b/tests/shell/testcases/sets/dumps/sets_with_ifnames.json-nft index ac428429..7b4849e0 100644 --- a/tests/shell/testcases/sets/dumps/sets_with_ifnames.json-nft +++ b/tests/shell/testcases/sets/dumps/sets_with_ifnames.json-nft @@ -260,8 +260,8 @@ }, "right": { "set": [ - "eth0", - "abcdef0" + "abcdef0", + "eth0" ] } } diff --git a/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft b/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft index 77a8baf5..8abca03a 100644 --- a/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft +++ b/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft @@ -39,7 +39,7 @@ table inet testifsets { chain v4icmp { iifname @simple counter packets 0 bytes 0 iifname @simple_wild counter packets 0 bytes 0 - iifname { "eth0", "abcdef0" } counter packets 0 bytes 0 + iifname { "abcdef0", "eth0" } counter packets 0 bytes 0 iifname { "abcdef*", "eth0" } counter packets 0 bytes 0 iifname vmap @map_wild }