parser_bison: fail when specifying multiple comments

Before this patch grammar supported specifying multiple comments, and
only the last value would be assigned.

This patch adds a function to test if an attribute is already assigned
and, if so, calls erec_queue with this attribute location.

Use this function in order to check for duplication (or more) of comments
for actions that support it.

> nft add table inet filter { flags "dormant"\; comment "test"\; comment "another"\;}

Error: You can only specify this once. This statement is duplicated.
add table inet filter { flags dormant; comment test; comment another;}
                                                     ^^^^^^^^^^^^^^^^

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 7242c4c3d6f75883edb2e2caa2b5a714c49a75bd..524903fd80d37b77a0ac1f4b5651c8b21ae47e44 100644 (file)
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -121,6 +121,17 @@ static struct expr *handle_concat_expr(const struct location *loc,
        return expr;
 }
 
+static bool already_set(const void *attr, const struct location *loc,
+                       struct parser_state *state)
+{
+       if (!attr)
+               return false;
+
+       erec_queue(error(loc, "You can only specify this once. This statement is duplicated."),
+                  state->msgs);
+       return true;
+}
+
 #define YYLLOC_DEFAULT(Current, Rhs, N)        location_update(&Current, Rhs, N)
 
 #define symbol_value(loc, str) \
@@ -1556,6 +1567,10 @@ table_options            :       FLAGS           STRING
                        }
                        |       comment_spec
                        {
+                               if (already_set($<table>0->comment, &@$, state)) {
+                                       xfree($1);
+                                       YYERROR;
+                               }
                                $<table>0->comment = $1;
                        }
                        ;
@@ -1795,6 +1810,10 @@ set_block                :       /* empty */     { $$ = $<set>-1; }
                        |       set_block       set_mechanism   stmt_separator
                        |       set_block       comment_spec    stmt_separator
                        {
+                               if (already_set($1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $1->comment = $2;
                                $$ = $1;
                        }
@@ -1923,6 +1942,10 @@ map_block                :       /* empty */     { $$ = $<set>-1; }
                        }
                        |       map_block       comment_spec    stmt_separator
                        {
+                               if (already_set($1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $1->comment = $2;
                                $$ = $1;
                        }
@@ -2061,6 +2084,10 @@ counter_block            :       /* empty */     { $$ = $<obj>-1; }
                        }
                        |       counter_block     comment_spec
                        {
+                               if (already_set($<obj>1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $<obj>1->comment = $2;
                        }
                        ;
@@ -2074,6 +2101,10 @@ quota_block              :       /* empty */     { $$ = $<obj>-1; }
                        }
                        |       quota_block     comment_spec
                        {
+                               if (already_set($<obj>1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $<obj>1->comment = $2;
                        }
                        ;
@@ -2087,6 +2118,10 @@ ct_helper_block          :       /* empty */     { $$ = $<obj>-1; }
                        }
                        |       ct_helper_block     comment_spec
                        {
+                               if (already_set($<obj>1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $<obj>1->comment = $2;
                        }
                        ;
@@ -2104,6 +2139,10 @@ ct_timeout_block :       /*empty */
                        }
                        |       ct_timeout_block     comment_spec
                        {
+                               if (already_set($<obj>1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $<obj>1->comment = $2;
                        }
                        ;
@@ -2117,6 +2156,10 @@ ct_expect_block          :       /*empty */      { $$ = $<obj>-1; }
                        }
                        |       ct_expect_block     comment_spec
                        {
+                               if (already_set($<obj>1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $<obj>1->comment = $2;
                        }
                        ;
@@ -2130,6 +2173,10 @@ limit_block              :       /* empty */     { $$ = $<obj>-1; }
                        }
                        |       limit_block     comment_spec
                        {
+                               if (already_set($<obj>1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $<obj>1->comment = $2;
                        }
                        ;
@@ -2143,6 +2190,10 @@ secmark_block            :       /* empty */     { $$ = $<obj>-1; }
                        }
                        |       secmark_block     comment_spec
                        {
+                               if (already_set($<obj>1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $<obj>1->comment = $2;
                        }
                        ;
@@ -2156,6 +2207,10 @@ synproxy_block           :       /* empty */     { $$ = $<obj>-1; }
                        }
                        |       synproxy_block     comment_spec
                        {
+                               if (already_set($<obj>1->comment, &@2, state)) {
+                                       xfree($2);
+                                       YYERROR;
+                               }
                                $<obj>1->comment = $2;
                        }
                        ;
@@ -4000,6 +4055,10 @@ set_elem_option          :       TIMEOUT                 time_spec
                        }
                        |       comment_spec
                        {
+                               if (already_set($<expr>0->comment, &@1, state)) {
+                                       xfree($1);
+                                       YYERROR;
+                               }
                                $<expr>0->comment = $1;
                        }
                        ;
@@ -4034,6 +4093,10 @@ set_elem_expr_option     :       TIMEOUT                 time_spec
                        }
                        |       comment_spec
                        {
+                               if (already_set($<expr>0->comment, &@1, state)) {
+                                       xfree($1);
+                                       YYERROR;
+                               }
                                $<expr>0->comment = $1;
                        }
                        ;

diff --git a/tests/shell/testcases/optionals/comments_objects_dup_0 b/tests/shell/testcases/optionals/comments_objects_dup_0
new file mode 100755 (executable)
index 0000000..79d975a
--- /dev/null
+++ b/tests/shell/testcases/optionals/comments_objects_dup_0
@@ -0,0 +1,97 @@
+#!/bin/bash
+
+EXPECTED='table ip filter {
+       quota q {
+               over 1200 bytes
+               comment "test1"
+               comment "test1"
+       }
+}
+'
+
+$NFT -f - <<< "$EXPECTED"
+if [ $? -eq 0 ]
+then
+       exit 1
+fi
+
+EXPECTED='table ip filter {
+       counter c {
+               packets 0 bytes 0
+               comment "test2"
+               comment "test2"
+       }
+}
+'
+
+$NFT -f - <<< "$EXPECTED"
+if [ $? -eq 0 ]
+then
+       exit 1
+fi
+
+EXPECTED='table ip filter {
+       ct helper h {
+               type "sip" protocol tcp
+               l3proto ip
+               comment "test3"
+               comment "test3"
+       }
+}
+'
+
+$NFT -f - <<< "$EXPECTED"
+if [ $? -eq 0 ]
+then
+       exit 1
+fi
+
+EXPECTED='table ip filter {
+       ct expectation e {
+               protocol tcp
+               dport 666
+               timeout 100ms
+               size 96
+               l3proto ip
+               comment "test4"
+               comment "test4"
+       }
+}
+'
+
+$NFT -f - <<< "$EXPECTED"
+if [ $? -eq 0 ]
+then
+       exit 1
+fi
+
+EXPECTED='table ip filter {
+       limit l {
+               rate 400/hour
+               comment "test5"
+               comment "test5"
+       }
+}
+'
+
+$NFT -f - <<< "$EXPECTED"
+if [ $? -eq 0 ]
+then
+       exit 1
+fi
+
+EXPECTED='table ip filter {
+       synproxy s {
+               mss 1460
+               wscale 2
+               comment "test6"
+               comment "test6"
+       }
+}
+'
+
+$NFT -f - <<< "$EXPECTED"
+if [ $? -eq 0 ]
+then
+       exit 1
+fi