[PATCH] add proper variable quoting to grub-mkconfig_lib (Debian bug #612417)

[Alexander Kurtz <kurtz.alex@googlemail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1026 bytes --]

Hi,

currently you can't use an image which has whitespace in its filename as
GRUB background image because grub-mkconfig_lib lacks proper variable
quoting (see [1] for more information). I've attached three patches
which should fix this problem:

 quote-big.patch:
  This patch fixes the problem with the minimal set of changes.

 quote-medium.patch:
  This patch adds proper quoting wherever it is safe to do so.

 quote-small.patch
  This patch additionally adds quoting in cases like this:
   ${grub-probe} --foo --bar => "${grub-probe}" --foo --bar
  This breaks things if ${grub-probe} contains additional parameters.

Please note that I already submitted similar patches a while ago[2], but
these are a little outdated now. The attached patches have been
refreshed and should apply cleanly to the current trunk.

What do you think?

Best regards

Alexander Kurtz

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=612417#5
[2] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=612417#10

[-- Attachment #1.2: quote-small.patch --]
[-- Type: text/x-patch, Size: 770 bytes --]

diff -Naur bazaar/util/grub-mkconfig_lib.in small/util/grub-mkconfig_lib.in
--- bazaar/util/grub-mkconfig_lib.in	2011-04-05 10:38:13.357139000 +0200
+++ small/util/grub-mkconfig_lib.in	2011-04-05 14:10:09.617962451 +0200
@@ -44,20 +44,20 @@
 
 make_system_path_relative_to_its_root ()
 {
-  ${grub_mkrelpath} $1
+  ${grub_mkrelpath} "${1}"
 }
 
 is_path_readable_by_grub ()
 {
-  path=$1
+  path="${1}"
 
   # abort if path doesn't exist
-  if test -e $path ; then : ;else
+  if test -e "${path}" ; then : ;else
     return 1
   fi
 
   # abort if file is in a filesystem we can't read
-  if ${grub_probe} -t fs $path > /dev/null 2>&1 ; then : ; else
+  if ${grub_probe} -t fs "${path}" > /dev/null 2>&1 ; then : ; else
     return 1
   fi
 

[-- Attachment #1.3: quote-medium.patch --]
[-- Type: text/x-patch, Size: 7070 bytes --]

diff -Naur bazaar/util/grub-mkconfig_lib.in medium/util/grub-mkconfig_lib.in
--- bazaar/util/grub-mkconfig_lib.in	2011-04-05 10:38:13.357139000 +0200
+++ medium/util/grub-mkconfig_lib.in	2011-04-05 14:10:21.136952907 +0200
@@ -16,19 +16,19 @@
 
 transform="@program_transform_name@"
 
-prefix=@prefix@
-exec_prefix=@exec_prefix@
-datarootdir=@datarootdir@
-datadir=@datadir@
-bindir=@bindir@
-sbindir=@sbindir@
-pkgdatadir=${datadir}/`echo @PACKAGE_TARNAME@ | sed "${transform}"`
+prefix="@prefix@"
+exec_prefix="@exec_prefix@"
+datarootdir="@datarootdir@"
+datadir="@datadir@"
+bindir="@bindir@"
+sbindir="@sbindir@"
+pkgdatadir="${datadir}/`echo "@PACKAGE_TARNAME@" | sed "${transform}"`"
 
-if test "x$grub_probe" = x; then
-  grub_probe=${sbindir}/`echo grub-probe | sed ${transform}`
+if test "x${grub_probe}" = "x"; then
+  grub_probe="${sbindir}/`echo grub-probe | sed "${transform}"`"
 fi
-if test "x$grub_mkrelpath" = x; then
-  grub_mkrelpath=${bindir}/`echo grub-mkrelpath | sed ${transform}`
+if test "x${grub_mkrelpath}" = "x"; then
+  grub_mkrelpath="${bindir}/`echo grub-mkrelpath | sed "${transform}"`"
 fi
 
 if $(which gettext >/dev/null 2>/dev/null) ; then
@@ -39,25 +39,25 @@
 
 grub_warn ()
 {
-  echo "Warning: $@" >&2
+  echo "Warning: ${@}" >&2
 }
 
 make_system_path_relative_to_its_root ()
 {
-  ${grub_mkrelpath} $1
+  ${grub_mkrelpath} "${1}"
 }
 
 is_path_readable_by_grub ()
 {
-  path=$1
+  path="${1}"
 
   # abort if path doesn't exist
-  if test -e $path ; then : ;else
+  if test -e "${path}" ; then : ;else
     return 1
   fi
 
   # abort if file is in a filesystem we can't read
-  if ${grub_probe} -t fs $path > /dev/null 2>&1 ; then : ; else
+  if ${grub_probe} -t fs "${path}" > /dev/null 2>&1 ; then : ; else
     return 1
   fi
 
@@ -72,24 +72,24 @@
 
 convert_system_path_to_grub_path ()
 {
-  path=$1
+  path="${1}"
 
   grub_warn "convert_system_path_to_grub_path() is deprecated.  Use prepare_grub_to_access_device() instead."
 
   # abort if GRUB can't access the path
-  if is_path_readable_by_grub ${path} ; then : ; else
+  if is_path_readable_by_grub "${path}" ; then : ; else
     return 1
   fi
 
-  if drive=`${grub_probe} -t drive $path` ; then : ; else
+  if drive="`${grub_probe} -t drive "${path}"`" ; then : ; else
     return 1
   fi
 
-  if relative_path=`make_system_path_relative_to_its_root $path` ; then : ; else
+  if relative_path="`make_system_path_relative_to_its_root "${path}"`" ; then : ; else
     return 1
   fi
 
-  echo ${drive}${relative_path}
+  echo "${drive}${relative_path}"
 }
 
 save_default_entry ()
@@ -103,15 +103,15 @@
 
 prepare_grub_to_access_device ()
 {
-  device=$1
+  device="${1}"
 
   # Abstraction modules aren't auto-loaded.
-  abstraction="`${grub_probe} --device ${device} --target=abstraction`"
+  abstraction="`${grub_probe} --device "${device}" --target=abstraction`"
   for module in ${abstraction} ; do
     echo "insmod ${module}"
   done
 
-  partmap="`${grub_probe} --device ${device} --target=partmap`"
+  partmap="`${grub_probe} --device "${device}" --target=partmap`"
   for module in ${partmap} ; do
     case "${module}" in
       netbsd | openbsd)
@@ -121,23 +121,23 @@
     esac
   done
 
-  fs="`${grub_probe} --device ${device} --target=fs`"
+  fs="`${grub_probe} --device "${device}" --target=fs`"
   for module in ${fs} ; do
     echo "insmod ${module}"
   done
 
   # If there's a filesystem UUID that GRUB is capable of identifying, use it;
   # otherwise set root as per value in device.map.
-  echo "set root='`${grub_probe} --device ${device} --target=drive`'"
-  if fs_uuid="`${grub_probe} --device ${device} --target=fs_uuid 2> /dev/null`" ; then
+  echo "set root='`${grub_probe} --device "${device}" --target=drive`'"
+  if fs_uuid="`${grub_probe} --device "${device}" --target=fs_uuid 2> /dev/null`" ; then
     echo "search --no-floppy --fs-uuid --set=root ${fs_uuid}"
   fi
 }
 
 grub_file_is_not_garbage ()
 {
-  if test -f "$1" ; then
-    case "$1" in
+  if test -f "${1}" ; then
+    case "${1}" in
       *.dpkg-*) return 1 ;; # debian dpkg
       README*)  return 1 ;; # documentation
     esac
@@ -149,21 +149,21 @@
 
 version_test_numeric ()
 {
-  local a=$1
-  local cmp=$2
-  local b=$3
-  if [ "$a" = "$b" ] ; then
-    case $cmp in
+  local a="${1}"
+  local cmp="${2}"
+  local b="${3}"
+  if [ "${a}" = "${b}" ] ; then
+    case "${cmp}" in
       ge|eq|le) return 0 ;;
       gt|lt) return 1 ;;
     esac
   fi
-  if [ "$cmp" = "lt" ] ; then
-    c=$a
-    a=$b
-    b=$c
+  if [ "${cmp}" = "lt" ] ; then
+    c="${a}"
+    a="${b}"
+    b="${c}"
   fi
-  if (echo $a ; echo $b) | sort -n | head -n 1 | grep -qx $b ; then
+  if (echo "${a}" ; echo "${b}") | sort -n | head -n 1 | grep -qx "${b}" ; then
     return 0
   else
     return 1
@@ -172,54 +172,54 @@
 
 version_test_gt ()
 {
-  local a=`echo $1 | sed -e "s/[^-]*-//"`
-  local b=`echo $2 | sed -e "s/[^-]*-//"`
+  local a="`echo "${1}" | sed -e "s/[^-]*-//"`"
+  local b="`echo "${2}" | sed -e "s/[^-]*-//"`"
   local cmp=gt
-  if [ "x$b" = "x" ] ; then
+  if [ "x${b}" = "x" ] ; then
     return 0
   fi
-  case $a:$b in
+  case "${a}:${b}" in
     *.old:*.old) ;;
-    *.old:*) a=`echo -n $a | sed -e s/\.old$//` ; cmp=gt ;;
-    *:*.old) b=`echo -n $b | sed -e s/\.old$//` ; cmp=ge ;;
+    *.old:*) a="`echo -n "${a}" | sed -e 's/\.old$//'`" ; cmp=gt ;;
+    *:*.old) b="`echo -n "${b}" | sed -e 's/\.old$//'`" ; cmp=ge ;;
   esac
-  version_test_numeric $a $cmp $b
-  return $?
+  version_test_numeric "${a}" "${cmp}" "${b}"
+  return "${?}"
 }
 
 version_find_latest ()
 {
   local a=""
-  for i in $@ ; do
-    if version_test_gt "$i" "$a" ; then
-      a="$i"
+  for i in "${@}" ; do
+    if version_test_gt "${i}" "${a}" ; then
+      a="${i}"
     fi
   done
-  echo "$a"
+  echo "${a}"
 }
 
 # One layer of quotation is eaten by "", the second by sed, and the third by
 # printf; so this turns ' into \'.  Note that you must use the output of
 # this function in a printf format string.
 gettext_quoted () {
-  $gettext "$@" | sed "s/'/'\\\\\\\\''/g"
+  ${gettext} "${@}" | sed "s/'/'\\\\\\\\''/g"
 }
 
 # Run the first argument through gettext_quoted, and then pass that and all
 # remaining arguments to printf.  This is a useful abbreviation and tends to
 # be easier to type.
 gettext_printf () {
-  local format="$1"
+  local format="${1}"
   shift
-  printf "$(gettext_quoted "$format")" "$@"
+  printf "`gettext_quoted "${format}"`" "${@}"
 }
 
 uses_abstraction () {
-  device=$1
+  device="${1}"
 
-  abstraction="`${grub_probe} --device ${device} --target=abstraction`"
+  abstraction="`${grub_probe} --device "${device}" --target=abstraction`"
   for module in ${abstraction}; do
-    if test "x${module}" = "x$2"; then
+    if test "x${module}" = "x${2}"; then
       return 0
     fi
   done

[-- Attachment #1.4: quote-big.patch --]
[-- Type: text/x-patch, Size: 7084 bytes --]

diff -Naur bazaar/util/grub-mkconfig_lib.in big/util/grub-mkconfig_lib.in
--- bazaar/util/grub-mkconfig_lib.in	2011-04-05 10:38:13.357139000 +0200
+++ big/util/grub-mkconfig_lib.in	2011-04-05 14:10:31.668952910 +0200
@@ -16,19 +16,19 @@
 
 transform="@program_transform_name@"
 
-prefix=@prefix@
-exec_prefix=@exec_prefix@
-datarootdir=@datarootdir@
-datadir=@datadir@
-bindir=@bindir@
-sbindir=@sbindir@
-pkgdatadir=${datadir}/`echo @PACKAGE_TARNAME@ | sed "${transform}"`
+prefix="@prefix@"
+exec_prefix="@exec_prefix@"
+datarootdir="@datarootdir@"
+datadir="@datadir@"
+bindir="@bindir@"
+sbindir="@sbindir@"
+pkgdatadir="${datadir}/`echo "@PACKAGE_TARNAME@" | sed "${transform}"`"
 
-if test "x$grub_probe" = x; then
-  grub_probe=${sbindir}/`echo grub-probe | sed ${transform}`
+if test "x${grub_probe}" = "x"; then
+  grub_probe="${sbindir}/`echo grub-probe | sed "${transform}"`"
 fi
-if test "x$grub_mkrelpath" = x; then
-  grub_mkrelpath=${bindir}/`echo grub-mkrelpath | sed ${transform}`
+if test "x${grub_mkrelpath}" = "x"; then
+  grub_mkrelpath="${bindir}/`echo grub-mkrelpath | sed "${transform}"`"
 fi
 
 if $(which gettext >/dev/null 2>/dev/null) ; then
@@ -39,25 +39,25 @@
 
 grub_warn ()
 {
-  echo "Warning: $@" >&2
+  echo "Warning: ${@}" >&2
 }
 
 make_system_path_relative_to_its_root ()
 {
-  ${grub_mkrelpath} $1
+  "${grub_mkrelpath}" "${1}"
 }
 
 is_path_readable_by_grub ()
 {
-  path=$1
+  path="${1}"
 
   # abort if path doesn't exist
-  if test -e $path ; then : ;else
+  if test -e "${path}" ; then : ;else
     return 1
   fi
 
   # abort if file is in a filesystem we can't read
-  if ${grub_probe} -t fs $path > /dev/null 2>&1 ; then : ; else
+  if "${grub_probe}" -t fs "${path}" > /dev/null 2>&1 ; then : ; else
     return 1
   fi
 
@@ -72,24 +72,24 @@
 
 convert_system_path_to_grub_path ()
 {
-  path=$1
+  path="${1}"
 
   grub_warn "convert_system_path_to_grub_path() is deprecated.  Use prepare_grub_to_access_device() instead."
 
   # abort if GRUB can't access the path
-  if is_path_readable_by_grub ${path} ; then : ; else
+  if is_path_readable_by_grub "${path}" ; then : ; else
     return 1
   fi
 
-  if drive=`${grub_probe} -t drive $path` ; then : ; else
+  if drive="`"${grub_probe}" -t drive "${path}"`" ; then : ; else
     return 1
   fi
 
-  if relative_path=`make_system_path_relative_to_its_root $path` ; then : ; else
+  if relative_path="`make_system_path_relative_to_its_root "${path}"`" ; then : ; else
     return 1
   fi
 
-  echo ${drive}${relative_path}
+  echo "${drive}${relative_path}"
 }
 
 save_default_entry ()
@@ -103,15 +103,15 @@
 
 prepare_grub_to_access_device ()
 {
-  device=$1
+  device="${1}"
 
   # Abstraction modules aren't auto-loaded.
-  abstraction="`${grub_probe} --device ${device} --target=abstraction`"
+  abstraction="`"${grub_probe}" --device "${device}" --target=abstraction`"
   for module in ${abstraction} ; do
     echo "insmod ${module}"
   done
 
-  partmap="`${grub_probe} --device ${device} --target=partmap`"
+  partmap="`"${grub_probe}" --device "${device}" --target=partmap`"
   for module in ${partmap} ; do
     case "${module}" in
       netbsd | openbsd)
@@ -121,23 +121,23 @@
     esac
   done
 
-  fs="`${grub_probe} --device ${device} --target=fs`"
+  fs="`"${grub_probe}" --device "${device}" --target=fs`"
   for module in ${fs} ; do
     echo "insmod ${module}"
   done
 
   # If there's a filesystem UUID that GRUB is capable of identifying, use it;
   # otherwise set root as per value in device.map.
-  echo "set root='`${grub_probe} --device ${device} --target=drive`'"
-  if fs_uuid="`${grub_probe} --device ${device} --target=fs_uuid 2> /dev/null`" ; then
+  echo "set root='`"${grub_probe}" --device "${device}" --target=drive`'"
+  if fs_uuid="`"${grub_probe}" --device "${device}" --target=fs_uuid 2> /dev/null`" ; then
     echo "search --no-floppy --fs-uuid --set=root ${fs_uuid}"
   fi
 }
 
 grub_file_is_not_garbage ()
 {
-  if test -f "$1" ; then
-    case "$1" in
+  if test -f "${1}" ; then
+    case "${1}" in
       *.dpkg-*) return 1 ;; # debian dpkg
       README*)  return 1 ;; # documentation
     esac
@@ -149,21 +149,21 @@
 
 version_test_numeric ()
 {
-  local a=$1
-  local cmp=$2
-  local b=$3
-  if [ "$a" = "$b" ] ; then
-    case $cmp in
+  local a="${1}"
+  local cmp="${2}"
+  local b="${3}"
+  if [ "${a}" = "${b}" ] ; then
+    case "${cmp}" in
       ge|eq|le) return 0 ;;
       gt|lt) return 1 ;;
     esac
   fi
-  if [ "$cmp" = "lt" ] ; then
-    c=$a
-    a=$b
-    b=$c
+  if [ "${cmp}" = "lt" ] ; then
+    c="${a}"
+    a="${b}"
+    b="${c}"
   fi
-  if (echo $a ; echo $b) | sort -n | head -n 1 | grep -qx $b ; then
+  if (echo "${a}" ; echo "${b}") | sort -n | head -n 1 | grep -qx "${b}" ; then
     return 0
   else
     return 1
@@ -172,54 +172,54 @@
 
 version_test_gt ()
 {
-  local a=`echo $1 | sed -e "s/[^-]*-//"`
-  local b=`echo $2 | sed -e "s/[^-]*-//"`
+  local a="`echo "${1}" | sed -e "s/[^-]*-//"`"
+  local b="`echo "${2}" | sed -e "s/[^-]*-//"`"
   local cmp=gt
-  if [ "x$b" = "x" ] ; then
+  if [ "x${b}" = "x" ] ; then
     return 0
   fi
-  case $a:$b in
+  case "${a}:${b}" in
     *.old:*.old) ;;
-    *.old:*) a=`echo -n $a | sed -e s/\.old$//` ; cmp=gt ;;
-    *:*.old) b=`echo -n $b | sed -e s/\.old$//` ; cmp=ge ;;
+    *.old:*) a="`echo -n "${a}" | sed -e 's/\.old$//'`" ; cmp=gt ;;
+    *:*.old) b="`echo -n "${b}" | sed -e 's/\.old$//'`" ; cmp=ge ;;
   esac
-  version_test_numeric $a $cmp $b
-  return $?
+  version_test_numeric "${a}" "${cmp}" "${b}"
+  return "${?}"
 }
 
 version_find_latest ()
 {
   local a=""
-  for i in $@ ; do
-    if version_test_gt "$i" "$a" ; then
-      a="$i"
+  for i in "${@}" ; do
+    if version_test_gt "${i}" "${a}" ; then
+      a="${i}"
     fi
   done
-  echo "$a"
+  echo "${a}"
 }
 
 # One layer of quotation is eaten by "", the second by sed, and the third by
 # printf; so this turns ' into \'.  Note that you must use the output of
 # this function in a printf format string.
 gettext_quoted () {
-  $gettext "$@" | sed "s/'/'\\\\\\\\''/g"
+  "${gettext}" "${@}" | sed "s/'/'\\\\\\\\''/g"
 }
 
 # Run the first argument through gettext_quoted, and then pass that and all
 # remaining arguments to printf.  This is a useful abbreviation and tends to
 # be easier to type.
 gettext_printf () {
-  local format="$1"
+  local format="${1}"
   shift
-  printf "$(gettext_quoted "$format")" "$@"
+  printf "`gettext_quoted "${format}"`" "${@}"
 }
 
 uses_abstraction () {
-  device=$1
+  device="${1}"
 
-  abstraction="`${grub_probe} --device ${device} --target=abstraction`"
+  abstraction="`"${grub_probe}" --device "${device}" --target=abstraction`"
   for module in ${abstraction}; do
-    if test "x${module}" = "x$2"; then
+    if test "x${module}" = "x${2}"; then
       return 0
     fi
   done

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]