From bad72d96c78a7273e1c8f549d2637405e3dec075 Mon Sep 17 00:00:00 2001
From: xonix <xonixx@gmail.com>
Date: Wed, 22 Dec 2021 14:15:44 +0200
Subject: [PATCH] Insufficient validation in `@define` #85

---
 makesure.awk                                  |  8 +-
 tests/10_define.sh                            |  2 +-
 ..._validation.sh => 16_define_validation.sh} |  8 +-
 tests/16_define_validation.tush               | 83 +++++++++++++++++++
 ...error.sh => 16_define_validation_error.sh} | 14 ++++
 tests/16_prelude_fail.sh                      |  6 --
 tests/16_prelude_fail.tush                    |  7 --
 tests/22_nat_order.tush                       |  4 +-
 tests/24_define_validation.tush               | 26 ------
 9 files changed, 107 insertions(+), 51 deletions(-)
 rename tests/{24_define_validation.sh => 16_define_validation.sh} (76%)
 create mode 100644 tests/16_define_validation.tush
 rename tests/{24_define_validation_error.sh => 16_define_validation_error.sh} (62%)
 delete mode 100644 tests/16_prelude_fail.sh
 delete mode 100644 tests/16_prelude_fail.tush
 delete mode 100644 tests/24_define_validation.tush

diff --git a/makesure.awk b/makesure.awk
index bc458de..1dbf588 100755
--- a/makesure.awk
+++ b/makesure.awk
@@ -152,7 +152,7 @@ function handleDefineLine(line,   kv) {
     DefinesCode = addL(DefinesCode, line "\nexport " kv[0])
 }
 function checkValidDefineSyntax(line) {
-  if (line ~ /[ \t]*[A-Za-z_][A-Za-z0-9_]*=((([^ ;'"]|(\\ ))([^ ;]|(\\ ))*)|('[^']*')|("((\\\\)|(\\")|[^"])*")|(\$'((\\\\)|(\\')|[^'])*'))+[ \t]*(#.*)?$/)
+  if (line ~ /^[ \t]*[A-Za-z_][A-Za-z0-9_]*=(([A-Za-z0-9_]|(\\.))+|('[^']*')|("((\\\\)|(\\")|[^"])*")|(\$'((\\\\)|(\\')|[^'])*'))+[ \t]*(#.*)?$/)
     return 1
   addError("Invalid define declaration")
   return 0
@@ -514,9 +514,9 @@ function shellExec(script, comment,   res) {
   } else
     script = Shell " -e -c " quoteArg(script)
 
-  # This is hard to unit-test properly.
-  # The issue with Ctrl-C only happens with Gawk 4.1.3.
-  # The manual test exists via `expect -f tests/manual_ctrl_c.expect.txt`
+    # This is hard to unit-test properly.
+    # The issue with Ctrl-C only happens with Gawk 4.1.3.
+    # The manual test exists via `expect -f tests/manual_ctrl_c.expect.txt`
   script = "trap 'exit 7' INT;" script
 
   #print script
diff --git a/tests/10_define.sh b/tests/10_define.sh
index c5a08e6..ec13277 100644
--- a/tests/10_define.sh
+++ b/tests/10_define.sh
@@ -1,7 +1,7 @@
 
 
 @define A=aaa
-@define B=${A}bbb
+@define B="${A}bbb"
 @define Commented=value # just a comment
 
 @goal testA
diff --git a/tests/24_define_validation.sh b/tests/16_define_validation.sh
similarity index 76%
rename from tests/24_define_validation.sh
rename to tests/16_define_validation.sh
index d787617..27d8abd 100644
--- a/tests/24_define_validation.sh
+++ b/tests/16_define_validation.sh
@@ -3,11 +3,11 @@
 @define B='bbb bbb'
 @define B1='bbb # bbb'
 @define B2='bbb ; bbb'
-@define B3='bbb ; bbb';
-@define B4='bbb ; bbb'    ;
 @define C=$'cc\'c  cc\'c'
 @define C1='cc'\''c  cc'\''c'
 @define D=a\ b\ \ c
+@define D1=a\b\c\d
+@define D2=a\'\c\"
 @define E="eee eee"
 @define F="eee \\ \" eee"
 
@@ -18,8 +18,6 @@
 @define AC2=aaa#         # comment
 @define BC1='bbb bbb'  # comment
 @define BC2='bbb bbb'#  # comment
-@define BC3='bbb bbb';  # comment
-@define BC4='bbb ; bbb'   ;  # comment
 @define CC1=$'cc\'c  cc\'c'  #
 @define DC1=a\ b\ c #
 @define EC1="eee"  # comment
@@ -28,7 +26,7 @@
 @define LC1="$A$B$C$D$E"                 # other comment
 
 @goal default
-  for x in A B B1 B2 B3 B4 C C1 D E F K L AC1 AC2 BC1 BC2 BC3 BC4 CC1 DC1 EC1 KC1 LC1
+  for x in A B B1 B2 C C1 D D1 D2 E F K L AC1 AC2 BC1 BC2 CC1 DC1 EC1 KC1 LC1
   do
     printf '%-3s=%s\n' "$x" "${!x}"
   done
diff --git a/tests/16_define_validation.tush b/tests/16_define_validation.tush
new file mode 100644
index 0000000..d36dcfd
--- /dev/null
+++ b/tests/16_define_validation.tush
@@ -0,0 +1,83 @@
+
+$ cd "$MYDIR"; ./$MAKESURE -f tests/16_define_validation.sh
+|   goal 'default' ...
+| A  =aaa
+| B  =bbb bbb
+| B1 =bbb # bbb
+| B2 =bbb ; bbb
+| C  =cc'c cc'c
+| C1 =cc'c cc'c
+| D  =a b  c
+| D1 =abcd
+| D2 =a'c"
+| E  =eee eee
+| F  =eee \ " eee
+| K  =aaabbbcc'ca b  ceee
+| L  =aaabbb bbb cc'c cc'ca b  c eee eee
+| AC1=aaa
+| AC2=aaa#
+| BC1=bbb bbb
+| BC2=bbb bbb#
+| CC1=cc'c cc'c
+| DC1=a b c
+| EC1=eee
+| KC1=aaabbbcc'ca b ceee
+| LC1=aaabbb bbbcc'c cc'ca b  ceee eee
+
+$ cd "$MYDIR"; ./$MAKESURE -f tests/16_define_validation_error.sh
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:2: @define echo
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:3: @define echo;
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:4: @define echo;echo
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:5: @define echo ; echo
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:6: @define echo 'Hello'
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:7: @define echo 'Hello';
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:8: @define A=aaa echo 'Hello'
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:9: @define B=aaa\ aaa echo 'Hello'
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:10: @define C='aaa aaa' echo 'Hello'
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:12: @define echo                       # comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:13: @define echo 'Hello'           # comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:14: @define echo 'Hello';           # comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:15: @define echo 'Hello'  ;# comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:16: @define A=aaa echo 'Hello'        # comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:17: @define B=aaa\ aaa echo 'Hello'# comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:18: @define C='aaa aaa' echo 'Hello' # comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:19: @define C='aaa aaa' echo 'Hello' ; # comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:22: @define B3='bbb ; bbb';
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:23: @define B4='bbb ; bbb'    ;
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:24: @define BC3='bbb bbb';  # comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:25: @define BC4='bbb ; bbb'   ;  # comment
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:28: @define IL1=a||b
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:29: @define IL2=a&b
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:30: @define IL3=a&&b
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:31: @define IL4=a!b
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:32: @define IL5=a-b
+@ Invalid define declaration:
+@ tests/16_define_validation_error.sh:33: @define IL5=a+b
+? 1
+
diff --git a/tests/24_define_validation_error.sh b/tests/16_define_validation_error.sh
similarity index 62%
rename from tests/24_define_validation_error.sh
rename to tests/16_define_validation_error.sh
index f4e97c3..a766a01 100644
--- a/tests/24_define_validation_error.sh
+++ b/tests/16_define_validation_error.sh
@@ -18,6 +18,20 @@
 @define C='aaa aaa' echo 'Hello' # comment
 @define C='aaa aaa' echo 'Hello' ; # comment
 
+# Let's disallow ';' for simplicity and unification
+@define B3='bbb ; bbb';
+@define B4='bbb ; bbb'    ;
+@define BC3='bbb bbb';  # comment
+@define BC4='bbb ; bbb'   ;  # comment
+
+# Illegal chars. This is more restrictive than shell but simpler to parse
+@define IL1=a||b
+@define IL2=a&b
+@define IL3=a&&b
+@define IL4=a!b
+@define IL5=a-b
+@define IL5=a+b
+
 @goal default
   echo 'Should not show'
 
diff --git a/tests/16_prelude_fail.sh b/tests/16_prelude_fail.sh
deleted file mode 100644
index 32ede7e..0000000
--- a/tests/16_prelude_fail.sh
+++ /dev/null
@@ -1,6 +0,0 @@
-
-# TODO this should be disallowed in https://github.com/xonixx/makesure/issues/85
-@define A=hello; exit 47
-
-@goal default
-  echo "default"
\ No newline at end of file
diff --git a/tests/16_prelude_fail.tush b/tests/16_prelude_fail.tush
deleted file mode 100644
index 68c8d43..0000000
--- a/tests/16_prelude_fail.tush
+++ /dev/null
@@ -1,7 +0,0 @@
-
-TODO shall we delete this? See https://github.com/xonixx/makesure/issues/85
-
-$ cd "$MYDIR"; ./$MAKESURE -f tests/16_prelude_fail.sh
-|   goal 'default' ...
-|   goal 'default' failed
-? 47
diff --git a/tests/22_nat_order.tush b/tests/22_nat_order.tush
index 542475b..a72afac 100644
--- a/tests/22_nat_order.tush
+++ b/tests/22_nat_order.tush
@@ -16,7 +16,7 @@ $ cd "$MYDIR"; ./$MAKESURE -f tests/22_nat_order.sh nat_order_test
 | 13_doc.tush
 | 14_private.tush
 | 15_lib.tush
-| 16_prelude_fail.tush
+| 16_define_validation.tush
 | 17_empty_prelude.tush
 | 18_vars_priority.tush
 | 19_optimize_goals.tush
@@ -40,7 +40,7 @@ $ cd "$MYDIR"; ./$MAKESURE -f tests/22_nat_order nat_order_test
 | ../13_doc.tush
 | ../14_private.tush
 | ../15_lib.tush
-| ../16_prelude_fail.tush
+| ../16_define_validation.tush
 | ../17_empty_prelude.tush
 | ../18_vars_priority.tush
 | ../19_optimize_goals.tush
diff --git a/tests/24_define_validation.tush b/tests/24_define_validation.tush
deleted file mode 100644
index e7a7fb3..0000000
--- a/tests/24_define_validation.tush
+++ /dev/null
@@ -1,26 +0,0 @@
-
-$ cd "$MYDIR"; ./$MAKESURE -f tests/24_define_validation.sh
-|   goal 'default' ...
-| A  =aaa
-| B  =bbb bbb
-| B1 =bbb # bbb
-| B2 =bbb ; bbb
-| B3 =bbb ; bbb
-| B4 =bbb ; bbb
-| C  =cc'c cc'c
-| C1 =cc'c cc'c
-| D  =a b  c
-| E  =eee eee
-| K  =aaabbbcc'ca b  ceee
-| L  =aaabbb bbb cc'c cc'ca b  c eee eee
-| AC1=aaa
-| AC2=aaa#
-| BC1=bbb bbb
-| BC2=bbb bbb#
-| BC3=bbb bbb
-| BC4=bbb ; bbb
-| CC1=cc'c cc'c
-| DC1=a b c
-| EC1=eee
-| KC1=aaabbbcc'ca b ceee
-| LC1=aaabbb bbbcc'c cc'ca b  ceee eee