From c0c89d5f5164078cc69b4e6cd4c3a4a04ec22614 Mon Sep 17 00:00:00 2001 From: "M. Vefa Bicakci" Date: Wed, 29 Jun 2022 20:08:37 +0000 Subject: [PATCH] stx: Do not use shell for curl and wget When using a download URL that includes the character '&' (as part of a query string), the following build failure is encountered, because the '&' character was is interpreted by the shell as a special character for backgrounding the 'wget' command. debrepack - ERROR: 2022-06-29 20:04:31 (3.79 MB/s) - \ '...' saved [...] debrepack - ERROR: [ Failed - "wget -t 5 --wait=15 \ http://a_url_with_& -O ..." ] This issue is resolved by not using the shell for spawning download commands. (That is, by setting "shell=False" in Python's subprocess.Popen constructor.) This commit implements an in-place solution that detects the type of the argument and sets the "shell=" keyword argument to subprocess.Popen accordingly. Verification: - Downloading from URLs with '&' characters works as expected as part of the "debrepack" phase of build-pkgs. - No negative behaviour observed with a build from scratch. Closes-Bug: 1988343 Change-Id: I2a529f60b9a57b6d139f95d31bcd18c51a0fbecb Signed-off-by: M. Vefa Bicakci --- build-tools/stx/debrepack.py | 7 ++++--- build-tools/stx/downloader | 6 +++--- build-tools/stx/utils.py | 12 ++++++++++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/build-tools/stx/debrepack.py b/build-tools/stx/debrepack.py index 3245367c..f5825855 100755 --- a/build-tools/stx/debrepack.py +++ b/build-tools/stx/debrepack.py @@ -187,10 +187,11 @@ def checksum(dl_file, checksum, cmd, logger): def download(url, savepath, logger): - logger.info(f"Download {url} to {savepath}") - download_cmd = "wget -t 5 --wait=15 %s -O %s" - run_shell_cmd(download_cmd % (url, savepath), logger) + + # Need to avoid using the shell as the URL may include '&' characters. + run_shell_cmd(["wget", "-t", "5", "--wait=15", url, "-O", savepath], logger) + return True diff --git a/build-tools/stx/downloader b/build-tools/stx/downloader index e112bde5..2596341d 100755 --- a/build-tools/stx/downloader +++ b/build-tools/stx/downloader @@ -227,9 +227,9 @@ class DebDownloader(BaseDownloader): dl_file = '_'.join([_name, _version, self.arch]) + '.deb' ret = os.path.join(self.dl_dir, dl_file) tmp_file = ".".join([ret, "tmp"]) - dl_cmd = "rm -rf %s; curl -k -f %s -o %s" % (tmp_file, url, tmp_file) - utils.run_shell_cmd(dl_cmd, logger) - utils.run_shell_cmd("mv %s %s" % (tmp_file, ret), logger) + utils.run_shell_cmd(["rm", "-rf", tmp_file], logger) + utils.run_shell_cmd(["curl", "-k", "-f", url, "-o", tmp_file], logger) + utils.run_shell_cmd(["mv", tmp_file, ret], logger) return ret try: diff --git a/build-tools/stx/utils.py b/build-tools/stx/utils.py index 96128d97..b205a9c5 100755 --- a/build-tools/stx/utils.py +++ b/build-tools/stx/utils.py @@ -122,11 +122,19 @@ def limited_walk(dir, max_depth=1): def run_shell_cmd(cmd, logger): + if type(cmd) is str: + shell = True + cmd_str = cmd + elif type(cmd) in (tuple, list): + shell = False + cmd_str = " ".join(cmd) + else: + raise Exception("Unrecognized 'cmd' type '%s'. Must be one of [str, list, tuple]." % (type(cmd))) - logger.info(f'[ Run - "{cmd}" ]') + logger.info(f'[ Run - "{cmd_str}" ]') try: process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - universal_newlines=True, shell=True) + universal_newlines=True, shell=shell) # process.wait() outs, errs = process.communicate() except Exception: