This patch enables handle-based rule positioning for JSON commands by distinguishing between explicit and implicit "add" operations: - Explicit commands ({"add": {"rule": ...}}) route through json_parse_cmd_replace, which converts the handle field to position, enabling rules to be inserted at specific locations - Implicit commands ({"rule": ...} from export/import) continue using json_parse_cmd_add, which ignores handles for backward compatibility The semantics are: ADD with handle: inserts rule AFTER the specified handle INSERT with handle: inserts rule BEFORE the specified handle This maintains export/import compatibility while enabling programmatic rule positioning via the JSON API. Includes comprehensive test cases covering: - Multiple additions at the same handle position - INSERT before handle - Error handling for deleted handles - Export/import compatibility (implicit adds ignore handles) Link: https://patchwork.ozlabs.org/project/netfilter-devel/patch/20251029224530.1962783-2-knecht.alexandre@gmail.com/ Suggested-by: Phil Sutter Suggested-by: Florian Westphal Signed-off-by: Alexandre Knecht --- src/parser_json.c | 7 +- .../0008rule_add_position_comprehensive_0 | 135 ++++++++++++++++++ 2 files changed, 139 insertions(+), 3 deletions(-) create mode 100755 tests/shell/testcases/json/0008rule_add_position_comprehensive_0 diff --git a/src/parser_json.c b/src/parser_json.c index 7b4f3384..b9b3bef0 100644 --- a/src/parser_json.c +++ b/src/parser_json.c @@ -4045,7 +4045,8 @@ static struct cmd *json_parse_cmd_replace(struct json_ctx *ctx, return NULL; } - if ((op == CMD_INSERT || op == CMD_ADD) && h.handle.id) { + if (h.handle.id && + (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE)) { h.position.id = h.handle.id; h.handle.id = 0; } @@ -4328,9 +4329,9 @@ static struct cmd *json_parse_cmd(struct json_ctx *ctx, json_t *root) enum cmd_ops op; struct cmd *(*cb)(struct json_ctx *ctx, json_t *, enum cmd_ops); } parse_cb_table[] = { - { "add", CMD_ADD, json_parse_cmd_add }, + { "add", CMD_ADD, json_parse_cmd_replace }, { "replace", CMD_REPLACE, json_parse_cmd_replace }, - { "create", CMD_CREATE, json_parse_cmd_add }, + { "create", CMD_CREATE, json_parse_cmd_replace }, { "insert", CMD_INSERT, json_parse_cmd_replace }, { "delete", CMD_DELETE, json_parse_cmd_add }, { "list", CMD_LIST, json_parse_cmd_list }, diff --git a/tests/shell/testcases/json/0008rule_add_position_comprehensive_0 b/tests/shell/testcases/json/0008rule_add_position_comprehensive_0 new file mode 100755 index 00000000..171de992 --- /dev/null +++ b/tests/shell/testcases/json/0008rule_add_position_comprehensive_0 @@ -0,0 +1,135 @@ +#!/bin/bash + +# NFT_TEST_REQUIRES(NFT_TEST_HAVE_json) + +# Comprehensive test for JSON rule positioning with handle parameter +# Tests the scenarios requested by Florian Westphal + +set -e + +$NFT flush ruleset + +# Setup: create table and chain +$NFT add table inet test +$NFT add chain inet test c + +# Add 3 initial rules to get handles 2, 3, 4 +$NFT add rule inet test c tcp dport 1111 accept # will be handle 2 +$NFT add rule inet test c tcp dport 2222 accept # will be handle 3 +$NFT add rule inet test c tcp dport 3333 accept # will be handle 4 + +# Test 1: Positioning at handle 2 should result in: rule2, newRule, rule3, rule4 +echo "Test 1: Add rule after handle 2" +$NFT -j -f - << 'EOF' +{"nftables": [{"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": 2, "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 9991}}, {"accept": null}]}}}]} +EOF + +EXPECTED="table inet test { + chain c { + tcp dport 1111 accept + tcp dport 9991 accept + tcp dport 2222 accept + tcp dport 3333 accept + } +}" + +GET="$($NFT list ruleset)" +if [ "$EXPECTED" != "$GET" ]; then + echo "Test 1 failed: rule not positioned correctly after handle 2" + $DIFF -u <(echo "$EXPECTED") <(echo "$GET") + exit 1 +fi + +# Test 2: Positioning at handle 2 again should result in: rule2, newRule2, newRule, rule3, rule4 +echo "Test 2: Add another rule after handle 2 (should be after original handle 2, not the new rule)" +$NFT -j -f - << 'EOF' +{"nftables": [{"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": 2, "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 9992}}, {"accept": null}]}}}]} +EOF + +EXPECTED="table inet test { + chain c { + tcp dport 1111 accept + tcp dport 9992 accept + tcp dport 9991 accept + tcp dport 2222 accept + tcp dport 3333 accept + } +}" + +GET="$($NFT list ruleset)" +if [ "$EXPECTED" != "$GET" ]; then + echo "Test 2 failed: second rule not positioned correctly after handle 2" + $DIFF -u <(echo "$EXPECTED") <(echo "$GET") + exit 1 +fi + +# Test 3: Positioning at a deleted handle should fail +$NFT flush ruleset +$NFT add table inet test +$NFT add chain inet test c +$NFT add rule inet test c tcp dport 1111 accept # handle 2 +$NFT add rule inet test c tcp dport 2222 accept # handle 3 +$NFT delete rule inet test c handle 2 + +echo "Test 3: Positioning at deleted handle should fail" +if $NFT -j -f - 2>/dev/null << 'EOF' +{"nftables": [{"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": 2, "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 9993}}, {"accept": null}]}}}]} +EOF +then + echo "Test 3 failed: should have failed when positioning at deleted handle" + exit 1 +fi + +# Test 4: INSERT before handle +$NFT flush ruleset +$NFT add table inet test +$NFT add chain inet test c +$NFT add rule inet test c tcp dport 1111 accept # handle 2 +$NFT add rule inet test c tcp dport 2222 accept # handle 3 +$NFT add rule inet test c tcp dport 3333 accept # handle 4 + +echo "Test 4: Insert rule before handle 3" +$NFT -j -f - << 'EOF' +{"nftables": [{"insert": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": 3, "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 9994}}, {"accept": null}]}}}]} +EOF + +EXPECTED="table inet test { + chain c { + tcp dport 1111 accept + tcp dport 9994 accept + tcp dport 2222 accept + tcp dport 3333 accept + } +}" + +GET="$($NFT list ruleset)" +if [ "$EXPECTED" != "$GET" ]; then + echo "Test 4 failed: rule not inserted correctly before handle 3" + $DIFF -u <(echo "$EXPECTED") <(echo "$GET") + exit 1 +fi + +# Test 5: Verify implicit add (export format) ignores handles +echo "Test 5: Implicit add should ignore handles (export/import compatibility)" +$NFT flush ruleset + +# This is export format (no "add" wrapper) - handles should be ignored +$NFT -j -f - << 'EOF' +{"nftables": [{"table": {"family": "inet", "name": "test"}}, {"chain": {"family": "inet", "table": "test", "name": "c"}}, {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": 999, "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 5555}}, {"accept": null}]}}]} +EOF + +# Should succeed even though handle 999 doesn't exist (because it's ignored) +EXPECTED="table inet test { + chain c { + tcp dport 5555 accept + } +}" + +GET="$($NFT list ruleset)" +if [ "$EXPECTED" != "$GET" ]; then + echo "Test 5 failed: implicit add should have ignored handle" + $DIFF -u <(echo "$EXPECTED") <(echo "$GET") + exit 1 +fi + +echo "All tests passed!" -- 2.51.0