JSON: Sort out rule position and handles in general

First of all, don't print the position property when listing rules. This
was there only because libnftnl JSON output has it too, but since the
preferred way to *add* a rule at some location is via 'handle' keyword,
keeping "position" in output would be non-intuitive. Changing "position"
property name to "handle" instead is also a no-go since that would clash
with the real rule handle.

Secondly, turn all handle output on regardless of octx->handle setting.
For a programmatic API like JSON, this should be fine.

Thirdly, fix rule locations when parsing JSON: Respect "handle" property
for CMD_INSERT and CMD_ADD and ignore "pos" at all (actually even a
typo, should have read "position"). Also support "index" property
recently added to standard syntax.

Finally, adjust nft-test.py for the above changes: There is no
"position" property to drop from rule output, and "handle" property will
always be present.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/json.c b/src/json.c
index 1b73b919d5df2758b280665d04d8dc9b3d4535b3..622a10b30c580a6cd81dab792f07c66df05453bc 100644 (file)
--- a/src/json.c
+++ b/src/json.c
@@ -89,14 +89,12 @@ static json_t *set_print_json(struct output_ctx *octx, const struct set *set)
                type = "set";
        }
 
-       root = json_pack("{s:s, s:s, s:s, s:o}",
+       root = json_pack("{s:s, s:s, s:s, s:o, s:I}",
                        "family", family2str(set->handle.family),
                        "name", set->handle.set.name,
                        "table", set->handle.table.name,
-                       "type", set_dtype_json(set->key));
-       if (octx->handle)
-               json_object_set_new(root, "handle",
-                                   json_integer(set->handle.handle.id));
+                       "type", set_dtype_json(set->key),
+                       "handle", set->handle.handle.id);
        if (datatype_ext)
                json_object_set_new(root, "map", json_string(datatype_ext));
 
@@ -181,10 +179,7 @@ static json_t *rule_print_json(struct output_ctx *octx,
                         "family", family2str(rule->handle.family),
                         "table", rule->handle.table.name,
                         "chain", rule->handle.chain.name,
-                        "position", rule->handle.position.id);
-       if (octx->handle)
-               json_object_set_new(root, "handle",
-                                   json_integer(rule->handle.handle.id));
+                        "handle", rule->handle.handle.id);
        if (rule->comment)
                json_object_set_new(root, "comment",
                                    json_string(rule->comment));
@@ -208,13 +203,11 @@ static json_t *chain_print_json(const struct output_ctx *octx,
 {
        json_t *root, *tmp;
 
-       root = json_pack("{s:s, s:s, s:s}",
+       root = json_pack("{s:s, s:s, s:s, s:I}",
                         "family", family2str(chain->handle.family),
                         "table", chain->handle.table.name,
-                        "name", chain->handle.chain.name);
-       if (octx->handle)
-               json_object_set_new(root, "handle",
-                                   json_integer(chain->handle.handle.id));
+                        "name", chain->handle.chain.name,
+                        "handle", chain->handle.handle.id);
 
        if (chain->flags & CHAIN_F_BASECHAIN) {
                tmp = json_pack("{s:s, s:s, s:i, s:s}",
@@ -248,13 +241,11 @@ static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj)
        const char *unit;
        uint64_t rate;
 
-       root = json_pack("{s:s, s:s, s:s}",
+       root = json_pack("{s:s, s:s, s:s, s:I}",
                        "family", family2str(obj->handle.family),
                        "name", obj->handle.obj.name,
-                       "table", obj->handle.table.name);
-       if (octx->handle)
-               json_object_set_new(root, "handle",
-                                   json_integer(obj->handle.handle.id));
+                       "table", obj->handle.table.name,
+                       "handle", obj->handle.handle.id);
 
        switch (obj->type) {
        case NFT_OBJECT_COUNTER:
@@ -374,12 +365,10 @@ static json_t *table_print_json(const struct output_ctx *octx,
 {
        json_t *root, *tmp;
 
-       root = json_pack("{s:s, s:s}",
+       root = json_pack("{s:s, s:s, s:I}",
                         "family", family2str(table->handle.family),
-                        "name", table->handle.table.name);
-       if (octx->handle)
-               json_object_set_new(root, "handle",
-                                   json_integer(table->handle.handle.id));
+                        "name", table->handle.table.name,
+                        "handle", table->handle.handle.id);
 
        tmp = table_flags_json(table);
        if (tmp)

diff --git a/src/parser_json.c b/src/parser_json.c
index 60929386be4da9524e5e524ef8ca3b93bba57af1..1c5994f811e30ecbfd4e79f2d6df685c2f55bc39 100644 (file)
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2770,14 +2770,18 @@ static struct cmd *json_parse_cmd_replace(struct json_ctx *ctx,
                            "chain", &h.chain.name,
                            "expr", &tmp))
                return NULL;
+       json_unpack(root, "{s:I}", "handle", &h.handle.id);
+       json_unpack(root, "{s:I}", "index", &h.index.id);
 
-       if (op == CMD_REPLACE &&
-           json_unpack_err(ctx, root, "{s:I}", "handle", &h.handle.id))
+       if (op == CMD_REPLACE && !h.handle.id) {
+               json_error(ctx, "Handle is required when replacing a rule.");
                return NULL;
+       }
 
-       if (op == CMD_INSERT &&
-           json_unpack_err(ctx, root, "{s:i}", "pos", &h.position.id))
-               return NULL;
+       if ((op == CMD_INSERT || op == CMD_ADD) && h.handle.id) {
+               h.position.id = h.handle.id;
+               h.handle.id = 0;
+       }
 
        h.family = parse_family(family);
        if (h.family < 0) {

diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
index edc0b4b11daf73518e151bfe0590ff6374cc689f..c02294ac545629f4afc1feb0f375d03b98c89aac 100755 (executable)
--- a/tests/py/nft-test.py
+++ b/tests/py/nft-test.py
@@ -855,10 +855,7 @@ def rule_add(rule, filename, lineno, force_all_family_option, filename_path):
                 json_output = json.loads(json_output)
                 for item in json_output["nftables"]:
                     if "rule" in item:
-                        if "handle" in item["rule"]:
-                            del(item["rule"]["handle"])
-                        if "position" in item["rule"]:
-                            del(item["rule"]["position"])
+                        del(item["rule"]["handle"])
                         json_output = item["rule"]
                         break
                 json_input = json.dumps(json_output["expr"], sort_keys = True)
@@ -917,10 +914,7 @@ def rule_add(rule, filename, lineno, force_all_family_option, filename_path):
             json_output = json.loads(json_output)
             for item in json_output["nftables"]:
                 if "rule" in item:
-                    if "handle" in item["rule"]:
-                        del(item["rule"]["handle"])
-                    if "position" in item["rule"]:
-                        del(item["rule"]["position"])
+                    del(item["rule"]["handle"])
                     json_output = item["rule"]
                     break
             json_output = json.dumps(json_output["expr"], sort_keys = True)