Project.git(list/str): reduce reliance on shlex.split()
For convenience, Project.git() supports passing either a list (good) or
a string with whitespaces (bad). The latter is parsed with shlex.split()
This saves some typing but the caller has to be extremely careful to
never use the shlex.split() convenience with unsanitized inputs.
Fixes commit 3ac600acaa
("git: clean west ref space after fetching")
where the caller was not careful and concatenated `update-ref -d ` with
unsanitized input, possibly containing special characters as found in
bug #679. Fix this bug by converting the string to a list.
While at it, look for a few other, frequent and risky invocations and
convert their string argument to a list too. The following test hack was
used to semi-automate the search for these other locations:
```
--- a/src/west/manifest.py
+++ b/src/west/manifest.py
@@ -897,6 +897,8 @@ class Project:
:param cwd: directory to run git in (default: ``self.abspath``)
'''
if isinstance(cmd, str):
+ print(cmd)
+ breakpoint()
cmd_list = shlex.split(cmd)
else:
cmd_list = list(cmd)
```
While at it, also convert to a list a couple non-risky but very frequent
invocations. This speeds up the test hack above.
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
This commit is contained in:
parent
bcd58e0e38
commit
41007648d9
|
@ -685,7 +685,7 @@ class Compare(_ProjectCommand):
|
|||
if self.ignore_branches:
|
||||
return False
|
||||
|
||||
return bool(project.git('branch --show-current',
|
||||
return bool(project.git(['branch', '--show-current'],
|
||||
capture_stdout=True,
|
||||
capture_stderr=True).stdout.strip())
|
||||
|
||||
|
@ -1226,7 +1226,7 @@ class Update(_ProjectCommand):
|
|||
# out the new detached HEAD, then print some helpful context.
|
||||
if take_stats:
|
||||
start = perf_counter()
|
||||
project.git('checkout --detach ' + sha)
|
||||
project.git(['checkout', '--detach', sha])
|
||||
if take_stats:
|
||||
stats['checkout new manifest-rev'] = perf_counter() - start
|
||||
self.post_checkout_help(project, current_branch,
|
||||
|
@ -1318,7 +1318,7 @@ class Update(_ProjectCommand):
|
|||
# This remote is added as a convenience for the user.
|
||||
# However, west always fetches project data by URL, not name.
|
||||
# The user is therefore free to change the URL of this remote.
|
||||
project.git(f'remote add -- {project.remote_name} {project.url}')
|
||||
project.git(['remote', 'add', '--', project.remote_name, project.url])
|
||||
else:
|
||||
self.small_banner(f'{project.name}: cloning from {cache_dir}')
|
||||
# Clone the project from a local cache repository. Set the
|
||||
|
@ -1889,14 +1889,14 @@ def _clean_west_refspace(project):
|
|||
# Clean the refs/west space to ensure they do not show up in 'git log'.
|
||||
|
||||
# Get all the ref names that start with refs/west/.
|
||||
list_refs_cmd = ('for-each-ref --format="%(refname)" -- ' +
|
||||
QUAL_REFS + '**')
|
||||
list_refs_cmd = ['for-each-ref', '--format=%(refname)', '--',
|
||||
QUAL_REFS + '**']
|
||||
cp = project.git(list_refs_cmd, capture_stdout=True)
|
||||
west_references = cp.stdout.decode('utf-8').strip()
|
||||
|
||||
# Safely delete each one.
|
||||
for ref in west_references.splitlines():
|
||||
delete_ref_cmd = 'update-ref -d ' + ref
|
||||
delete_ref_cmd = ['update-ref', '-d', ref]
|
||||
project.git(delete_ref_cmd)
|
||||
|
||||
def _update_manifest_rev(project, new_manifest_rev):
|
||||
|
|
|
@ -951,7 +951,7 @@ class Project:
|
|||
# Though we capture stderr, it will be available as the stderr
|
||||
# attribute in the CalledProcessError raised by git() in
|
||||
# Python 3.5 and above if this call fails.
|
||||
cp = self.git(f'rev-parse {rev}^{{commit}}', capture_stdout=True,
|
||||
cp = self.git(['rev-parse', f'{rev}^{{commit}}'], capture_stdout=True,
|
||||
cwd=cwd, capture_stderr=True)
|
||||
# Assumption: SHAs are hex values and thus safe to decode in ASCII.
|
||||
# It'll be fun when we find out that was wrong and how...
|
||||
|
@ -1023,7 +1023,7 @@ class Project:
|
|||
# instead, which prints an empty string (i.e., just a newline,
|
||||
# which we strip) for the top-level directory.
|
||||
_logger.debug(f'{self.name}: checking if cloned')
|
||||
res = self.git('rev-parse --show-cdup', check=False, cwd=cwd,
|
||||
res = self.git(['rev-parse', '--show-cdup'], check=False, cwd=cwd,
|
||||
capture_stderr=True, capture_stdout=True)
|
||||
|
||||
return not (res.returncode or res.stdout.strip())
|
||||
|
|
Loading…
Reference in New Issue