[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[checks/scripts] [PATCH 4/4] bashisms update



Hi,

This is the fourth of a series of patches updating checks/scripts
bashisms checks with some changes that have been made to checkbashisms
over the past few months.

It's possible that some hunks of this patch may fail to apply as-is due
to conflicts with earlier patches; if this turns out to be the case,
please let me know and I'll prepare an updated patch taking in to
account whichever earlier patches have been applied

Hopefully all the patches are self-explanatory, but please let me know
if there are any questions or issues.

Regards,

Adam

bashisms.diff
-------------

1) Make a number of the tests less prone to false-positives

2) Allow checks to be performed against single quoted strings before
they are removed (currently echo with backslashes)

3) Enhance the $RANDOM, $(OS|MACH)TYPE, $HOST(TYPE|NAME), $DIRSTACK and
$EUID checks to allow optional braces around the variable name

4) Add checks for $UID, $SECONDS, $BASH_[A-Z]+, $SHELLOPTS, $PIPESTATUS
and $SHLVL.

5) Move the herestring (<<<) check to string bashisms to allow matching

         bar="$(cut '-d|' -f2 <<< "$foo")"

6) Allow a space between >& and the file descriptor

7) Enhance the "read without variable" test to also catch attempts to
pass options other than -r

8) Enhance the "brace expansion" test to allow "${foo},${bar}".
Admittedly the FPs in the existing test are unlikely to be seen in full
shell scripts, but they crop up quite a lot in shell snippets in
makefiles (which I've added basic support for to checkbashisms)

9) Add checks for suspend, caller, complete, compgen, declare, typeset,
disown, builtin, set -[BHT], alias -p, unalias -a, local with options or
an assigned value (i.e. "local -foo" or "local foo=bar"), VAR+=foo, 
pushd and popd, echo with backslashes, /dev/{tcp,udp}, export with
options other than -p, ulimit, shopt, type, time, dirs, process
substitution, readonly -[af], calls to sh using -[rD], long options or
[-+]O and functions with invalid names.

10) Enhance the "echo -e" test to also match the combination of -e and
-n

11) Add tests for the majority of the above
--- checks/scripts.orig	2008-06-13 14:51:32.000000000 +0100
+++ checks/scripts	2008-06-13 15:00:57.000000000 +0100
@@ -542,6 +542,11 @@
 	    if ($cat_string eq "" and $checkbashisms and !$within_another_shell) {
 		my $found = 0;
 		my $match = '';
+		my $LEADIN = '(?:(^|[`&;(|{])\s*|(if|do|while)\s+)';
+		my @bashism_single_quote_regexs = (
+		  $LEADIN . 'echo\s+(?:-[^e]+\s+)?([\'])[^\']*(\\\[abcEfnrtv\\\0])+.*?[\']',
+			# unsafe echo with backslashes
+		);
 		my @bashism_string_regexs = (
 		  '\$\[\w+\]',		       # arith not allowed
 		  '\$\{\w+\:\d+(?::\d+)?\}',   # ${foo:3[:1]}
@@ -550,33 +555,73 @@
 		  '\$\{!\w+[\@*]\}',	       # ${!prefix[*|@]}
 		  '\$\{!\w+\}',		       # ${!name}
 		  '(\$\(|\`)\s*\<\s*\S+\s*(\)|\`)', # $(\< foo) should be $(cat foo)
-		  '\$RANDOM\b',		       # $RANDOM
-		  '\$(OS|MACH)TYPE\b',         # $(OS|MACH)TYPE
-		  '\$HOST(TYPE|NAME)\b',       # $HOST(TYPE|NAME)
-		  '\$DIRSTACK\b',              # $DIRSTACK
-		  '\$EUID\b',                  # $EUID should be "id -u"
+		  '\$\{?RANDOM\}?\b',	       # $RANDOM
+		  '\$\{?(OS|MACH)TYPE\}?\b',   # $(OS|MACH)TYPE
+		  '\$\{?HOST(TYPE|NAME)\}?\b', # $HOST(TYPE|NAME)
+		  '\$\{?DIRSTACK\}?\b',        # $DIRSTACK
+		  '\$\{?EUID\}?\b',            # $EUID should be "id -u"
+		  '\$\{?UID\}?\b',	       # $UID should be "id -ru"
+		  '\$\{?SECONDS\}?\b',	       # $SECONDS
+		  '\$\{?BASH_[A-Z]+\}?\b',     # $BASH_SOMETHING
+		  '\$\{?SHELLOPTS\}?\b',       # $SHELLOPTS
+		  '\$\{?PIPESTATUS\}?\b',      # $PIPESTATUS
+		  '\$\{?SHLVL\}?\b',	       # $SHLVL
+		  '<<<',                       # <<< here string
+		  $LEADIN . 'echo\s+(?:-[^e]+\s+)?([\"])[^\"]*(\\\[abcEfnrtv\\\0])+.*?[\"]',
+			# unsafe echo with backslashes
 		);
 		my @bashism_regexs = (
 		  'function \w+\(\s*\)',       # function is useless
 					       # should be '.', not 'source'
-		  '(?:^|\s+)source\s+(?:\.\/|\/|\$)[^\s]+',
-		  '(\[|test|-o|-a)\s*[^\s]+\s+==\s', # should be 'b = a'
+		  $LEADIN . 'source\s+(?:\.\/|\/|\$)[^\s]+',
+		  '(test|-o|-a)\s*[^\s]+\s+==\s', # should be 'b = a'
+		  '\[\s+[^\]]+\s+==\s',        # should be 'b = a'
 		  '\s(\|\&)',		       # pipelining is not POSIX
-		  '[^\\\]\{([^\s]+?,)+[^\\\}\s]+\}', # brace expansion
+		  '[^\\\]\{(?:[^\s\\\}]+?,)+[^\\\}\s]+\}', # brace expansion
 		  '(?:^|\s+)\w+\[\d+\]=',      # bash arrays, H[0]
-		  '(?:^|\s+)read\s*(?:;|$)',   # read without variable
-		  '(?:^|\s+)kill\s+-[^sl]\w*', # kill -[0-9] or -[A-Z]
-		  '(?:^|\s+)trap\s+["\']?.*["\']?\s+.*[1-9]', # trap with signal numbers
+		  $LEADIN . '(read\s*(-[^r]+)*(?:;|$))',
+			# read without variable or with option other than -r
+		  $LEADIN . 'kill\s+-[^sl]\w*', # kill -[0-9] or -[A-Z]
+		  $LEADIN . 'trap\s+["\']?.*["\']?\s+.*[1-9]', # trap with signal numbers
 		  '\&>',		       # cshism
-		  '(<\&|>\&)\s*((-|\d+)[^\s;|\)\`&]|[^-\d])', # should be >word 2>&1
+		  '(<\&|>\&)\s*((-|\d+)[^\s;|)`&\\\\]|[^-\d\s]+)', # should be >word 2>&1
 		  '\[\[(?!:)',		       # alternative test command
-		  '(?:^|\s+)select\s+\w+',     # 'select' is not POSIX
-		  '(?:^|\s+)echo\s+-e',        # echo -e
-		  '(?:^|\s+)exec\s+-[acl]',    # exec -c/-l/-a name
+		  $LEADIN . 'select\s+\w+',    # 'select' is not POSIX
+		  $LEADIN . 'echo\s+(-n\s+)?-n?en?',  # echo -e
+		  $LEADIN . 'exec\s+-[acl]',   # exec -c/-l/-a name
 		  '(?:^|\s+)let\s',	       # let ...
 		  '(?<![\$\(])\(\(.*\)\)',     # '((' should be '$(('
+		  '\$\[[^][]+\]',	       # '$[' should be '$(('
 		  '(\[|test)\s+-a',	       # test with unary -a (should be -e)
-		  '<<<',                       # <<< here string
+		  '/dev/(tcp|udp)',	       # /dev/(tcp|udp)
+		  $LEADIN . '\w+\+=',	       # should be "VAR="${VAR}foo"
+		  $LEADIN . 'suspend\s',
+		  $LEADIN . 'caller\s',
+		  $LEADIN . 'complete\s',
+		  $LEADIN . 'compgen\s',
+		  $LEADIN . 'declare\s',
+		  $LEADIN . 'typeset\s',
+		  $LEADIN . 'disown\s',
+		  $LEADIN . 'builtin\s',
+		  $LEADIN . 'set\s+-[BHT]+',   # set -[BHT]
+		  $LEADIN . 'alias\s+-p',      # alias -p
+		  $LEADIN . 'unalias\s+-a',    # unalias -a
+		  $LEADIN . 'local\s+-[a-zA-Z]+', # local -opt
+		  $LEADIN . 'local\s+\w+=',    # local foo=bar
+		  '(?:^|\s+)\s*\(?\w*[^\(\w\s]+\S*?\s*\(\)[^\"]?',
+			# function names should only contain [a-z0-9_]
+		  $LEADIN . '(push|pop)d\b',   # (push|pod)d
+		  $LEADIN . 'export\s+-[^p]',  # export only takes -p as an option
+		  $LEADIN . 'ulimit\b',
+		  $LEADIN . 'shopt\b',
+		  $LEADIN . 'type\s',
+		  $LEADIN . 'time\s',
+		  $LEADIN . 'dirs\b',
+		  '(?:^|\s+)[<>]\(.*?\)',      # <() process substituion
+		  '(?:^|\s+)readonly\s+-[af]', # readonly -[af]
+		  $LEADIN . '(sh|\$\{?SHELL\}?) -[rD]', # sh -[rD]
+		  $LEADIN . '(sh|\$\{?SHELL\}?) --\w+', # sh --long-option
+		  $LEADIN . '(sh|\$\{?SHELL\}?) [-+]O', # sh [-+]O
 		);
 
 		# since this test is ugly, I have to do it by itself
@@ -595,6 +640,17 @@
 		# Ignore anything inside single quotes; it could be an
 		# argument to grep or the like.
 		my $line = $_;
+
+		unless ($found) {
+		   for my $re (@bashism_single_quote_regexs) {
+			if ($line =~ m/($re)/) {
+			    $found = 1;
+			    ($match) = m/($re)/;
+			    last;
+			}
+		    }
+		}
+
 		unless ($found) {
 		    $line =~ s/(^|[^\\](?:\\\\)*)\'(?:\\.|[^\\\'])+\'/$1''/g;
 		    for my $re (@bashism_string_regexs) {
--- testset/maintainer-scripts/debian/prerm.orig	2008-06-13 14:54:06.000000000 +0100
+++ testset/maintainer-scripts/debian/prerm	2008-06-13 14:57:55.000000000 +0100
@@ -43,6 +43,7 @@
 
 # More false positives for bashism checks.  None of these are errors.
 echo "$line" | grep -q '{fonts/map,}/{\$progname,pdftex,dvips,}//'
+echo "$line" | grep -q "${fonts},${foo}"
 echo '$[1+2]'
 printf "foo |& bar"
 perl -e "print q( kill -HUP $? )"
@@ -55,3 +56,90 @@
 
 # This is the only install-sgmlcatalog call that's allowed.
 install-sgmlcatalog --quiet --remove package
+
+# More bashisms checks
+
+read -x foo
+read -x
+read -r foo
+read foo
+read
+
+echo "a\\b"
+echo 'a\nb'
+
+echo "${UID}"
+echo "$EUID"
+echo "$SHLVL"
+echo "$DIRSTACK"
+echo "$SECONDS"
+echo "$BASH"
+echo "$BASH_FOO"
+echo "$SHELLOPTS"
+echo "$PIPESTATUS"
+
+bar="$(cut '-d|' -f2 <<< "$foo")"
+
+VAR=1
+VAR+=a
+
+echos() {
+  echo -n -e "bar"
+  echo -e -n "bar"
+  echo -en "bar"
+  echo -ne "bar"
+  echo "bar"
+  echo "echo -e foo"
+}
+
+ech.os() {
+  echo foo >& 2
+  echo foo >&bar
+  echo foo >& bar
+}
+
+foobar.() {
+  suspend x
+  suspended x
+  caller x
+  complete x
+  compgen x
+  declare -a foo
+}
+
+.foobar() {
+  typeset -x bar
+  disown 1
+  builtin foo
+  set -B
+  alias -p
+  unalias -a
+}
+
+IFS="()"
+
+ulimit
+shopt
+type -v bar
+time ls
+dirs
+diff <(tac a) <(tac b)
+
+pushd
+
+local foo=bar
+local -x foo
+
+popd
+
+readonly -f
+
+echo bar > /dev/tcp
+export x
+export -x x
+export -p x
+
+sh -x
+sh -D
+sh --foo
+sh +O
--- testset/tags.maintainer-scripts.orig	2008-06-13 10:39:25.000000000 +0100
+++ testset/tags.maintainer-scripts	2008-06-13 15:01:12.000000000 +0100
@@ -9,7 +9,7 @@
 E: maintainer-scripts: install-sgmlcatalog-deprecated postinst:98
 E: maintainer-scripts: install-sgmlcatalog-deprecated postrm:46
 E: maintainer-scripts: interpreter-without-predep control/config #!/usr/bin/python
-E: maintainer-scripts: maintainer-script-calls-init-script-directly prerm:54
+E: maintainer-scripts: maintainer-script-calls-init-script-directly prerm:55
 E: maintainer-scripts: maintainer-script-does-not-check-for-existence-of-wm-menu-config postinst:31
 E: maintainer-scripts: maintainer-script-modifies-inetd-conf postinst:91
 E: maintainer-scripts: maintainer-script-modifies-inetd-conf postinst:92
@@ -64,6 +64,35 @@
 W: maintainer-scripts: possible-bashism-in-maintainer-script postinst:20 'H[0]='
 W: maintainer-scripts: possible-bashism-in-maintainer-script postinst:21 '${H[0]}'
 W: maintainer-scripts: possible-bashism-in-maintainer-script postinst:23 '${H[@]}'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:101 'foobar.() '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:102 '  suspend '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:104 '  caller '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:105 '  complete '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:106 '  compgen '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:107 '  declare '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:110 '.foobar() '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:111 '  typeset '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:112 '  disown '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:113 '  builtin '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:114 '  set -B'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:115 '  alias -p'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:116 '  unalias -a'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:121 'ulimit'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:122 'shopt'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:123 'type '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:124 'time '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:125 'dirs'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:126 ' <(tac a)'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:128 'pushd'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:130 'local foo='
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:131 'local -x'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:133 'popd'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:135 'readonly -f'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:137 '/dev/tcp'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:139 'export -x'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:143 'sh -D'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:144 'sh --foo'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:145 'sh +O'
 W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:19 '[ "$2" == '
 W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:23 'function foo( )'
 W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:26 '&>'
@@ -71,7 +100,29 @@
 W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:31 'trap "echo hi" EXIT HUP 3'
 W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:33 '[['
 W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:34 '    kill -HUP'
-W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:51 '${line:3:1}'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:52 '${line:3:1}'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:62 'read -x foo'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:63 'read -x'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:66 'read'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:68 'echo "a\\b"'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:69 'echo 'a\nb''
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:71 '${UID'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:72 '$EUID'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:73 '$SHLVL'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:74 '$DIRSTACK'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:75 '$SECONDS'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:77 '$BASH_FOO'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:78 '$SHELLOPTS'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:79 '$PIPESTATUS'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:81 '<<<'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:84 'VAR+='
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:87 '  echo -n -e'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:88 '  echo -e'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:89 '  echo -en'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:90 '  echo -ne'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:95 'ech.os() '
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:97 '>&bar'
+W: maintainer-scripts: possible-bashism-in-maintainer-script prerm:98 '>& bar'
 W: maintainer-scripts: possibly-insecure-handling-of-tmp-files-in-maintainer-script postinst:50
 W: maintainer-scripts: possibly-insecure-handling-of-tmp-files-in-maintainer-script postrm:39
 W: maintainer-scripts: postinst-does-not-load-confmodule
@@ -79,5 +130,10 @@
 W: maintainer-scripts: postrm-does-not-purge-debconf
 W: maintainer-scripts: postrm-has-useless-call-to-ldconfig
 W: maintainer-scripts: read-in-maintainer-script postinst:18
+W: maintainer-scripts: read-in-maintainer-script prerm:62
+W: maintainer-scripts: read-in-maintainer-script prerm:63
+W: maintainer-scripts: read-in-maintainer-script prerm:64
+W: maintainer-scripts: read-in-maintainer-script prerm:65
+W: maintainer-scripts: read-in-maintainer-script prerm:66
 W: maintainer-scripts: start-stop-daemon-in-maintainer-script postinst:132
 W: maintainer-scripts: update-alternatives-remove-called-in-postrm

Reply to: