From 8fe75b8e9cb16c216f098988e3d78126bf16d4c2 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Fri, 7 Jun 2024 16:43:22 -0700
Subject: [PATCH] parser: make lead # in assignment value indicate a comment

technically a # leading a value in an assignment expression is allowed,
however people are also using it to a comment at the end of a line.
ie.

  @{var1}=value1    # comment about this value or for a given system

this unsurprisingly leads to odd/unexpected behavior when the variable
is used.

  allow rw /@{var1},

expands into
  allow rw /{value1,#,comment,about,this,value,or,for,a,given,system},

change a leading # as value in an assignment expression to a comment.
If the # is really supposed to lead the value, require it to be escaped
or in quotes.
ie.

  @{var1}=value1 \#not_a_comment

Note: this could potentially break som policy if the # was used as the
      leading character for a value in an assignment expression, but
      is worth it to avoid the confusion.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 parser/parser_lex.l    | 10 +++++++++-
 parser/tst/equality.sh | 12 ++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/parser/parser_lex.l b/parser/parser_lex.l
index 22fd2b5a4..62303fa39 100644
--- a/parser/parser_lex.l
+++ b/parser/parser_lex.l
@@ -255,9 +255,11 @@ MODES		{MODE_CHARS}+
 WS		[[:blank:]]
 NUMBER		[[:digit:]]+
 
+ID_FIRST_CHARS	[^ \t\r\n"!,#]
+ID_FIRST	{ID_CHARS}|(,{ID_CHARS}|\\[ ]|\\\t|\\\"|\\!|\\,|\\#)
 ID_CHARS	[^ \t\r\n"!,]
 ID 		{ID_CHARS}|(,{ID_CHARS}|\\[ ]|\\\t|\\\"|\\!|\\,)
-IDS		{ID}+
+IDS		{ID_FIRST}{ID}*
 INC_ID 		[^ \t\r\n"!,<>]|(,[^ \t\r\n"!,<>]|\\[ ]|\\\t|\\\"|\\!|\\,)
 INC_IDS		{INC_ID}+
 POST_VAR_ID_CHARS	[^ \t\n"!,]{-}[=\+]
@@ -507,6 +509,12 @@ GT		>
 		yyerror(_("Variable declarations do not accept trailing commas"));
 	}
 
+	#.*\r?\n	{ /* normal comment */
+		DUMP_AND_DEBUG("comment(%d): %s\n", current_lineno, yytext);
+		current_lineno++;
+		POP();
+	}
+
 	\\\n	{ DUMP_PREPROCESS; current_lineno++ ; }
 
 	\r?\n	{
diff --git a/parser/tst/equality.sh b/parser/tst/equality.sh
index a0badfa55..082b68f7c 100755
--- a/parser/tst/equality.sh
+++ b/parser/tst/equality.sh
@@ -643,6 +643,18 @@ verify_binary_equality "attachment slash filtering" \
                         @{FOO}=/foo
 			   /t @{BAR}/@{FOO} { }"
 
+# verify comment at end of variable assignment is not treated as a value
+verify_binary_equality "comment at end of set var" \
+                       "/t { /bin/ r, }" \
+                       "@{BAR}=/bin/   #a tail comment
+			   /t { @{BAR} r, }"
+
+verify_binary_equality "value like comment at end of set var" \
+                       "/t { /{bin/,#value} r, }" \
+                       "@{BAR}=bin/   \#value
+			   /t { /@{BAR} r, }"
+
+
 # This can potentially fail as ideally it requires a better dfa comparison
 # routine as it can generates hormomorphic dfas. The enumeration of the
 # dfas dumped will be different, even if the binary is the same