On Tue, Aug 04, 2020 at 12:38:46PM +0200, Jose M. Guisado Gomez wrote: > This patch fixes a bug in which nft did not print any output when > specifying --echo and --json and reading nft native syntax. > > This patch respects behavior when input is json, in which the output > would be the identical input plus the handles. > > Adds a json_echo member inside struct nft_ctx to build and store the json object > containing the json command objects, the object is built using a mock > monitor to reuse monitor json code. This json object is only used when > we are sure we have not read json from input. > > Fixes: https://bugzilla.netfilter.org/show_bug.cgi?id=1446 > > Signed-off-by: Jose M. Guisado Gomez <guigom@xxxxxxxxxx> [...] > diff --git a/src/monitor.c b/src/monitor.c > index 3872ebcf..868e31b5 100644 > --- a/src/monitor.c > +++ b/src/monitor.c > @@ -221,12 +221,14 @@ static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type, > if (nft_output_handle(&monh->ctx->nft->output)) > nft_mon_print(monh, " # handle %" PRIu64 "", > t->handle.handle.id); > + nft_mon_print(monh, "\n"); > break; > case NFTNL_OUTPUT_JSON: > monitor_print_table_json(monh, cmd, t); > + if(!nft_output_echo(&monh->ctx->nft->output)) > + nft_mon_print(monh, "\n"); > break; > } > - nft_mon_print(monh, "\n"); > table_free(t); > nftnl_table_free(nlt); > return MNL_CB_OK; > @@ -258,12 +260,14 @@ static int netlink_events_chain_cb(const struct nlmsghdr *nlh, int type, > c->handle.chain.name); > break; > } > + nft_mon_print(monh, "\n"); > break; > case NFTNL_OUTPUT_JSON: > monitor_print_chain_json(monh, cmd, c); > + if(!nft_output_echo(&monh->ctx->nft->output)) > + nft_mon_print(monh, "\n"); > break; > } > - nft_mon_print(monh, "\n"); > chain_free(c); > nftnl_chain_free(nlc); > return MNL_CB_OK; > @@ -304,12 +308,14 @@ static int netlink_events_set_cb(const struct nlmsghdr *nlh, int type, > set->handle.set.name); > break; > } > + nft_mon_print(monh, "\n"); > break; > case NFTNL_OUTPUT_JSON: > monitor_print_set_json(monh, cmd, set); > + if(!nft_output_echo(&monh->ctx->nft->output)) > + nft_mon_print(monh, "\n"); > break; > } > - nft_mon_print(monh, "\n"); > set_free(set); > out: > nftnl_set_free(nls); > @@ -441,6 +447,7 @@ static int netlink_events_setelem_cb(const struct nlmsghdr *nlh, int type, > nft_mon_print(monh, "%s element %s %s %s ", > cmd, family2str(family), table, setname); > expr_print(dummyset->init, &monh->ctx->nft->output); > + nft_mon_print(monh, "\n"); > break; > case NFTNL_OUTPUT_JSON: > dummyset->handle.family = family; > @@ -450,9 +457,10 @@ static int netlink_events_setelem_cb(const struct nlmsghdr *nlh, int type, > /* prevent set_free() from trying to free those */ > dummyset->handle.set.name = NULL; > dummyset->handle.table.name = NULL; > + if(!nft_output_echo(&monh->ctx->nft->output)) ^ nitpick: 'if' is not a function, add space between if and parens. > + nft_mon_print(monh, "\n"); > break; > } > - nft_mon_print(monh, "\n"); > set_free(dummyset); > out: > nftnl_set_free(nls); > @@ -492,12 +500,14 @@ static int netlink_events_obj_cb(const struct nlmsghdr *nlh, int type, > obj->handle.obj.name); > break; > } > + nft_mon_print(monh, "\n"); > break; > case NFTNL_OUTPUT_JSON: > monitor_print_obj_json(monh, cmd, obj); > + if(!nft_output_echo(&monh->ctx->nft->output)) ^ same here and everywhere else. > + nft_mon_print(monh, "\n"); > break; > } > - nft_mon_print(monh, "\n"); > obj_free(obj); > nftnl_obj_free(nlo); > return MNL_CB_OK; > @@ -542,12 +552,14 @@ static int netlink_events_rule_cb(const struct nlmsghdr *nlh, int type, > r->handle.handle.id); > break; > } > + nft_mon_print(monh, "\n"); > break; > case NFTNL_OUTPUT_JSON: > monitor_print_rule_json(monh, cmd, r); > + if(!nft_output_echo(&monh->ctx->nft->output)) > + nft_mon_print(monh, "\n"); > break; > } > - nft_mon_print(monh, "\n"); > rule_free(r); > nftnl_rule_free(nlr); > return MNL_CB_OK; > @@ -912,6 +924,8 @@ int netlink_echo_callback(const struct nlmsghdr *nlh, void *data) > { > struct netlink_cb_data *nl_cb_data = data; > struct netlink_ctx *ctx = nl_cb_data->nl_ctx; > + struct nft_ctx *nft = ctx->nft; > + > struct netlink_mon_handler echo_monh = { > .format = NFTNL_OUTPUT_DEFAULT, > .ctx = ctx, > @@ -922,8 +936,15 @@ int netlink_echo_callback(const struct nlmsghdr *nlh, void *data) > if (!nft_output_echo(&echo_monh.ctx->nft->output)) > return MNL_CB_OK; > > - if (nft_output_json(&ctx->nft->output)) > - return json_events_cb(nlh, &echo_monh); > + if (nft_output_json(&nft->output)) { > + if (!nft->json_root) { > + nft->json_echo = json_array(); > + if (!nft->json_echo) > + memory_allocation_error(); > + echo_monh.format = NFTNL_OUTPUT_JSON; > + } else Nitpick: Use curly brace '{' here in the else side of the branch for consistency (even if it's only on single line). > + return json_events_cb(nlh, &echo_monh); > + } > > return netlink_events_cb(nlh, &echo_monh); > } > diff --git a/src/parser_json.c b/src/parser_json.c > index 59347168..ef33063d 100644 > --- a/src/parser_json.c > +++ b/src/parser_json.c > @@ -3884,11 +3884,21 @@ int json_events_cb(const struct nlmsghdr *nlh, struct netlink_mon_handler *monh) > > void json_print_echo(struct nft_ctx *ctx) > { > - if (!ctx->json_root) > - return; > - > - json_dumpf(ctx->json_root, ctx->output.output_fp, JSON_PRESERVE_ORDER); > - json_cmd_assoc_free(); > - json_decref(ctx->json_root); > - ctx->json_root = NULL; > + if (!ctx->json_root) { > + if (!ctx->json_echo) > + return; > + else { > + ctx->json_echo = json_pack("{s:o}", "nftables", ctx->json_echo); > + json_dumpf(ctx->json_echo, ctx->output.output_fp, JSON_PRESERVE_ORDER); > + json_decref(ctx->json_echo); > + ctx->json_echo = NULL; > + fprintf(ctx->output.output_fp, "\n"); > + fflush(ctx->output.output_fp); > + } > + } else { > + json_dumpf(ctx->json_root, ctx->output.output_fp, JSON_PRESERVE_ORDER); > + json_cmd_assoc_free(); > + json_decref(ctx->json_root); > + ctx->json_root = NULL; > + } I'd suggest: void json_print_echo(struct nft_ctx *ctx) { if (!ctx->json_root) return; if (ctx->json_echo) { ctx->json_echo = json_pack("{s:o}", "nftables", ctx->json_echo); json_dumpf(ctx->json_echo, ctx->output.output_fp, JSON_PRESERVE_ORDER); json_decref(ctx->json_echo); ctx->json_echo = NULL; fprintf(ctx->output.output_fp, "\n"); fflush(ctx->output.output_fp); } else { json_dumpf(ctx->json_root, ctx->output.output_fp, JSON_PRESERVE_ORDER); json_cmd_assoc_free(); json_decref(ctx->json_root); ctx->json_root = NULL; } } Thanks.
