Project.git(list/str): reduce reliance on shlex.split() #683
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marc-hb
force-pushed
the
fix-quote-bug
branch
from
68e4293
to
f9bbb5c
Compare
Keep up with the times and new policies. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
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 3ac600a ("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 zephyrproject-rtos#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>
marc-hb
force-pushed
the
fix-quote-bug
branch
from
f9bbb5c
to
6325366
Compare
mbolivar-ampere
approved these changes
Sep 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 3ac600a ("git: clean west ref space after fetching") where the caller was not careful and concatenated
update-ref -dwith 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:
While at it, also convert to a list a couple non-risky but very frequent invocations. This speeds up the test hack above.