-
Task
-
Resolution: Done
-
Normal
-
7.9
-
None
-
False
-
False
-
8
Background
We have a helper function, utils.run_subproccess(), for running other programs. It takes the command to execute as a string, splits it, and then passes the list to python's subprocess.Popen() to run.
This is okay as long as the code calling run_subprocess() takes care to use shlex.quote() to quote the arguments it is passing. Otherwise things like spaces in filenames will be incorrectly split. Passing the command as a list instead of a string mitigates this by having the programmer calling run_subprocess()put each argument into a separate list element rather than relying on run_subprocess() to split it.
In depth explanation if more information is desired: https://gist.github.com/abadger/ea234ee7a7238b343bcfd6537d6e92e7
Work to be performed
- Change the run_subprocess() code to take a list instead of a string (1 hour, remove the splitting code, modify the docstring, and change the logging to join the list which was passed in.)
- Change all callers of run_subprocess() to send a list instead of a string.
- There are currently 33 calls to run_subprocess(). Examining a grep of the source code, I think about 20 of these will be easy to port, similar to this: https://gist.github.com/abadger/615c9d85a201c3f61c6673172e3a7b46
- The remainder will require varying degrees of work. See pkghandler.call_yum_cmd() for an example that requires a lot of changes (two of the callers of call_yum_cmd() also need one-line modifications): https://github.com/oamg/convert2rhel/blob/main/convert2rhel/pkghandler.py#L102
- Modify the use of run_subprocess() in the unit_tests()
- There are currently 134 mentions of run_subprocess() in the unit tests.
- The majority of these will not need changes. We're mocking the function to return a specific value (the mock won't care how the function is called)..
- Although not a majority, there are many tests where we check that run_subprocess() is called with the arguments we expect. It will take time to work through all of these but the change is simply to look for the expected arguments as a list instead of a string.