Skip to content
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

Open
MichaelChirico opened this issue May 18, 2020 · 4 comments
Open

Comments

@MichaelChirico
Copy link
Owner

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

  • Bug author - Neil Schneider
  • Creation time - 2014-12-30 20:46:47 UTC
  • Bugzilla link
  • Status - NEW
  • Alias - None
  • Component - Windows GUI / Window specific
  • Version - R 3.1.2
  • Hardware - x86_64/x64/amd64 (64-bit) Windows 64-bit
  • Importance - P5 enhancement
  • Assignee - R-core
  • URL -
  • Modification time - 2017-01-17 18:18 UTC
@MichaelChirico
Copy link
Owner Author

Created attachment 1703 [details]
File to be passed arguments.


METADATA

  • Comment author - Neil Schneider
  • Timestamp - 2014-12-30 20:47:35 UTC

INCLUDED PATCH

@MichaelChirico
Copy link
Owner Author

Created attachment 1744 [details]
system.R with system2 that maintains backward compatibility

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

  • Comment author - Neil Schneider
  • Timestamp - 2015-02-13 16:12:37 UTC

INCLUDED PATCH

@MichaelChirico
Copy link
Owner Author

Test code would now look like this:

#Does not run

system2("RScript.exe", args=c("C:/Temp files/example.R", "arg1", "arg2 with
spaces", "arg3"))

Fatal error: cannot open file 'C:/Temp': Permission denied

Warning message:
running command '"RScript.exe" C:/Temp files/example.R arg1 arg2 with spaces arg3' had status 2

#Runs with arguments split on spaces

system2("RScript.exe", args=c(shQuote("C:/Temp files/example.R"), "arg1",
"arg2 with spaces", "arg3"))

[1] "hello world!"
[1] "arg1" "arg2" "with" "spaces" "arg3"

#Runs with arguments shQuoted

system2("RScript.exe", args=c("C:/Temp files/example.R", "arg1", "arg2 with
spaces", "arg3"), quote.args = TRUE)

[1] "hello world!"
[1] "arg1" "arg2 with spaces" "arg3"


METADATA

  • Comment author - Neil Schneider
  • Timestamp - 2015-02-13 16:22:39 UTC

@MichaelChirico
Copy link
Owner Author

I noticed this "bug" as well. It's confusing because

  • The system2 help page prominently mentions quoting as a distinguishing feature: "Unlike ‘system’, ‘command’ is always quoted by ‘shQuote’, so it must be a single command without arguments."

  • The arguments for system2 are called "args" and referred to as a "character vector". What's the point of letting the user specify separate arguments if they're just going to be pasted together?

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

  • Comment author - Frederick Eaton
  • Timestamp - 2017-01-17 18:18:15 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant