Shell Hall of Shame
These pages show actual scripts, written and used in the "real world", which have potentially serious mistakes in them. The reader is expected to understand the common pitfalls associated with writing shell scripts.
The scripts herein may be copyrighted by various people or companies. I don't care. If you feel you need to protect your "rights" on utter crap, I feel rather sorry for you.
In my experience, commercial Unix flavors tend to have rather poorly written shell scripts. My theory for why this occurs is that shell scripting is usually done at the last minute, when someone needs a little tweak here or there, or needs a simple job done, or to maintain backward compatibility when a complex application has changed. As such, I believe that companies assign scripting jobs to junior programmers, and reserve the labor of their most experienced developers for the complex applications and tools. These junior programmers don't always have the experience or knowledge to see all their mistakes. And then, the senior programmers, who've been working on databases or kernels or whatever, either haven't learned the syntax of shell scripting in depth, or aren't brought in for code reviews on the scripts.
In any case, whether my theory is correct or not, the resulting products are often quite seriously flawed. Here are some examples.
1. /usr/bin/lpr (HP-UX 11.11)
1 #!/usr/bin/sh
2 # @(#)B.11.11_LR
3 # lpr script -- which makes lp look like lpr
4
5 options=
6 files=
7 remove=false
8
9 while [ $# -gt 0 ]
10 do
11 case $1 in
12 -c)
13 options="$options $1";;
14 -d*)
15 options="$options $1";;
16 -m)
17 options="$options $1";;
18 -n*)
19 options="$options $1";;
20 -o*)
21 options="$options $1";;
22 -r)
23 remove=true;;
24 -s)
25 options="$options $1";;
26 -t*)
27 options="$options $1";;
28 -w)
29 options="$options $1";;
30 -*)
31 echo "$0: option $1 not recognized"
32 exit 1;;
33 *)
34 files="$files $1";;
35 esac
36 shift
37 done
38
39 cat $files | lp -s $options
40
41 if [ $remove = true ]
42 then
43 rm -f $files
44 fi
This is an example of a wrapper script. Stylistically, this looks pretty good. There's just enough commenting to show what we're doing, and the variables have short but easily understandable names. The indenting is correct (if a bit overzealous). I'm not personally fond of the extraneous whitespace after each case label, but it's not a bad thing.
The problem here is what happens when a filename contains whitespace. Instead of being handled and passed along individually, all the filenames are being concatenated together into a single string, with a single space between them. And then the shell's word splitting is used to divide the filenames apart. If a filename already contains a space (or tab, newline, etc.), the resulting list of names given to cat on line 39 will not match the original filenames given to the wrapper script.
A similar problem might occur if any of the other options contains a space. The options variable builds up a list of them, and then (again on line 39), word splitting is used to break them into a list, for the lp command. If any options are given which contain whitespace, they will be split apart. (This may or may not be within the acceptable range of input for lpr or lp options, so this part might not actually be a problem. But I wouldn't want to take that chance.)
There is, in general, no clean way to handle this problem in the classic Bourne shell. But HP wasn't limited to the use of the Bourne shell when they wrote this program. It's part of a commercial product -- a Unix derivative that runs only on their own hardware. They don't need it to be portable to other platforms. They could use any of the features of their own shells, and this includes both POSIX and Korn shell (ksh88), which are shipped with the operating system. ksh88's arrays are good enough to be used for this purpose:
# ksh88
unset files i
...
case $1 in
...
*) files[$i]="$1"; i=$(($i+1));;
esac
...
cat "${files[@]}" | lp ...That would have avoided any problems with filenames containing whitespace. A similar bit of code could be used to build up an array of the options to be passed to lp.
Page 2: /usr/dt/bin/Xsession ->