west cannot process a git branch name containing a single quote ' #679
Labels
Comments
|
Thanks for the report! I think I know what's happening and how to fix it but this looks tedious to reproduce because the quote must be in a remote branch, not just in a local branch. Can you share a public manifest/repo that reproduces? A small one if possible. |
https://github.com/mbolivar-ampere/west/tree/test-'-for-marc |
|
Thanks @mbolivar-ampere . I still cannot reproduce, probably because the management of @GarrettCarter-eaton which west version are you using? Please look at this bug template for "inspiration": https://github.com/zephyrproject-rtos/zephyr/issues/new?assignees=&labels=bug&projects=&template=001_bug_report.md&title= |
|
To Reproduce
I'm noticing the behavior differs depending on what revision of the dependency repo you specify. When I specify a git SHA, West seems to process all the refs in the repo which leads to the quote processing failure. Expected behavior Impact Logs and console output Environment
|
marc-hb
added a commit
to marc-hb/west
that referenced
this issue
Sep 1, 2023
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
added a commit
to marc-hb/west
that referenced
this issue
Sep 1, 2023
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>
|
Thanks @GarrettCarter-eaton for the test case, it really saved my time. Fix submitted in #683, would you be in a position to test it? Check the west/README.txt for installing directly from source. If you go and do that, do |
marc-hb
added a commit
to marc-hb/west
that referenced
this issue
Sep 1, 2023
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>
mbolivar-ampere
pushed a commit
that referenced
this issue
Sep 1, 2023
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 #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>
|
Please re-open if not fixed by #683 |
|
Just FYI, tested with c936a4a and issue is fixed. |
|
I can confirm that this bug is resolved in the v1.2.0 release of West. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
If a repo that west clones and processes contains a branch name using a single quote, the following error is received:
Example branch name:
LTK-18338-No-link-for-privacy-policy-on-WebUI's-Login-pageThe text was updated successfully, but these errors were encountered: