Hi Simon,
On Tue, Jan 10, 2017 at 07:53:48PM +0000, Simon McVittie wrote:
> I'm currently using git-buildpackage with a custom --builder command
> (vectis, https://github.com/smcv/vectis) that among other things accepts
> a command-line option of the form:
>
> --extra-repository='deb [trusted=yes] http://localhost/path suite
> component'
>
> With git-buildpackage, I have to backslash-escape the spaces:
>
> --extra-repository='deb\ [trusted=yes]\ ...'
>
> or it will treat "--extra-repository=deb", "[trusted=yes]", etc. as
> separate command-line arguments for vectis. This is because
> gbp.command_wrappers.Command(..., shell=True) just joins the command
> to the arguments with " ".join() rather than doing proper shell quoting.
> That's correct for the --builder as currently documented, but not for
> its arguments.
>
> It would be easy to run into this with debuild if you're using something
> like gbp buildpackage --set-envvar=DEBFULLNAME="Simon McVittie"
> (a debuild option) or gbp buildpackage --check-command="lintian -Ii"
> (a dpkg-buildpackage option).
Thanks for the detailed description. Interesting that this went
unnoticed for so long. I've added a test so we hopefully don't regress
there.
>
> Doing proper shell quoting in Python 3 is trival (shlex.quote, which
> I use a lot in vectis) but Python 2 doesn't have that function. However,
> there is another way to get the --builder interpreted as a shell
> one-liner and still pass it arguments: you can invoke
>
> ["sh", "-c", ('%s "$@"' % builder), "sh"] + arguments
I went for using pipes.quote which is python2's equivalent of
shlex.quote and fixed up buildpackage-rpm as well so we're consistent.
Decided to leave quoting to the caller rather than doing it in Command
itself to be consistent between shell=True and cases where command
itself is already a script with arguments from e.g. gbp.conf like
hooks.
Cheers,
-- Guido
>
> with shell=False, and it will do the right thing. The first "sh" in
> that monster is argv[0] for the subprocess, and the second is $0 for
> the shell one-liner.
>
> I attach a possible patch.
>
> S
> From a088aef91ffa9de60c6d7ac3278360ce0ee571e1 Mon Sep 17 00:00:00 2001
> From: Simon McVittie <s...@debian.org>
> Date: Tue, 10 Jan 2017 19:49:08 +0000
> Subject: [PATCH] buildpackage: don't treat dpkg arguments as shell input
>
> If one of gbp buildpackage's command-line arguments (after processing
> by the calling shell if applicable) includes spaces or shell
> metacharacters, it's unexpected to subject it to a layer of shell
> interpretation. Conversely, the builder option is documented to be
> a shell command and so must go through one layer of shell
> interpretation.
>
> This makes an invocation like
>
> gbp buildpackage --check-command="lintian -Ii"
>
> work the way it was presumably intended.
> ---
> gbp/scripts/buildpackage.py | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/gbp/scripts/buildpackage.py b/gbp/scripts/buildpackage.py
> index 6524f01..c7ad903 100755
> --- a/gbp/scripts/buildpackage.py
> +++ b/gbp/scripts/buildpackage.py
> @@ -727,7 +727,11 @@ def main(argv):
> )(dir=build_dir)
>
> # Finally build the package:
> - RunAtCommand(options.builder, dpkg_args, shell=True,
> + RunAtCommand('sh',
> + ['-c', options.builder + ' "$@"',
> + 'sh'] + # $0 for the shell one-liner
> + dpkg_args, # $@ for the shell one-liner
> + shell=False, # we start the shell explicitly
> extra_env=Hook.md(build_env,
> {'GBP_BUILD_DIR': build_dir})
> )(dir=build_dir)
> --
> 2.11.0
>
