-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUGZILLA #16127] system2 arguments are not wrapped with shQuote #5576
Comments
Created attachment 1703 [details] METADATA
INCLUDED PATCH
|
Created attachment 1744 [details] This is an updated version of the system.R. This version maintains backward compatibility of system2, while adding the requested feature of shQuoting the arguments as well as the command. METADATA
INCLUDED PATCH
|
Test code would now look like this: #Does not run
Fatal error: cannot open file 'C:/Temp': Permission denied Warning message: #Runs with arguments split on spaces
[1] "hello world!" #Runs with arguments shQuoted
[1] "hello world!" METADATA
|
I noticed this "bug" as well. It's confusing because
Certainly the documentation should be fixed, but I think the functionality too. I support Neil's patch, but it's unfortunate that users will now have to put 'quote.args=T' after every system2 invocation. It's almost preferable to deprecate 'system2' and introduce a 'system3' with good defaults. Or make quote.args=T the default. It's not just a UI issue but a security issue as well. Note that "src/library/base/R/unix/system.unix.R" also needs to be updated, as well as the documentation in "src/library/base/man/system2.Rd". Note also that 'shQuote' takes character vector arguments, so I don't see the need for sapply in Neil's patch. Thank you Neil. METADATA
|
Created attachment 1702 [details]
system2 function with arguments wrapped with shQuote
The system2 function wraps the command with shQuote, but does not wrap the arguments. This can cause unintended results if the arguments contains file paths that include spaces.
This can be reproduced using the following code and the attached example.R script. (example.R should be in a file path with a space.)
#Fatal error: cannot open file 'C:/Temp': Permission denied
system2("RScript.exe", "C:/Temp files/example.R")
#Prints "Hello World!" and character(0) since there are no additional arguments.
system2("RScript.exe", shQuote("C:/Temp files/example.R"))
#Sending additional arguments to example.R will split the second argument at the spaces.
system2("RScript.exe", args=c(shQuote("C:/Temp files/example.R"), "arg1", "arg2 with spaces", "arg3"))
I have also attached an updated system.R file which will individually wrap the arguments using shQuote and sapply. Line 73-74:
command <- paste(c(shQuote(command), env,
sapply(args, shQuote, USE.NAMES=FALSE)), collapse = " ")
The documentation on system2 states that args is "a character vector of arguments to command". The current code does not differentiate between args="arg1 arg2 arg3" and args=c("arg1", "arg2", "arg3"). This proposed enhancement would better match the documentation, but has the potential to cause problems with people who have used system2 in the non-vector space delimited argument format.
METADATA
The text was updated successfully, but these errors were encountered: