diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1cb31f3..d56a543 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,12 +1,12 @@ -Contributions to **rio** are welcome from anyone and are best sent as pull requests on [the GitHub repository](https://github.com/leeper/rio/). This page provides some instructions to potential contributors about how to add to the package. +Contributions to **rio** are welcome from anyone and are best sent as pull requests on [the GitHub repository](https://github.com/gesistsa/rio/). This page provides some instructions to potential contributors about how to add to the package. - 1. Contributions can be submitted as [a pull request](https://help.github.com/articles/creating-a-pull-request/) on GitHub by forking or cloning the [repo](https://github.com/leeper/rio/), making changes and submitting the pull request. + 1. Contributions can be submitted as [a pull request](https://help.github.com/articles/creating-a-pull-request/) on GitHub by forking or cloning the [repo](https://github.com/gesistsa/rio/), making changes and submitting the pull request. 2. Pull requests should involve only one commit per substantive change. This means if you change multiple files (e.g., code and documentation), these changes should be committed together. If you don't know how to do this (e.g., you are making changes in the GitHub web interface) just submit anyway and the maintainer will clean things up. 3. All contributions must be submitted consistent with the package license ([GPL-2](http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html)). - 4. All contributions need to be noted in the `Authors@R` field in the [DESCRIPTION](https://github.com/leeper/rio/blob/master/DESCRIPTION). Just follow the format of the existing entries to add your name (and, optionally, email address). Substantial contributions should also be noted in [`inst/CITATION`](https://github.com/leeper/rio/blob/master/inst/CITATION). + 4. All contributions need to be noted in the `Authors@R` field in the [DESCRIPTION](https://github.com/gesistsa/rio/blob/master/DESCRIPTION). Just follow the format of the existing entries to add your name (and, optionally, email address). Substantial contributions should also be noted in [`inst/CITATION`](https://github.com/gesistsa/rio/blob/master/inst/CITATION). 5. This package uses royxgen code and documentation markup, so changes should be made to roxygen comments in the source code `.R` files. If changes are made, roxygen needs to be run. The easiest way to do this is a command line call to: `Rscript -e devtools::document()`. Please resolve any roxygen errors before submitting a pull request. @@ -20,10 +20,10 @@ Some specific types of changes that you might make are: - Import is based on S3 dispatch to functions of the form `.import.rio_FORMAT()`. Export works the same, but with `.export.rio_FORMAT()`. New import/export methods should take this form. There's no need to change the body of the `import()` or `export()` functions; S3 will take care of dispatch. All `.import()` methods must accept a `file` and `which` argument: `file` represents the path to the file and `which` can be used to extract sheets or files from multi-object files (e.g., zip, Excel workbooks, etc.). `.export()` methods take two arguments: `file` and `x`, where `file` is the path to the file and `x` is the data frame being exported. Most of the work of import and export methods involves mapping these arguments to their corresponding argument names in the various underlying packages. See the Vignette: `remap`. - - The S3 methods should be documented in [NAMESPACE](https://github.com/leeper/rio/blob/master/NAMESPACE) using `S3method()`, which is handled automatically by roxygen markup in the source code. - - Any new format support needs to be documented in each of the following places: [README.Rmd](https://github.com/leeper/rio/blob/master/README.Rmd), [the vignette](https://github.com/leeper/rio/blob/master/vignettes/rio.Rmd), and the appropriate Rd file in [`/man`](https://github.com/leeper/rio/tree/master/man). + - The S3 methods should be documented in [NAMESPACE](https://github.com/gesistsa/rio/blob/master/NAMESPACE) using `S3method()`, which is handled automatically by roxygen markup in the source code. + - Any new format support needs to be documented in each of the following places: [README.Rmd](https://github.com/gesistsa/rio/blob/master/README.Rmd), [the vignette](https://github.com/gesistsa/rio/blob/master/vignettes/rio.Rmd), and the appropriate Rd file in [`/man`](https://github.com/gesistsa/rio/tree/master/man). - - New formats or new options for handling formats should have a test added in [`/tests/testthat`](https://github.com/leeper/rio/tree/master/tests/testthat) called `test_format_FORMAT.R` that completely covers the function's behavior. This may require adding an example file to [`inst/examples`](https://github.com/leeper/rio/tree/master/inst/examples) (e.g., for testing `import()`). + - New formats or new options for handling formats should have a test added in [`/tests/testthat`](https://github.com/gesistsa/rio/tree/master/tests/testthat) called `test_format_FORMAT.R` that completely covers the function's behavior. This may require adding an example file to [`inst/examples`](https://github.com/gesistsa/rio/tree/master/inst/examples) (e.g., for testing `import()`). 3. Changes requiring a new package dependency should be discussed on the GitHub issues page before submitting a pull request. diff --git a/DESCRIPTION b/DESCRIPTION index 84299ef..057a097 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: rio Type: Package Title: A Swiss-Army Knife for Data I/O -Version: 1.0.2 +Version: 1.0.3 Authors@R: c(person("Jason", "Becker", role = "aut", email = "jason@jbecker.co"), person("Chung-hong", "Chan", role = c("aut", "cre"), email = "chainsawtiney@gmail.com", comment = c(ORCID = "0000-0002-6232-7530")), diff --git a/NEWS.md b/NEWS.md index be0a9ad..fbd0e75 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +# rio 1.0.3 + +* Adds `trust` parameter to functions that are used to load various R environment formats (`.R`, `.Rds`, `.Rdata`, etc). This parameter is defaulted to `TRUE` today to ensure backwards compatibility. A deprecation notice warns this will default to `FALSE` in `rio` 2.0. We are informing users that these data types should only be loaded from trusted sources, which should be affirmatively attested to. + # rio 1.0.2 * For missing files in `import_list` it gives more informative warnings fix #389 diff --git a/R/import_methods.R b/R/import_methods.R index 2a292e2..a966197 100644 --- a/R/import_methods.R +++ b/R/import_methods.R @@ -124,12 +124,14 @@ import_delim <- function(file, which = 1, sep = "auto", header = "auto", strings } #' @export -.import.rio_r <- function(file, which = 1, ...) { +.import.rio_r <- function(file, which = 1, trust = TRUE, ...) { + .check_trust(trust, format = ".R") .docall(dget, ..., args = list(file = file)) } #' @export -.import.rio_dump <- function(file, which = 1, envir = new.env(), ...) { +.import.rio_dump <- function(file, which = 1, envir = new.env(), trust = TRUE, ...) { + .check_trust(trust, format = "dump") source(file = file, local = envir) if (missing(which)) { if (length(ls(envir)) > 1) { @@ -145,23 +147,25 @@ import_delim <- function(file, which = 1, sep = "auto", header = "auto", strings } #' @export -.import.rio_rds <- function(file, which = 1, ...) { +.import.rio_rds <- function(file, which = 1, trust = TRUE, ...) { + .check_trust(trust, format = "Rds") readRDS(file = file) } #' @export -.import.rio_rdata <- function(file, which = 1, envir = new.env(), ...) { +.import.rio_rdata <- function(file, which = 1, envir = new.env(), trust = TRUE, ...) { + .check_trust(trust, format = "RData") load(file = file, envir = envir) if (missing(which)) { if (length(ls(envir)) > 1) { - warning("Rdata file contains multiple objects. Returning first object.") + warning("Rdata file contains multiple objects. Returning first object.") } - which <- 1 + which <- 1 } if (is.numeric(which)) { - get(ls(envir)[which], envir) + get(ls(envir)[which], envir) } else { - get(ls(envir)[grep(which, ls(envir))[1]], envir) + get(ls(envir)[grep(which, ls(envir))[1]], envir) } } diff --git a/R/utils.R b/R/utils.R index d781128..534dd2c 100644 --- a/R/utils.R +++ b/R/utils.R @@ -136,3 +136,16 @@ escape_xml <- function(x, replacement = c("&", """, "<", ">", "&a } return(file) } + +.check_trust <- function(trust, format) { + lifecycle::deprecate_warn( + when = "2.0.0", + what = "import(trust)", + details = paste0("Trust will be set to FALSE by default for ", format, ".") + ) + if (isFALSE(trust)) { + stop(format, "files may execute arbitary code. Only load", format, "files that you personally generated or can trust the origin.", call. = FALSE) + } + NULL +} + diff --git a/tests/testthat/test_format_R.R b/tests/testthat/test_format_R.R index bb6eee1..e11f619 100644 --- a/tests/testthat/test_format_R.R +++ b/tests/testthat/test_format_R.R @@ -13,3 +13,13 @@ test_that("Export / Import to .R dump file", { expect_true(is.data.frame(import(iris_file))) }) }) + +test_that("Deprecation of untrusted dump", { + withr::with_tempfile("iris_file", fileext = ".dump", code = { + export(iris, iris_file) + ## expect deprecation to work + lifecycle::expect_deprecated(import(iris_file), regexp = "set to FALSE by default") + ## expect false to error + expect_error(import(iris_file, trust = FALSE)) + }) +}) diff --git a/tests/testthat/test_format_rdata.R b/tests/testthat/test_format_rdata.R index 4a9ea5e..ce34011 100644 --- a/tests/testthat/test_format_rdata.R +++ b/tests/testthat/test_format_rdata.R @@ -26,6 +26,13 @@ test_that("Export to and import from Rdata", { ## expect error otherwise expect_error(export(iris$Species, iris_file)) }) + withr::with_tempfile("iris_file", fileext = ".Rdata", code = { + export(iris, iris_file) + ## expect deprecation to work + lifecycle::expect_deprecated(import(iris_file), regexp = "set to FALSE by default") + ## expect false to error + expect_error(import(iris_file, trust = FALSE)) + }) }) test_that("Export to and import from rda", { diff --git a/tests/testthat/test_format_rds.R b/tests/testthat/test_format_rds.R index 545786f..281d253 100644 --- a/tests/testthat/test_format_rds.R +++ b/tests/testthat/test_format_rds.R @@ -14,3 +14,14 @@ test_that("Export to rds (non-data frame)", { expect_true(length(import(list_file)) == 2L) }) }) + +test_that("Deprecation of untrusted rds", { + withr::with_tempfile("iris_file", fileext = ".rds", code = { + export(iris, iris_file) + ## expect deprecation to work + lifecycle::expect_deprecated(import(iris_file), regexp = "set to FALSE by default") + ## expect false to error + expect_error(import(iris_file, trust = FALSE)) + }) +}) +