On Sat, Sep 06, 2014 at 01:30:54AM +0800, lolilolicon wrote:
Something like `source=('$pkgname-$pkgver.tar.gz')` in the PKGBUILD would "just work" with this eval. This could be a security issue since URLs are arbitrary external data.
Sounds somewhat contrived. External data, yes, but then you might consider that the *entire* PKGBUILD is external data which is fed to makepkg. What makes this particular case special? Is there an immediate problem that this fixes?
Instead, expand the dlagent command line into an array, replace the %o, %u place holders, and run the resultant command line as is.
A shortcoming of this is the dlagent command line cannot have embedded spaces. If this turns out to be a problem, we can do an early eval before substituting %o and %u. --- scripts/makepkg.sh.in | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8e8a64c..a134453 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -288,12 +288,12 @@ get_protocol() { get_downloadclient() { local proto=$1
- # loop through DOWNLOAD_AGENTS variable looking for protocol + # loop through DLAGENTS variable looking for protocol local i for i in "${DLAGENTS[@]}"; do local handler="${i%%::*}" if [[ $proto = "$handler" ]]; then - local agent="${i##*::}" + local agent="${i#*::}"
While I agree with this change (and other similar fixups), it seems unrelated to the rest of the commit...
break fi done @@ -308,8 +308,7 @@ get_downloadclient() { # ensure specified program is installed local program="${agent%% *}" if [[ ! -x $program ]]; then - local baseprog="${program##*/}" - error "$(gettext "The download program %s is not installed.")" "$baseprog" + error "$(gettext "The download program '%s' is not installed.")" "$program" plain "$(gettext "Aborting...")" exit 1 # $E_MISSING_PROGRAM fi @@ -342,15 +341,16 @@ download_file() { local proto=$(get_protocol "$netfile")
# find the client we should use for this URL - local dlcmd - dlcmd=$(get_downloadclient "$proto") || exit $? + local -a cmdline + local IFS=' '
How sure are you that this won't affect some later operation in the function?
+ cmdline=($(get_downloadclient "$proto")) || exit
Why not use read to split the result? IFS=' ' read -a cmdline < <(get_downloadclient "$proto") [[ $cmdline ]] || exit
local filename=$(get_filename "$netfile") local url=$(get_url "$netfile")
if [[ $proto = "scp" ]]; then # scp downloads should not pass the protocol in the url - url="${url##*://}" + url="${url#*://}" fi
msg2 "$(gettext "Downloading %s...")" "$filename" @@ -359,20 +359,18 @@ download_file() { local dlfile="${url##*/}"
# replace %o by the temporary dlfile if it exists - if [[ $dlcmd = *%o* ]]; then - dlcmd=${dlcmd//\%o/\"$filename.part\"} - dlfile="$filename.part" + if [[ ${cmdline[*]} = *%o* ]]; then + dlfile=$filename.part + cmdline=("${cmdline[@]//%o/"$dlfile"}") fi # add the URL, either in place of %u or at the end - if [[ $dlcmd = *%u* ]]; then - dlcmd=${dlcmd//\%u/\"$url\"} + if [[ ${cmdline[*]} = *%u* ]]; then + cmdline=("${cmdline[@]//%u/"$url"}") else - dlcmd="$dlcmd \"$url\"" + cmdline+=("$url") fi
- local ret=0 - eval "$dlcmd >&2 || ret=\$?" - if (( ret )); then + if ! command -- "${cmdline[@]}" >&2; then [[ ! -s $dlfile ]] && rm -f -- "$dlfile" error "$(gettext "Failure while downloading %s")" "$filename" plain "$(gettext "Aborting...")" -- 2.1.0