support/download: protect from custom commands with spaces in args
PATCH 1/1 support/download: protect from custom commands with spaces in args
Some users may provide custom download commands with spaces in their arguments, like so:
BR2_HG="hg --config foo.bar='some space-separated value'"
However, the way we currently call those commands does not account for the extra quotes, and each space-separated part of the command is interpreted as separate arguments.
Fix that by calling 'eval' on the commands.
Because of the eval, we must further quote our own arguments, to avoid the eval further splitting them in case there are spaces (even though we do not support paths with spaces, better be clean from the onset to avoid breakage in the future).
We change all the wrappers to use a wrapper-function, even those with a single call, so they all look alike.
Note that we do not single-quote some of the variables, like ${verbose} because it can be empty and we really do not want to generate an empty-string argument. That's not a problem, as ${verbose} would not normally contain space-separated values (it could get set to something like '-q -v' but in that case we'd still want two arguments, so that's fine).
--- support/download/bzr | 8 +++++++- support/download/cp | 8 +++++++- support/download/cvs | 10 ++++++++-- support/download/git | 14 ++++++++++---- support/download/hg | 14 ++++++++++---- support/download/scp | 8 +++++++- support/download/svn | 8 +++++++- support/download/wget | 8 +++++++- 8 files changed, 63 insertions(+), 15 deletions(-)
support/download/bzr +7 -1
@@ -26,4 +26,10 @@ repo="${2}" rev="${3}" basename="${4}" -${BZR} export ${verbose} --root="${basename}/" --format=tgz "${output}" "${repo}" -r "${rev}" +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_bzr() { + eval ${BZR} "${@}" +} + +_bzr export ${verbose} --root="'${basename}/'" --format=tgz "'${output}'" "'${repo}'" -r "'${rev}'"
support/download/cp +7 -1
@@ -28,4 +28,10 @@ shift $((OPTIND-1)) output="${1}" source="${2}" -${LOCALFILES} ${verbose} "${source}" "${output}" +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_localfiles() { + eval ${LOCALFILES} "${@}" +} + +_localfiles ${verbose} "'${source}'" "'${output}'"
support/download/cvs +8 -2
@@ -26,6 +26,12 @@ rev="${3}" rawname="${4}" basename="${5}" +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_cvs() { + eval ${CVS} "${@}" +} + if [[ ${rev} =~ ^[0-9] ]]; then # Date, because a tag or a branch cannot begin with a number select="-D"
@@ -35,7 +41,7 @@ else fi export TZ=UTC -${CVS} ${verbose} -z3 -d":pserver:anonymous@${repo}" \ - co -d "${basename}" ${select} "${rev}" -P "${rawname}" +_cvs ${verbose} -z3 -d"':pserver:anonymous@${repo}'" \ + co -d "'${basename}'" ${select} "'${rev}'" -P "'${rawname}'" tar czf "${output}" "${basename}"
support/download/git +10 -4
@@ -25,6 +25,12 @@ repo="${2}" cset="${3}" basename="${4}" +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_git() { + eval ${GIT} "${@}" +} + # Try a shallow clone, since it is faster than a full clone - but that only # works if the version is a ref (tag or branch). Before trying to do a shallow # clone we check if ${cset} is in the list provided by git ls-remote. If not
@@ -33,9 +39,9 @@ basename="${4}" # Messages for the type of clone used are provided to ease debugging in case of # problems git_done=0 -if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then +if [ -n "$(_git ls-remote "'${repo}'" "'${cset}'" 2>&1)" ]; then printf "Doing shallow clone\n" - if ${GIT} clone ${verbose} --depth 1 -b "${cset}" --bare "${repo}" "${basename}"; then + if _git clone ${verbose} --depth 1 -b "'${cset}'" --bare "'${repo}'" "'${basename}'"; then git_done=1 else printf "Shallow clone failed, falling back to doing a full clone\n"
@@ -43,10 +49,10 @@ if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then fi if [ ${git_done} -eq 0 ]; then printf "Doing full clone\n" - ${GIT} clone ${verbose} --mirror "${repo}" "${basename}" + _git clone ${verbose} --mirror "'${repo}'" "'${basename}'" fi GIT_DIR="${basename}" \ -${GIT} archive --prefix="${basename}/" -o "${output}.tmp" --format=tar "${cset}" +_git archive --prefix="'${basename}/'" -o "'${output}.tmp'" --format=tar "'${cset}'" gzip <"${output}.tmp" >"${output}"
support/download/hg +10 -4
@@ -25,8 +25,14 @@ repo="${2}" cset="${3}" basename="${4}" -${HG} clone ${verbose} --noupdate "${repo}" "${basename}" +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_hg() { + eval ${HG} "${@}" +} -${HG} archive ${verbose} --repository "${basename}" --type tgz \ - --prefix "${basename}" --rev "${cset}" \ - "${output}" +_hg clone ${verbose} --noupdate "'${repo}'" "'${basename}'" + +_hg archive ${verbose} --repository "'${basename}'" --type tgz \ + --prefix "'${basename}'" --rev "'${cset}'" \ + "'${output}'"
support/download/scp +7 -1
@@ -23,4 +23,10 @@ shift $((OPTIND-1)) output="${1}" url="${2}" -${SCP} ${verbose} "${url}" "${output}" +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_scp() { + eval ${SCP} "${@}" +} + +_scp ${verbose} "'${url}'" "'${output}'"
support/download/svn +7 -1
@@ -25,6 +25,12 @@ repo="${2}" rev="${3}" basename="${4}" -${SVN} export ${verbose} "${repo}@${rev}" "${basename}" +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_svn() { + eval ${SVN} "${@}" +} + +_svn export ${verbose} "'${repo}@${rev}'" "'${basename}'" tar czf "${output}" "${basename}"
support/download/wget +7 -1
@@ -23,4 +23,10 @@ shift $((OPTIND-1)) output="${1}" url="${2}" -${WGET} ${verbose} -O "${output}" "${url}" +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_wget() { + eval ${WGET} "${@}" +} + +_wget ${verbose} -O "'${output}'" "'${url}'"
Signature · 1 line
1.9.1
PATCH 1/1 support/download: protect from custom commands with spaces in args
Quoted 25 lines · Yann E. MORIN
Some users may provide custom download commands with spaces in their arguments, like so:
BR2_HG="hg --config foo.bar='some space-separated value'"However, the way we currently call those commands does not account for the extra quotes, and each space-separated part of the command is interpreted as separate arguments.
Fix that by calling 'eval' on the commands.
Because of the eval, we must further quote our own arguments, to avoid the eval further splitting them in case there are spaces (even though we do not support paths with spaces, better be clean from the onset to avoid breakage in the future).
We change all the wrappers to use a wrapper-function, even those with a single call, so they all look alike.
Note that we do not single-quote some of the variables, like ${verbose} because it can be empty and we really do not want to generate an empty-string argument. That's not a problem, as ${verbose} would not normally contain space-separated values (it could get set to something like '-q -v' but in that case we'd still want two arguments, so that's fine).
Quoted 3 lines
Reported-by Thomas De Schampheleire <[email protected]>Signed-off-by "Yann E. MORIN" <[email protected]>Cc Thomas De Schampheleire <[email protected]>
[tests: regular hg, wget, scp, git downloads + custom hg with spaces in command-line]
Thomas Petazzoni
AppliedPATCH 1/1 support/download: protect from custom commands with spaces in args
In reply to Yann E. MORINDear Yann E. MORIN,
Quoted 28 lines · Yann E. MORIN
Some users may provide custom download commands with spaces in their arguments, like so:
BR2_HG="hg --config foo.bar='some space-separated value'"However, the way we currently call those commands does not account for the extra quotes, and each space-separated part of the command is interpreted as separate arguments.
Fix that by calling 'eval' on the commands.
Because of the eval, we must further quote our own arguments, to avoid the eval further splitting them in case there are spaces (even though we do not support paths with spaces, better be clean from the onset to avoid breakage in the future).
We change all the wrappers to use a wrapper-function, even those with a single call, so they all look alike.
Note that we do not single-quote some of the variables, like ${verbose} because it can be empty and we really do not want to generate an empty-string argument. That's not a problem, as ${verbose} would not normally contain space-separated values (it could get set to something like '-q -v' but in that case we'd still want two arguments, so that's fine).
Reported-by Thomas De Schampheleire <[email protected]>Signed-off-by "Yann E. MORIN" <[email protected]>Cc Thomas De Schampheleire <[email protected]>
Quoted 10 lines
--- support/download/bzr | 8 +++++++- support/download/cp | 8 +++++++- support/download/cvs | 10 ++++++++-- support/download/git | 14 ++++++++++---- support/download/hg | 14 ++++++++++---- support/download/scp | 8 +++++++- support/download/svn | 8 +++++++- support/download/wget | 8 +++++++- 8 files changed, 63 insertions(+), 15 deletions(-)
Applied, thanks.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 support/download: protect from custom commands with spaces in args
Andi, All,
On 2016-01-24 11:06 +0900, Andi Shyti spake thusly:
Quoted 4 lines
Quoted 3 lines
Some users may provide custom download commands with spaces in their arguments, like so:
BR2_HG="hg --config foo.bar='some space-separated value'"
Quoted 8 lines
this patch is breaking the build. With configs like this:
BR2_LINUX_KERNEL=y
BR2_LINUX_KERNEL_CUSTOM_GIT=y BR2_LINUX_KERNEL_CUSTOM_REPO_URL="git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git" BR2_LINUX_KERNEL_DEFCONFIG="exynos"
BR2_LINUX_KERNEL_DTS_SUPPORT=y BR2_LINUX_KERNEL_INTREE_DTS_NAME="exynos5422-odroidxu4"
You forgot to specify a kernel version here, which is the ultimate reason for the failure.
It does not make sense to use a git tree without specifying either a tag or a sha1 to checkout.
Quoted 11 lines
the command sent is:
git clone --mirror '
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git' ''git dosn't like empty strngs and provides this output:
fatal: The empty string is not a valid path
Besides, I have never seen spaces in branch name, repositories names and it is quite a corner case. I think this patch should be reverted (at least) in the case of git.
No, the patch is correct. What we should however check is that the version string is not empty, and bail out on that error instead.
Somehow, when the user forgets to specify a version string, we call the download scripts with no basename, and the empty string you see in the git call above should have been the basename of the directory to clone into, i.e. something like: linux-VERSION/
Care to have a look into that? ;-)
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 support/download: protect from custom commands with spaces in args
Hi Yann,
Quoted 15 lines
Quoted 6 lines
Quoted 5 lines
Some users may provide custom download commands with spaces in their
arguments, like so:BR2_HG="hg --config foo.bar='some space-separated value'"Quoted 8 lines
this patch is breaking the build. With configs like this:
BR2_LINUX_KERNEL=y
BR2_LINUX_KERNEL_CUSTOM_GIT=y BR2_LINUX_KERNEL_CUSTOM_REPO_URL="git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git" BR2_LINUX_KERNEL_DEFCONFIG="exynos"
BR2_LINUX_KERNEL_DTS_SUPPORT=y BR2_LINUX_KERNEL_INTREE_DTS_NAME="exynos5422-odroidxu4"
Quoted 7 lines
You forgot to specify a kernel version here, which is the ultimate
reason for the failure.It does not make sense to use a git tree without specifying either a tag
or a sha1 to checkout.
actually I haven't forgotten, but I normally expect to clone master out of a git clone without having specified any branch/sha1/tag (the way it was working before).
Indeed, by setting
BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION="master"
it worked.
Quoted 21 lines
Quoted 8 lines
the command sent is:
git clone --mirror '
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git' ''git dosn't like empty strngs and provides this output:
Quoted 7 lines
fatal: The empty string is not a valid path
Besides, I have never seen spaces in branch name, repositories
names and it is quite a corner case. I think this patch should be
reverted (at least) in the case of git.No, the patch is correct. What we should however check is that the
version string is not empty, and bail out on that error instead.
Quoted 7 lines
Somehow, when the user forgets to specify a version string, we call the
download scripts with no basename, and the empty string you see in the
git call above should have been the basename of the directory to clone
into, i.e. something like: linux-VERSION/
I think that buildroot should not fail if the version is not specified, but go ahead with a default solution, which can be master.
Something like this, perhaps?
linux/linux.mk | 5 +++++ 1 file changed, 5 insertions(+)
linux/linux.mk +5 -0
@@ -20,6 +20,11 @@ LINUX_SITE_METHOD = local else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_GIT),y) LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL)) LINUX_SITE_METHOD = git + +ifeq ($(strip $(LINUX_VERSION)),) +LINUX_VERSION = "master" +endif + else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_HG),y) LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL)) LINUX_SITE_METHOD = hg
Thanks,
Andi
PATCH 1/1 support/download: protect from custom commands with spaces in args
Quoted 80 lines · Andi Shyti
Hi Yann,
Quoted 15 lines
Quoted 6 lines
Quoted 5 lines
Some users may provide custom download commands with spaces in their
arguments, like so:BR2_HG="hg --config foo.bar='some space-separated value'"Quoted 8 lines
this patch is breaking the build. With configs like this:
BR2_LINUX_KERNEL=y
BR2_LINUX_KERNEL_CUSTOM_GIT=y BR2_LINUX_KERNEL_CUSTOM_REPO_URL="git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git" BR2_LINUX_KERNEL_DEFCONFIG="exynos"
BR2_LINUX_KERNEL_DTS_SUPPORT=y BR2_LINUX_KERNEL_INTREE_DTS_NAME="exynos5422-odroidxu4"Quoted 7 lines
You forgot to specify a kernel version here, which is the ultimate
reason for the failure.It does not make sense to use a git tree without specifying either a tag
or a sha1 to checkout.actually I haven't forgotten, but I normally expect to clone master out of a git clone without having specified any branch/sha1/tag (the way it was working before).
Indeed, by setting
BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION="master"
it worked.
Quoted 21 lines
Quoted 8 lines
the command sent is:
git clone --mirror '
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git' ''git dosn't like empty strngs and provides this output:
Quoted 7 lines
fatal: The empty string is not a valid path
Besides, I have never seen spaces in branch name, repositories
names and it is quite a corner case. I think this patch should be
reverted (at least) in the case of git.No, the patch is correct. What we should however check is that the
version string is not empty, and bail out on that error instead.Quoted 7 lines
Somehow, when the user forgets to specify a version string, we call the
download scripts with no basename, and the empty string you see in the
git call above should have been the basename of the directory to clone
into, i.e. something like: linux-VERSION/I think that buildroot should not fail if the version is not specified, but go ahead with a default solution, which can be master.
Something like this, perhaps?
linux/linux.mk | 5 +++++ 1 file changed, 5 insertions(+)linux/linux.mk +0
-0@@ -20,6 +20,11 @@ LINUX_SITE_METHOD = local else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_GIT),y)
linux/linux.mk LINUX_SITE_METHOD = local · line 30
Quoted 10 lines
linux/linux.mk +5
-0LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL)) LINUX_SITE_METHOD = git + +ifeq ($(strip $(LINUX_VERSION)),) +LINUX_VERSION = "master" +endif + else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_HG),y) LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL)) LINUX_SITE_METHOD = hg
No, this is bad practice: master is a moving target. Tomorrow it can/will point to another commit than today. If you use such a changing name in a build system, you do not get reproducible builds at all.
That is why it is reasonable to expect a real SHA or tag name for packages coming from version control.
/Thomas