This patch fixes JSON-based rule positioning when using "add rule" with a handle parameter. Previously, the handle was deleted before being used for positioning, causing rules to always be appended at the end of the chain instead of being placed after the specified rule handle. The fix follows the same pattern used in json_parse_cmd_replace(): - Parse the handle field from JSON - Convert handle to position for CMD_ADD operations - Remove the code that was deleting the handle field With NLM_F_APPEND set (as it always is for add operations), the kernel interprets position as "add after this handle", which matches the CLI behavior of "add rule position X". Before this fix: nft -j add rule ... handle 2 --> rule added at end After this fix: nft -j add rule ... handle 2 --> rule added after handle 2 The CLI version (nft add rule ... position X) was already working correctly. Tested with: # nft add table inet test # nft add chain inet test c # nft add rule inet test c tcp dport 80 accept # nft add rule inet test c tcp dport 443 accept # echo '{"nftables":[{"add":{"rule":{"family":"inet","table":"test","chain":"c","handle":2,"expr":[{"match":{"left":{"payload":{"protocol":"tcp","field":"dport"}},"op":"==","right":8080}},{"accept":null}]}}}]}' | nft -j -f - # nft -a list table inet test Result: Rule with port 8080 correctly placed after handle 2 (port 80). Signed-off-by: Alexandre Knecht --- src/parser_json.c | 11 +- .../json/0007rule_add_handle_position_0 | 22 ++++ .../0007rule_add_handle_position_0.json-nft | 101 ++++++++++++++++++ .../dumps/0007rule_add_handle_position_0.nft | 7 ++ 4 files changed, 138 insertions(+), 3 deletions(-) create mode 100755 tests/shell/testcases/json/0007rule_add_handle_position_0 create mode 100644 tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.json-nft create mode 100644 tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.nft diff --git a/src/parser_json.c b/src/parser_json.c index 7b4f3384..c974a9e2 100644 --- a/src/parser_json.c +++ b/src/parser_json.c @@ -3197,10 +3197,18 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root, return NULL; } + /* Parse handle and index (similar to json_parse_cmd_replace) */ + json_unpack(root, "{s:I}", "handle", &h.handle.id); if (!json_unpack(root, "{s:I}", "index", &h.index.id)) { h.index.id++; } + /* For CMD_ADD, convert handle to position for rule positioning */ + if ((op == CMD_ADD || op == CMD_CREATE) && h.handle.id) { + h.position.id = h.handle.id; + h.handle.id = 0; + } + rule = rule_alloc(int_loc, NULL); json_unpack(root, "{s:s}", "comment", &comment); @@ -3226,9 +3234,6 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root, rule_stmt_append(rule, stmt); } - if (op == CMD_ADD) - json_object_del(root, "handle"); - return cmd_alloc(op, obj, &h, int_loc, rule); err_free_rule: diff --git a/tests/shell/testcases/json/0007rule_add_handle_position_0 b/tests/shell/testcases/json/0007rule_add_handle_position_0 new file mode 100755 index 00000000..e7dde7c7 --- /dev/null +++ b/tests/shell/testcases/json/0007rule_add_handle_position_0 @@ -0,0 +1,22 @@ +#!/bin/bash + +# NFT_TEST_REQUIRES(NFT_TEST_HAVE_json) + +# Test that JSON add rule with handle parameter positions rule correctly + +set -e + +$NFT flush ruleset + +# Create table, chain, and two initial rules +# The first rule (port 80) will get handle 2 +# The second rule (port 443) will get handle 3 +SETUP='{"nftables": [{"add": {"table": {"family": "inet", "name": "test"}}}, {"add": {"chain": {"family": "inet", "table": "test", "name": "c"}}}, {"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 80}}, {"accept": null}]}}}, {"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 443}}, {"accept": null}]}}}]}' + +$NFT -j -f - <<< "$SETUP" + +# Add a new rule positioned after handle 2 (the port 80 rule) +# This rule should end up between port 80 and port 443 +ADD_RULE='{"nftables": [{"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": 2, "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 8080}}, {"accept": null}]}}}]}' + +$NFT -j -f - <<< "$ADD_RULE" diff --git a/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.json-nft b/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.json-nft new file mode 100644 index 00000000..667e14c1 --- /dev/null +++ b/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.json-nft @@ -0,0 +1,101 @@ +{ + "nftables": [ + { + "metainfo": { + "version": "VERSION", + "release_name": "RELEASE_NAME", + "json_schema_version": 1 + } + }, + { + "table": { + "family": "inet", + "name": "test", + "handle": 0 + } + }, + { + "chain": { + "family": "inet", + "table": "test", + "name": "c", + "handle": 0 + } + }, + { + "rule": { + "family": "inet", + "table": "test", + "chain": "c", + "handle": 0, + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "tcp", + "field": "dport" + } + }, + "right": 80 + } + }, + { + "accept": null + } + ] + } + }, + { + "rule": { + "family": "inet", + "table": "test", + "chain": "c", + "handle": 0, + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "tcp", + "field": "dport" + } + }, + "right": 8080 + } + }, + { + "accept": null + } + ] + } + }, + { + "rule": { + "family": "inet", + "table": "test", + "chain": "c", + "handle": 0, + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "tcp", + "field": "dport" + } + }, + "right": 443 + } + }, + { + "accept": null + } + ] + } + } + ] +} diff --git a/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.nft b/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.nft new file mode 100644 index 00000000..d3e8e1ba --- /dev/null +++ b/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.nft @@ -0,0 +1,7 @@ +table inet test { + chain c { + tcp dport 80 accept + tcp dport 8080 accept + tcp dport 443 accept + } +} -- 2.51.0