Package: abcde
Version: 2.8.1-1
Severity: normal
Tags: patch

Dear Maintainer,

abcde's Debian repository commit
b42de9e27e3c8f2d8fb190b40675f15ff79f5f4f introduced new customizable
functions mungetrackname(), mungeartistname() and mungealbumname().
However, the predefined functions do not work as documented in program's
man page.

A quote from abcde(1):


       mungefilename
              mungefilename() is an abcde shell function that can  be  over‐
              ridden  via  abcde.conf.  It takes CDDB data as $1 and outputs
              the resulting filename on stdout.
              [...]
              New  to  abcde  2.7.3  are the user definable functions munge‐
              trackname, mungeartistname and mungealbumname which default to
              mungefilename. [...]



It says that CDDB data is the first parameter $1 but the predefined
function is actually this:


        mungefilename ()
        {
                echo "$@" | sed -e 's/^\.*//' -e 's/ /_/g' | tr -d 
":><|*/\"'?[:cntrl:]"
        }


Note that the echo command is not using "$1" as documented but "$@".
That itself does not cause any problems here but read on...

The new predefined munge* functions don't use $1 either but instead have
unquoted $@ so they are actually passing all positional parameters and
shell's word-splitting splits all words as separate parameters.


        mungetrackname ()
        {
                mungefilename $@
        }


Even that doesn't cause problems in the _default_ settings because by
default mungefilename() converts all spaces to underscores (_) in
filenames.

But this all introduces a bug which is potentially triggered when user
tries to write her own mungefilename() function. Let's say that user
reads the documentation (including the note about $1) and wants his own
mungefilename() like this, for example:

        mungefilename ()
        {
                echo "$1" | tr -d ":><|*/\"'?[:cntrl:]"
        }

User wants to keep spaces in filenames. He doesn't define other munge*
functions because, as documented in the man page, they call
mungefilename() by default.

Now let's see how the call chain goes. Let's say user encodes a CD which
has a song called "Two Hands" Tracks' filenames are constructed like
this:

        TRACKFILE="$(mungetrackname "$TRACKNAME")"

And in this example the $TRACKFILE variable is "Two Hands" the
mungetrackname() gets it has $1. I'll repeat the predefined function
here:


        mungetrackname ()
        {
                mungefilename $@
        }


When that function calls mungefilename() the unquoted $@ splits $1 ("Two
Hands") to two arguments and the mungefilename() will get $1 ("Two") and
$2 ("Hands"). However, as user probably trusts the manual page and uses
only $1 in his custom mungefilename() function. All other parameters are
dropped and this results in song name with only singe word: "Two".

The attached patch fixes those problems with these changes:

  - use on only $1 in the predefined functions (as the man page says)
  - properly quote $1 to prevent shells' word-splitting.
diff --git i/abcde w/abcde
index 44bd94e..7798ef3 100755
--- i/abcde
+++ w/abcde
@@ -3588,31 +3588,31 @@ fi
 # Custom filename munging:
 mungefilename ()
 {
-	echo "$@" | sed -e 's/^\.*//' -e 's/ /_/g' | tr -d ":><|*/\"'?[:cntrl:]"
+	echo "$1" | sed -e 's/^\.*//' -e 's/ /_/g' | tr -d ":><|*/\"'?[:cntrl:]"
 }
 
 # Custom filename munging specific to track names:
 mungetrackname ()
 {
-	mungefilename $@
+	mungefilename "$1"
 }
 
 # Custom filename munging specific to artist names:
 mungeartistname ()
 {
-	mungefilename $@
+	mungefilename "$1"
 }
 
 # Custom filename munging specific to album names:
 mungealbumname ()
 {
-	mungefilename $@
+	mungefilename "$1"
 }
 
 # Custom genre munging:
 mungegenre ()
 {
-	echo $CDGENRE | tr "[:upper:]" "[:lower:]"
+	echo "$1" | tr "[:upper:]" "[:lower:]"
 }
 
 # pre_read
diff --git i/abcde.conf w/abcde.conf
index e255444..7ecfad0 100644
--- i/abcde.conf
+++ w/abcde.conf
@@ -480,28 +480,28 @@
 #   
 #mungefilename ()
 #{
-#	echo "$@" | sed -e 's/^\.*//' -e 's/ /_/g' | tr -d ":><|*/\"'?[:cntrl:]"
+#	echo "$1" | sed -e 's/^\.*//' -e 's/ /_/g' | tr -d ":><|*/\"'?[:cntrl:]"
 #}
 #
 # Custom filename munging specific to track names:
 # By default this function will call the mungefilename function.
 #mungetrackname ()
 #{
-#	mungefilename $@
+#	mungefilename "$1"
 #}
 #
 # Custom filename munging specific to artist names:
 # By default this function will call the mungefilename function.
 #mungeartistname ()
 #{
-#	mungefilename $@
+#	mungefilename "$1"
 #}
 #
 # Custom filename munging specific to album names:
 # By default this function will call the mungefilename function.
 #mungealbumname ()
 #{
-#	mungefilename $@
+#	mungefilename "$1"
 #}
 
 # Custom genre munging:
@@ -510,7 +510,7 @@
 # Uppercase.
 #mungegenre ()
 #{
-#	echo $CDGENRE | tr "[:upper:]" "[:lower:]"
+#	echo "$1" | tr "[:upper:]" "[:lower:]"
 #}
 
 

Reply via email to