On Tuesday 21 October 2008, Frans Pop wrote:
> I've taken a look at the two manual runs and it looks like there is
> some weird issue in busybox.
[...]
> In the "bad" case $package is correctly determined as "d-i", but after
> that the following comparison in the script evaluates to true:
> [ "$line" != "${line#*$SPACE}" ]
> I.e, it does not recognize that there are additional space-separated
> fields in $line. The result of that is that $var never gets set which
> results in the "incorrect number of arguments" error later.
Although it is fast, it seems to me that using shell parameter expansion
to split fields for preseeding is possibly asking for problems,
especially given that a preseed string can contain any amount of
weirdness. I also doubt we have any real chance to fix this issue in
busybox before lenny.
Therefore attached two patches for debconf-set-selections that change the
parsing to instead use grep and sed, two commands which IMO can be
expected to be better tested and more robust when it comes to this kind
of string manipulation. I think that the slight loss in speed is
justified by the increased stability.
The first patch also removes any leading and trailing whitespace for
multi-line definitions leaving only a single space between split lines.
This will make the values in the cdebconf database more compact. Because
of the way split lines are defined we do not lose any functionality.
The second patch also adds a few sanity checks for syntax errors. It also
adds correct support for the case where a user forgets to add separating
whitespace after the template type when he wants to set an empty value;
IMO it is reasonable to support that as it's an easy mistake to make.
We could even strengthen that test by not allowing empty values for e.g.
boolean and select templates, but I've not included that in these
patches.
I've tested the patches moderately and everything seems to work as
intended.
commit 14ac5e7cae1852cec284c300bdbf666297489fe4
Author: Frans Pop <fjp@debian.org>
Date: Tue Oct 21 16:51:18 2008 +0200
Change line continuation handling
diff --git a/packages/preseed/debconf-set-selections b/packages/preseed/debconf-set-selections
index 3697d19..ad43451 100755
--- a/packages/preseed/debconf-set-selections
+++ b/packages/preseed/debconf-set-selections
@@ -23,11 +23,13 @@ SPACE=$(echo -en "[ \t]")
for line in $(grep -v '^#\|^[[:space:]]*$' "$file" | sed "s/$CR//g"); do
IFS="$OLDIFS"
- if [ "$line" != "${line%\\\\}" ]; then
- multiline="$multiline${line%\\\\}"
+ line="$(echo "$line" | sed 's/^[[:space:]]*//')"
+ if echo "$line" | grep -q '\\$'; then
+ multiline="${multiline:+$multiline }$(echo "$line" | \
+ sed 's/[[:space:]]*\\$//')"
continue
elif [ -n "$multiline" ]; then
- line="$multiline$line"
+ line="${multiline:+$multiline }$line"
multiline=""
fi
commit f1bf9988f6b6bfb4da69541fc8a18aef2335a9c4
Author: Frans Pop <fjp@debian.org>
Date: Tue Oct 21 17:25:05 2008 +0200
Make parsing of fields more robust
diff --git a/packages/preseed/debconf-set-selections b/packages/preseed/debconf-set-selections
index ad43451..0b1ee46 100755
--- a/packages/preseed/debconf-set-selections
+++ b/packages/preseed/debconf-set-selections
@@ -13,12 +13,29 @@ fi
file="$1"
+parse_error() {
+ echo "$1" >> $logfile
+ log "Error parsing preconfiguration file; see '$logfile' for details"
+ exit 1
+}
+
+# Returns the first field in the current line
+first_field() {
+ echo "$line" | grep -q "[[:space:]]" || return 1
+ echo "$line" | sed -r 's/^([^[:space:]]*).*/\1/'
+}
+# Returns any fields after the first field in the current line
+rest_line() {
+ if echo "$line" | grep -q "[[:space:]]"; then
+ echo "$line" | sed 's/^[^[:space:]]*[[:space:]]*//'
+ fi
+}
+
OLDIFS="$IFS"
NEWLINE="
"
IFS="$NEWLINE"
CR=$(echo -en "\r")
-SPACE=$(echo -en "[ \t]")
# TODO: this squashes \r elsewhere in the line too
for line in $(grep -v '^#\|^[[:space:]]*$' "$file" | sed "s/$CR//g"); do
IFS="$OLDIFS"
@@ -39,20 +56,24 @@ for line in $(grep -v '^#\|^[[:space:]]*$' "$file" | sed "s/$CR//g"); do
var=""
type=""
val=""
- while [ "$line" != "${line#*$SPACE}" ]; do
- if [ -n "${line%%$SPACE*}" ]; then
- if [ -z "$package" ]; then
- package="${line%%$SPACE*}"
- elif [ -z "$var" ]; then
- var="${line%%$SPACE*}"
- elif [ -z "$type" ]; then
- type="${line%%$SPACE*}"
- val="${line#*$SPACE}"
- break
- fi
+ if ! package="$(first_field)"; then
+ parse_error "Syntax error: unable to determine template owner"
+ fi
+ line="$(rest_line)"
+ if ! var="$(first_field)"; then
+ parse_error "Syntax error: unable to determine template name"
+ fi
+ line="$(rest_line)"
+ if ! type="$(first_field)"; then
+ # Allow for lines without separator before an empty value
+ if [ "$line" ]; then
+ type="$line"
+ else
+ parse_error "Syntax error: unable to determine template type"
fi
- line="${line#*$SPACE}"
- done
+ fi
+ line="$(rest_line)"
+ val="$line"
if [ "$type" = seen ]; then
# Set seen flag.
Attachment:
signature.asc
Description: This is a digitally signed message part.