[PATCH 1/2 nft] parser: Centralize commentary rule

[PATCH 1/2 nft] parser: Centralize commentary rule

[Carlos Falgueras García]

It make more sense if this rule can not be empty, so it can be used both as
mandatory as optional. The higher rule should choice use it in a way or another.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/parser_bison.y | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index ef10dee..6f51a49 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -440,7 +440,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %destructor { close_scope(state); table_free($$); }	table_block_alloc
 %type <chain>			chain_block_alloc chain_block
 %destructor { close_scope(state); chain_free($$); }	chain_block_alloc
-%type <rule>			rule
+%type <rule>			rule rule_alloc
 %destructor { rule_free($$); }	rule
 
 %type <val>			set_flag_list	set_flag
@@ -1273,11 +1273,7 @@ ruleid_spec		:	chain_spec	handle_spec	position_spec
 			}
 			;
 
-comment_spec		:	/* empty */
-			{
-				$$ = NULL;
-			}
-			|	COMMENT		string
+comment_spec		:	COMMENT		string
 			{
 				$$ = $2;
 			}
@@ -1295,12 +1291,21 @@ ruleset_spec		:	/* empty */
 			}
 			;
 
-rule			:	stmt_list	comment_spec
+rule			:	rule_alloc
+			{
+				$$->comment = NULL;
+			}
+			|	rule_alloc	comment_spec
+			{
+				$$->comment = $2;
+			}
+			;
+
+rule_alloc		:	stmt_list
 			{
 				struct stmt *i;
 
 				$$ = rule_alloc(&@$, NULL);
-				$$->comment = $2;
 				list_for_each_entry(i, $1, list)
 					$$->num_stmts++;
 				list_splice_tail($1, &$$->stmts);
@@ -2029,9 +2034,9 @@ set_elem_option		:	TIMEOUT			time_spec
 			{
 				$<expr>0->timeout = $2 * 1000;
 			}
-			|	COMMENT			string
+			|	comment_spec
 			{
-				$<expr>0->comment = $2;
+				$<expr>0->comment = $1;
 			}
 			;
 
-- 
2.8.2

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2 nft] parser: Check commentaries length

[Carlos Falgueras García]

Checks the commentary maximum length and reports to user in case of error.

Example:
> nft add table t
> nft add chain t c
> nft add rule t c ip saddr 1.1.1.1 counter comment "abc...xyz" # len > 128
<cmdline>:1:47-N: Error: Comment too long. 128 characters maximum allowed
add rule t c ip saddr 1.1.1.1 counter comment abc...xyz
                                              ^^^^^^^^^

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 include/rule.h     | 2 ++
 src/parser_bison.y | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/include/rule.h b/include/rule.h
index bd24648..7e8daac 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -428,4 +428,6 @@ enum udata_type {
 };
 #define UDATA_TYPE_MAX (__UDATA_TYPE_MAX - 1)
 
+#define UDATA_COMMENT_MAXLEN 128
+
 #endif /* NFTABLES_RULE_H */
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 6f51a49..0bf0e27 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1275,6 +1275,11 @@ ruleid_spec		:	chain_spec	handle_spec	position_spec
 
 comment_spec		:	COMMENT		string
 			{
+				if (strlen($2) > UDATA_COMMENT_MAXLEN ) {
+					erec_queue(error(&@2, "Comment too long. %d characters maximum allowed", UDATA_COMMENT_MAXLEN),
+						   state->msgs);
+					YYERROR;
+				}
 				$$ = $2;
 			}
 			;
-- 
2.8.2

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2 nft] parser: Check commentaries length

[Pablo Neira Ayuso]

On Mon, May 30, 2016 at 06:35:40PM +0200, Carlos Falgueras García wrote:
> Checks the commentary maximum length and reports to user in case of error.

Also applied and I have rewritten the subject and description too to
indicate why we need this check.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2 nft] parser: Centralize commentary rule

[Pablo Neira Ayuso]

Applied, thanks.

I have rewritten the email subject, the typical jargon word for this
is "consolidation" instead of centralize.