From e87d2f9ef8a4a298de5514b30ec2d43d3c90a644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Neira=20Ayuso?= Date: Mon, 6 Jan 2014 00:51:14 +0100 Subject: src: new error reporting approach for XML/JSON parsers I have added a new structure for reporting some errors in parser that we can't cover with errno. In this patch, we have three errors that we can't cover with errno: NFT_PARSE_EBADINPUT : Bad XML/JSON format in the input NFT_PARSE_EMISSINGNODE : Missing node in our input NFT_PARSE_EBADTYPE : Wrong type value in a node Signed-off-by: Alvaro Neira Ayuso Signed-off-by: Pablo Neira Ayuso --- src/expr/data_reg.c | 54 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 23 deletions(-) (limited to 'src/expr/data_reg.c') diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c index 76231af..e487bc7 100644 --- a/src/expr/data_reg.c +++ b/src/expr/data_reg.c @@ -27,12 +27,13 @@ #include "internal.h" #ifdef JSON_PARSING -static int nft_data_reg_verdict_json_parse(union nft_data_reg *reg, json_t *data) +static int nft_data_reg_verdict_json_parse(union nft_data_reg *reg, json_t *data, + struct nft_parse_err *err) { int verdict; const char *verdict_str; - verdict_str = nft_jansson_parse_str(data, "verdict"); + verdict_str = nft_jansson_parse_str(data, "verdict", err); if (verdict_str == NULL) return -1; @@ -45,9 +46,10 @@ static int nft_data_reg_verdict_json_parse(union nft_data_reg *reg, json_t *data return 0; } -static int nft_data_reg_chain_json_parse(union nft_data_reg *reg, json_t *data) +static int nft_data_reg_chain_json_parse(union nft_data_reg *reg, json_t *data, + struct nft_parse_err *err) { - reg->chain = strdup(nft_jansson_parse_str(data, "chain")); + reg->chain = strdup(nft_jansson_parse_str(data, "chain", err)); if (reg->chain == NULL) { return -1; } @@ -55,19 +57,20 @@ static int nft_data_reg_chain_json_parse(union nft_data_reg *reg, json_t *data) return 0; } -static int nft_data_reg_value_json_parse(union nft_data_reg *reg, json_t *data) +static int nft_data_reg_value_json_parse(union nft_data_reg *reg, json_t *data, + struct nft_parse_err *err) { int i; char node_name[6]; - if (nft_jansson_parse_val(data, "len", NFT_TYPE_U8, ®->len) < 0) + if (nft_jansson_parse_val(data, "len", NFT_TYPE_U8, ®->len, err) < 0) return -1; for (i = 0; i < div_round_up(reg->len, sizeof(uint32_t)); i++) { sprintf(node_name, "data%d", i); if (nft_jansson_str2num(data, node_name, BASE_HEX, - ®->val[i], NFT_TYPE_U32) != 0) + ®->val[i], NFT_TYPE_U32, err) != 0) return -1; } @@ -75,23 +78,24 @@ static int nft_data_reg_value_json_parse(union nft_data_reg *reg, json_t *data) } #endif -int nft_data_reg_json_parse(union nft_data_reg *reg, json_t *data) +int nft_data_reg_json_parse(union nft_data_reg *reg, json_t *data, + struct nft_parse_err *err) { #ifdef JSON_PARSING const char *type; - type = nft_jansson_parse_str(data, "type"); + type = nft_jansson_parse_str(data, "type", err); if (type == NULL) return -1; /* Select what type of parsing is needed */ if (strcmp(type, "value") == 0) { - return nft_data_reg_value_json_parse(reg, data); + return nft_data_reg_value_json_parse(reg, data, err); } else if (strcmp(type, "verdict") == 0) { - return nft_data_reg_verdict_json_parse(reg, data); + return nft_data_reg_verdict_json_parse(reg, data, err); } else if (strcmp(type, "chain") == 0) { - return nft_data_reg_chain_json_parse(reg, data); + return nft_data_reg_chain_json_parse(reg, data, err); } return 0; @@ -103,13 +107,14 @@ int nft_data_reg_json_parse(union nft_data_reg *reg, json_t *data) #ifdef XML_PARSING static int nft_data_reg_verdict_xml_parse(union nft_data_reg *reg, - mxml_node_t *tree) + mxml_node_t *tree, + struct nft_parse_err *err) { int verdict; const char *verdict_str; verdict_str = nft_mxml_str_parse(tree, "verdict", MXML_DESCEND_FIRST, - NFT_XML_MAND); + NFT_XML_MAND, err); if (verdict_str == NULL) return DATA_NONE; @@ -123,12 +128,13 @@ static int nft_data_reg_verdict_xml_parse(union nft_data_reg *reg, } static int nft_data_reg_chain_xml_parse(union nft_data_reg *reg, - mxml_node_t *tree) + mxml_node_t *tree, + struct nft_parse_err *err) { const char *chain; chain = nft_mxml_str_parse(tree, "chain", MXML_DESCEND_FIRST, - NFT_XML_MAND); + NFT_XML_MAND, err); if (chain == NULL) return DATA_NONE; @@ -140,7 +146,8 @@ static int nft_data_reg_chain_xml_parse(union nft_data_reg *reg, } static int nft_data_reg_value_xml_parse(union nft_data_reg *reg, - mxml_node_t *tree) + mxml_node_t *tree, + struct nft_parse_err *err) { int i; char node_name[6]; @@ -156,7 +163,7 @@ static int nft_data_reg_value_xml_parse(union nft_data_reg *reg, */ if (nft_mxml_num_parse(tree, "len", MXML_DESCEND_FIRST, BASE_DEC, - ®->len, NFT_TYPE_U8, NFT_XML_MAND) != 0) + ®->len, NFT_TYPE_U8, NFT_XML_MAND, err) != 0) return DATA_NONE; /* Get and set */ @@ -165,7 +172,7 @@ static int nft_data_reg_value_xml_parse(union nft_data_reg *reg, if (nft_mxml_num_parse(tree, node_name, MXML_DESCEND_FIRST, BASE_HEX, ®->val[i], NFT_TYPE_U32, - NFT_XML_MAND) != 0) + NFT_XML_MAND, err) != 0) return DATA_NONE; } @@ -173,7 +180,8 @@ static int nft_data_reg_value_xml_parse(union nft_data_reg *reg, } #endif -int nft_data_reg_xml_parse(union nft_data_reg *reg, mxml_node_t *tree) +int nft_data_reg_xml_parse(union nft_data_reg *reg, mxml_node_t *tree, + struct nft_parse_err *err) { #ifdef XML_PARSING const char *type; @@ -194,11 +202,11 @@ int nft_data_reg_xml_parse(union nft_data_reg *reg, mxml_node_t *tree) } if (strcmp(type, "value") == 0) - return nft_data_reg_value_xml_parse(reg, node); + return nft_data_reg_value_xml_parse(reg, node, err); else if (strcmp(type, "verdict") == 0) - return nft_data_reg_verdict_xml_parse(reg, node); + return nft_data_reg_verdict_xml_parse(reg, node, err); else if (strcmp(type, "chain") == 0) - return nft_data_reg_chain_xml_parse(reg, node); + return nft_data_reg_chain_xml_parse(reg, node, err); return DATA_NONE; #else -- cgit v1.2.3 From 871c7fd0204325b947a5fde3ab8617ef89b9168f Mon Sep 17 00:00:00 2001 From: Arturo Borrero Date: Sat, 18 Jan 2014 17:01:44 +0100 Subject: utils: fix nft_str2verdict return value Some verdicts have a negative value. The caller of nft_str2verdict() checking if return was < 0 clash with enum nft_verdict. While at it, add error reporting of invalid verdicts. Signed-off-by: Arturo Borrero Gonzalez Signed-off-by: Pablo Neira Ayuso --- src/expr/data_reg.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'src/expr/data_reg.c') diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c index e487bc7..8812daf 100644 --- a/src/expr/data_reg.c +++ b/src/expr/data_reg.c @@ -37,9 +37,12 @@ static int nft_data_reg_verdict_json_parse(union nft_data_reg *reg, json_t *data if (verdict_str == NULL) return -1; - verdict = nft_str2verdict(verdict_str); - if (verdict < 0) + if (nft_str2verdict(verdict_str, &verdict) != 0) { + err->node_name = "verdict"; + err->error = NFT_PARSE_EBADTYPE; + errno = EINVAL; return -1; + } reg->verdict = (uint32_t)verdict; @@ -118,9 +121,12 @@ static int nft_data_reg_verdict_xml_parse(union nft_data_reg *reg, if (verdict_str == NULL) return DATA_NONE; - verdict = nft_str2verdict(verdict_str); - if (verdict < 0) + if (nft_str2verdict(verdict_str, &verdict) != 0) { + err->node_name = "verdict"; + err->error = NFT_PARSE_EBADTYPE; + errno = EINVAL; return DATA_NONE; + } reg->verdict = (uint32_t)verdict; -- cgit v1.2.3 From dec687412e31118a3add7bad8de6ac496f7c1c65 Mon Sep 17 00:00:00 2001 From: Arturo Borrero Date: Sat, 18 Jan 2014 17:56:45 +0100 Subject: data_reg: fix verdict format approach Patrick reports that the XML/JSON formats of the data_reg object are not accuarate. This patch updates these formats, so they are now as follow: * with raw data (this doesn't change). * with a concrete verdict (eg drop accept) and an optional , with destination. In XML: goto output In JSON: "data_reg" : { "type" : "verdict", "verdict" : "goto" "chain" : "output", } The default output format is updated to reflect these changes (minor collateral thing). When parsing set_elems, to know if we need to add the NFT_SET_ELEM_ATTR_CHAIN flag, a basic check for the chain not being NULL is done, instead of evaluating if the result of the parsing was DATA_CHAIN. The DATA_CHAIN symbol is no longer used in the data_reg XML/JSON parsing zone. While at it, I updated the error reporting stuff regarding data_reg/verdict, in order to leave a consistent state in the library. A JSON testfile is updated as well. Reported-by: Patrick McHardy Signed-off-by: Arturo Borrero Gonzalez Signed-off-by: Pablo Neira Ayuso --- src/expr/data_reg.c | 171 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 99 insertions(+), 72 deletions(-) (limited to 'src/expr/data_reg.c') diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c index 8812daf..a755948 100644 --- a/src/expr/data_reg.c +++ b/src/expr/data_reg.c @@ -32,10 +32,11 @@ static int nft_data_reg_verdict_json_parse(union nft_data_reg *reg, json_t *data { int verdict; const char *verdict_str; + const char *chain; verdict_str = nft_jansson_parse_str(data, "verdict", err); if (verdict_str == NULL) - return -1; + return DATA_NONE; if (nft_str2verdict(verdict_str, &verdict) != 0) { err->node_name = "verdict"; @@ -46,18 +47,15 @@ static int nft_data_reg_verdict_json_parse(union nft_data_reg *reg, json_t *data reg->verdict = (uint32_t)verdict; - return 0; -} + if (nft_jansson_node_exist(data, "chain")) { + chain = nft_jansson_parse_str(data, "chain", err); + if (chain == NULL) + return DATA_NONE; -static int nft_data_reg_chain_json_parse(union nft_data_reg *reg, json_t *data, - struct nft_parse_err *err) -{ - reg->chain = strdup(nft_jansson_parse_str(data, "chain", err)); - if (reg->chain == NULL) { - return -1; + reg->chain = strdup(chain); } - return 0; + return DATA_VERDICT; } static int nft_data_reg_value_json_parse(union nft_data_reg *reg, json_t *data, @@ -67,17 +65,17 @@ static int nft_data_reg_value_json_parse(union nft_data_reg *reg, json_t *data, char node_name[6]; if (nft_jansson_parse_val(data, "len", NFT_TYPE_U8, ®->len, err) < 0) - return -1; + return DATA_NONE; for (i = 0; i < div_round_up(reg->len, sizeof(uint32_t)); i++) { sprintf(node_name, "data%d", i); if (nft_jansson_str2num(data, node_name, BASE_HEX, ®->val[i], NFT_TYPE_U32, err) != 0) - return -1; + return DATA_NONE; } - return 0; + return DATA_VALUE; } #endif @@ -93,15 +91,12 @@ int nft_data_reg_json_parse(union nft_data_reg *reg, json_t *data, return -1; /* Select what type of parsing is needed */ - if (strcmp(type, "value") == 0) { + if (strcmp(type, "value") == 0) return nft_data_reg_value_json_parse(reg, data, err); - } else if (strcmp(type, "verdict") == 0) { + else if (strcmp(type, "verdict") == 0) return nft_data_reg_verdict_json_parse(reg, data, err); - } else if (strcmp(type, "chain") == 0) { - return nft_data_reg_chain_json_parse(reg, data, err); - } - return 0; + return DATA_NONE; #else errno = EOPNOTSUPP; return -1; @@ -115,6 +110,7 @@ static int nft_data_reg_verdict_xml_parse(union nft_data_reg *reg, { int verdict; const char *verdict_str; + const char *chain; verdict_str = nft_mxml_str_parse(tree, "verdict", MXML_DESCEND_FIRST, NFT_XML_MAND, err); @@ -130,25 +126,16 @@ static int nft_data_reg_verdict_xml_parse(union nft_data_reg *reg, reg->verdict = (uint32_t)verdict; - return DATA_VERDICT; -} - -static int nft_data_reg_chain_xml_parse(union nft_data_reg *reg, - mxml_node_t *tree, - struct nft_parse_err *err) -{ - const char *chain; - chain = nft_mxml_str_parse(tree, "chain", MXML_DESCEND_FIRST, - NFT_XML_MAND, err); - if (chain == NULL) - return DATA_NONE; + NFT_XML_OPT, err); + if (chain != NULL) { + if (reg->chain) + xfree(reg->chain); - if (reg->chain) - xfree(reg->chain); + reg->chain = strdup(chain); + } - reg->chain = strdup(chain); - return DATA_CHAIN; + return DATA_VERDICT; } static int nft_data_reg_value_xml_parse(union nft_data_reg *reg, @@ -195,25 +182,24 @@ int nft_data_reg_xml_parse(union nft_data_reg *reg, mxml_node_t *tree, node = mxmlFindElement(tree, tree, "data_reg", "type", NULL, MXML_DESCEND_FIRST); - if (node == NULL) { - errno = EINVAL; - return DATA_NONE; - } + if (node == NULL) + goto err; type = mxmlElementGetAttr(node, "type"); - if (type == NULL) { - errno = EINVAL; - return DATA_NONE; - } + if (type == NULL) + goto err; if (strcmp(type, "value") == 0) return nft_data_reg_value_xml_parse(reg, node, err); else if (strcmp(type, "verdict") == 0) return nft_data_reg_verdict_xml_parse(reg, node, err); - else if (strcmp(type, "chain") == 0) - return nft_data_reg_chain_xml_parse(reg, node, err); + return DATA_NONE; +err: + errno = EINVAL; + err->node_name = "data_reg"; + err->error = NFT_PARSE_EMISSINGNODE; return DATA_NONE; #else errno = EOPNOTSUPP; @@ -308,6 +294,67 @@ nft_data_reg_value_snprintf_default(char *buf, size_t size, return offset; } +static int +nft_data_reg_verdict_snprintf_def(char *buf, size_t size, + union nft_data_reg *reg, uint32_t flags) +{ + int len = size, offset = 0, ret = 0; + + ret = snprintf(buf, size, "%s ", nft_verdict2str(reg->verdict)); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + + if (reg->chain != NULL) { + ret = snprintf(buf+offset, size, "-> %s ", reg->chain); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + } + + return offset; +} + +static int +nft_data_reg_verdict_snprintf_xml(char *buf, size_t size, + union nft_data_reg *reg, uint32_t flags) +{ + int len = size, offset = 0, ret = 0; + + ret = snprintf(buf, size, "" + "%s", nft_verdict2str(reg->verdict)); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + + if (reg->chain != NULL) { + ret = snprintf(buf+offset, size, "%s", + reg->chain); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + } + + ret = snprintf(buf+offset, size, ""); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + + return offset; +} + +static int +nft_data_reg_verdict_snprintf_json(char *buf, size_t size, + union nft_data_reg *reg, uint32_t flags) +{ + int len = size, offset = 0, ret = 0; + + ret = snprintf(buf, size, "\"data_reg\":{\"type\":\"verdict\"," + "\"verdict\":\"%s\"", nft_verdict2str(reg->verdict)); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + + if (reg->chain != NULL) { + ret = snprintf(buf+offset, size, ",\"chain\":\"%s\"", + reg->chain); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + } + + ret = snprintf(buf+offset, size, "}"); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + + return offset; +} + int nft_data_reg_snprintf(char *buf, size_t size, union nft_data_reg *reg, uint32_t output_format, uint32_t flags, int reg_type) { @@ -327,44 +374,24 @@ int nft_data_reg_snprintf(char *buf, size_t size, union nft_data_reg *reg, break; } case DATA_VERDICT: - switch(output_format) { - case NFT_OUTPUT_DEFAULT: - return snprintf(buf, size, "%d ", reg->verdict); - case NFT_OUTPUT_XML: - return snprintf(buf, size, - "" - "%s" - "", - nft_verdict2str(reg->verdict)); - case NFT_OUTPUT_JSON: - return snprintf(buf, size, - "\"data_reg\":{" - "\"type\":\"verdict\"," - "\"verdict\":\"%s\"" - "}", nft_verdict2str(reg->verdict)); - default: - break; - } case DATA_CHAIN: switch(output_format) { case NFT_OUTPUT_DEFAULT: - return snprintf(buf, size, "%s ", reg->chain); + return nft_data_reg_verdict_snprintf_def(buf, size, + reg, flags); case NFT_OUTPUT_XML: - return snprintf(buf, size, - "" - "%s" - "", reg->chain); + return nft_data_reg_verdict_snprintf_xml(buf, size, + reg, flags); case NFT_OUTPUT_JSON: - return snprintf(buf, size, - "\"data_reg\":{\"type\":\"chain\"," - "\"chain\":\"%s\"" - "}", reg->chain); + return nft_data_reg_verdict_snprintf_json(buf, size, + reg, flags); default: break; } default: break; } + return -1; } -- cgit v1.2.3 From 59e949294f4688bafe44b7def2972987224520c8 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Mon, 20 Jan 2014 10:26:57 +0100 Subject: rename library to libnftnl We plan to use this library name for the higher layer library. Signed-off-by: Pablo Neira Ayuso --- src/expr/data_reg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/expr/data_reg.c') diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c index a755948..0523cb7 100644 --- a/src/expr/data_reg.c +++ b/src/expr/data_reg.c @@ -20,8 +20,8 @@ #include #include #include -#include -#include +#include +#include #include "expr_ops.h" #include "data_reg.h" #include "internal.h" -- cgit v1.2.3