[nft PATCH] parser_json: Fix and simplify verdict expression parsing

[nft PATCH] parser_json: Fix and simplify verdict expression parsing

[Phil Sutter]

Parsing of the "target" property was flawed in two ways:

* The value was extracted twice. Drop the first unconditional one.
* Expression allocation required since commit f1e8a129ee428 was broken,
  The expression was allocated only if the property was not present.

Fixes: f1e8a129ee428 ("src: Introduce chain_expr in jump and goto statements")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/parser_json.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index 19cdefd392014..4c7ee9911c42f 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1053,13 +1053,22 @@ static struct expr *json_parse_range_expr(struct json_ctx *ctx,
 	return range_expr_alloc(int_loc, expr_low, expr_high);
 }
 
+static struct expr *json_alloc_chain_expr(const char *chain)
+{
+	if (!chain)
+		return NULL;
+
+	return constant_expr_alloc(int_loc, &string_type, BYTEORDER_HOST_ENDIAN,
+				   NFT_CHAIN_MAXNAMELEN * BITS_PER_BYTE, chain);
+}
+
 static struct expr *json_parse_verdict_expr(struct json_ctx *ctx,
 					    const char *type, json_t *root)
 {
 	const struct {
 		int verdict;
 		const char *name;
-		bool chain;
+		bool need_chain;
 	} verdict_tbl[] = {
 		{ NFT_CONTINUE, "continue", false },
 		{ NFT_JUMP, "jump", true },
@@ -1068,27 +1077,19 @@ static struct expr *json_parse_verdict_expr(struct json_ctx *ctx,
 		{ NF_ACCEPT, "accept", false },
 		{ NF_DROP, "drop", false },
 	};
-	struct expr *chain_expr = NULL;
 	const char *chain = NULL;
 	unsigned int i;
 
-	json_unpack(root, "{s:s}", "target", &chain);
-	if (!chain)
-		chain_expr = constant_expr_alloc(int_loc, &string_type,
-						 BYTEORDER_HOST_ENDIAN,
-						 NFT_CHAIN_MAXNAMELEN *
-						 BITS_PER_BYTE, chain);
-
 	for (i = 0; i < array_size(verdict_tbl); i++) {
 		if (strcmp(type, verdict_tbl[i].name))
 			continue;
 
-		if (verdict_tbl[i].chain &&
+		if (verdict_tbl[i].need_chain &&
 		    json_unpack_err(ctx, root, "{s:s}", "target", &chain))
 			return NULL;
 
-		return verdict_expr_alloc(int_loc,
-					  verdict_tbl[i].verdict, chain_expr);
+		return verdict_expr_alloc(int_loc, verdict_tbl[i].verdict,
+					  json_alloc_chain_expr(chain));
 	}
 	json_error(ctx, "Unknown verdict '%s'.", type);
 	return NULL;
-- 
2.21.0

Re: [nft PATCH] parser_json: Fix and simplify verdict expression parsing

[Pablo Neira Ayuso]

On Mon, May 27, 2019 at 01:37:00PM +0200, Phil Sutter wrote:
> Parsing of the "target" property was flawed in two ways:
> 
> * The value was extracted twice. Drop the first unconditional one.
> * Expression allocation required since commit f1e8a129ee428 was broken,
>   The expression was allocated only if the property was not present.

Applied, thanks.