From 2c05be1996186ed59ec2a5e57a8e06b831644c91 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Fri, 3 Jul 2020 10:10:51 -0400 Subject: [PATCH 01/71] add check before guessing content-type --- R/parse-body.R | 2 +- tests/testthat/files/multipart-ctype.bin | 18 ++++++++++++++++++ tests/testthat/test-postbody.R | 7 +++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/files/multipart-ctype.bin diff --git a/R/parse-body.R b/R/parse-body.R index 4e8d3ea58..44f7c9f8a 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -192,7 +192,7 @@ parser_multi <- function() { toparse <- parse_multipart(value, boundary) # content-type detection lapply(toparse, function(x) { - if (!is.null(x$filename)) { + if (x$content_type %in% c(NULL, "application/octet-stream") && !is.null(x$filename)) { x$content_type <- getContentType(tools::file_ext(x$filename)) } parseRaw(x) diff --git a/tests/testthat/files/multipart-ctype.bin b/tests/testthat/files/multipart-ctype.bin new file mode 100644 index 000000000..049999443 --- /dev/null +++ b/tests/testthat/files/multipart-ctype.bin @@ -0,0 +1,18 @@ +-----------------------------90908882332870323642673870272 +Content-Disposition: form-data; name="file"; filename="sample.tsv" +Content-Type: text/tab-separated-values + +x y z +0.118413259 0.708780115 0.220421733 +0.7442446 0.539953502 0.02358455 +0.452216508 0.434335382 0.290591105 +0.177870351 0.329585854 0.867134948 +0.328333594 0.564684198 0.455520456 +0.680873191 0.57619709 0.015501929 +0.87209177 0.4487502 0.008787638 +0.50966466 0.958683547 0.251500478 +0.937677985 0.457557468 0.19860169 +0.352366596 0.49283506 0.161614863 +0.370598572 0.366986682 0.15623896 + +-----------------------------90908882332870323642673870272-- diff --git a/tests/testthat/test-postbody.R b/tests/testthat/test-postbody.R index 1cce315b9..4ad6c5f72 100644 --- a/tests/testthat/test-postbody.R +++ b/tests/testthat/test-postbody.R @@ -67,3 +67,10 @@ test_that("Test multipart parser", { expect_equal(attr(parsed_body[["img1"]], "filename"), "avatar2-small.png") expect_equal(parsed_body[["json"]], list(a=2,b=4,c=list(w=3,t=5))) }) + +test_that("Test multipart respect content-type", { + bin_file <- test_path("files/multipart-ctype.bin") + body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) + parsed_body <- parseBody(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272") + expect_equal(class(parsed_body$file), "character") +}) From 5c78836b63473e97a0956d093a33d695624db860 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Fri, 3 Jul 2020 11:13:49 -0400 Subject: [PATCH 02/71] PR suggestion, gitattributes --- R/parse-body.R | 7 +++++-- tests/testthat/files/.gitattributes | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 tests/testthat/files/.gitattributes diff --git a/R/parse-body.R b/R/parse-body.R index 44f7c9f8a..a5198700f 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -192,8 +192,11 @@ parser_multi <- function() { toparse <- parse_multipart(value, boundary) # content-type detection lapply(toparse, function(x) { - if (x$content_type %in% c(NULL, "application/octet-stream") && !is.null(x$filename)) { - x$content_type <- getContentType(tools::file_ext(x$filename)) + if (is.null(x$content_type) || isTRUE(x$content_type == "application/octet-stream")) { + if (!is.null(x$filename)) { + # Guess content-type from file extension + x$content_type <- getContentType(tools::file_ext(x$filename)) + } } parseRaw(x) }) diff --git a/tests/testthat/files/.gitattributes b/tests/testthat/files/.gitattributes new file mode 100644 index 000000000..291377b6b --- /dev/null +++ b/tests/testthat/files/.gitattributes @@ -0,0 +1 @@ +*.bin binary From 680bd8e5d5798ce979bca8f84a48e69d4062109d Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Fri, 3 Jul 2020 11:16:56 -0400 Subject: [PATCH 03/71] remove to readd --- tests/testthat/files/multipart-ctype.bin | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 tests/testthat/files/multipart-ctype.bin diff --git a/tests/testthat/files/multipart-ctype.bin b/tests/testthat/files/multipart-ctype.bin deleted file mode 100644 index 049999443..000000000 --- a/tests/testthat/files/multipart-ctype.bin +++ /dev/null @@ -1,18 +0,0 @@ ------------------------------90908882332870323642673870272 -Content-Disposition: form-data; name="file"; filename="sample.tsv" -Content-Type: text/tab-separated-values - -x y z -0.118413259 0.708780115 0.220421733 -0.7442446 0.539953502 0.02358455 -0.452216508 0.434335382 0.290591105 -0.177870351 0.329585854 0.867134948 -0.328333594 0.564684198 0.455520456 -0.680873191 0.57619709 0.015501929 -0.87209177 0.4487502 0.008787638 -0.50966466 0.958683547 0.251500478 -0.937677985 0.457557468 0.19860169 -0.352366596 0.49283506 0.161614863 -0.370598572 0.366986682 0.15623896 - ------------------------------90908882332870323642673870272-- From 161164ee1a2725b1b0771515c15b20d4acf65935 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Fri, 3 Jul 2020 11:17:54 -0400 Subject: [PATCH 04/71] readd binary --- tests/testthat/files/multipart-ctype.bin | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 tests/testthat/files/multipart-ctype.bin diff --git a/tests/testthat/files/multipart-ctype.bin b/tests/testthat/files/multipart-ctype.bin new file mode 100644 index 000000000..a76b50942 --- /dev/null +++ b/tests/testthat/files/multipart-ctype.bin @@ -0,0 +1,18 @@ +-----------------------------90908882332870323642673870272 +Content-Disposition: form-data; name="file"; filename="sample.tsv" +Content-Type: text/tab-separated-values + +x y z +0.118413259 0.708780115 0.220421733 +0.7442446 0.539953502 0.02358455 +0.452216508 0.434335382 0.290591105 +0.177870351 0.329585854 0.867134948 +0.328333594 0.564684198 0.455520456 +0.680873191 0.57619709 0.015501929 +0.87209177 0.4487502 0.008787638 +0.50966466 0.958683547 0.251500478 +0.937677985 0.457557468 0.19860169 +0.352366596 0.49283506 0.161614863 +0.370598572 0.366986682 0.15623896 + +-----------------------------90908882332870323642673870272-- From 2954c7400b884cb262c986ee34bfa54bfdee29a2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 13:09:10 -0400 Subject: [PATCH 05/71] Add back ... args to safeFromJSON() --- R/json.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/json.R b/R/json.R index 588ff9efe..dd391f88f 100644 --- a/R/json.R +++ b/R/json.R @@ -1,8 +1,8 @@ #' @importFrom jsonlite validate fromJSON toJSON #' @noRd -safeFromJSON <- function(txt) { +safeFromJSON <- function(txt, ...) { if (!validate(txt)) { stop("Argument 'txt' is not a valid JSON string.") } - fromJSON(txt) + fromJSON(txt, ...) } From 42331559ad15cf0d1bfbdac9ad66894184a6c04a Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 13:15:14 -0400 Subject: [PATCH 06/71] Adjust parsers to allow for arguments. Use direct matches only * Restructure parser list to use the pattern as the key and parser as the value. Allows the same parser to exist under different content type values. * Provide duplicate parsers for commonly seen content types * add tsv parser * add csv parser * Allow for arguments to be supplied to parsers * use other parsers to reduce copy/pasta --- NAMESPACE | 5 +- R/content-types.R | 4 +- R/globals.R | 2 +- R/parse-body.R | 301 +++++++++++++++++++-------------- R/zzz.R | 2 +- man/addParser.Rd | 50 ------ man/add_parser.Rd | 44 +++++ man/parsers.Rd | 71 ++++++-- tests/testthat/test-postbody.R | 18 +- 9 files changed, 295 insertions(+), 202 deletions(-) delete mode 100644 man/addParser.Rd create mode 100644 man/add_parser.Rd diff --git a/NAMESPACE b/NAMESPACE index a111c3ca3..a7502d9dd 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -2,8 +2,8 @@ export(PlumberEndpoint) export(PlumberStatic) -export(addParser) export(addSerializer) +export(add_parser) export(do_configure_https) export(do_deploy_api) export(do_forward) @@ -17,12 +17,15 @@ export(include_html) export(include_md) export(include_rmd) export(options_plumber) +export(parser_csv) export(parser_json) export(parser_multi) export(parser_octet) export(parser_query) export(parser_rds) +export(parser_read_file) export(parser_text) +export(parser_tsv) export(parser_yaml) export(plumb) export(plumber) diff --git a/R/content-types.R b/R/content-types.R index fb2613386..dc8c73e1c 100644 --- a/R/content-types.R +++ b/R/content-types.R @@ -42,7 +42,9 @@ knownContentTypes <- list( dotx='application/vnd.openxmlformats-officedocument.wordprocessingml.template', xlam='application/vnd.ms-excel.addin.macroEnabled.12', xlsb='application/vnd.ms-excel.sheet.binary.macroEnabled.12', - rds='application/rds') + rds='application/rds', + tsv="text/tab-separated-values", + csv="text/csv") getContentType <- function(ext, defaultType='application/octet-stream') { ct <- knownContentTypes[[tolower(ext)]] diff --git a/R/globals.R b/R/globals.R index f33915762..86c556bf9 100644 --- a/R/globals.R +++ b/R/globals.R @@ -1,4 +1,4 @@ .globals <- new.env() .globals$serializers <- list() .globals$processors <- new.env() -.globals$parsers <- list(func = list(), pattern = list()) +.globals$parsers <- list() diff --git a/R/parse-body.R b/R/parse-body.R index 4e8d3ea58..1f19aaacb 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -4,7 +4,7 @@ postBodyFilter <- function(req){ # This will return raw bytes body <- req$rook.input$read() type <- req$HTTP_CONTENT_TYPE - args <- parseBody(body, type) + args <- parse_body(body, type) req$args <- c(req$args, args) req$postBodyRaw <- body if (isTRUE(getOption("plumber.postBody", TRUE))) { @@ -16,77 +16,85 @@ postBodyFilter <- function(req){ forward() } -parseBody <- function(body, content_type = NULL) { +parse_body <- function(body, content_type = NULL) { if (!is.raw(body)) {body <- charToRaw(body)} toparse <- list(value = body, content_type = content_type) - parseRaw(toparse) + parse_raw(toparse) } -parseRaw <- function(toparse) { +parse_raw <- function(toparse) { if (length(toparse$value) == 0L) return(list()) - parser <- parserPicker(toparse$content_type, toparse$value[1], toparse$filename) - do.call(parser(), toparse) + parser <- parser_picker(toparse$content_type, toparse$value[1], toparse$filename) + do.call(parser, toparse) } -parserPicker <- function(content_type, first_byte, filename = NULL) { - #fast default to json when first byte is 7b (ascii {) - if (first_byte == as.raw(123L)) { - return(.globals$parsers$func[["json"]]) - } +parser_picker <- function(content_type, first_byte, filename = NULL) { + parsers <- .globals$parsers + + # parse as a query string if (is.null(content_type)) { - return(.globals$parsers$func[["query"]]) - } - # else try to find a match - patterns <- .globals$parsers$pattern - parser <- .globals$parsers$func[stri_startswith_fixed(content_type, patterns)] - # Should we warn when multiple parsers match? - # warning("Multiple body parsers matches for content-type : ", toparse$content_type, ". Parser ", names(parser)[1L], " used.") - if (length(parser) == 0L) { - if (is.null(filename)) { - return(.globals$parsers$func[["query"]]) - } else { - return(.globals$parsers$func[["octet"]]) + # fast default to json when first byte is 7b (ascii {) + if (first_byte == as.raw(123L)) { + return(parsers$json) } - } else { - return(parser[[1L]]) + + return(parsers$query) } -} + # remove trailing content type information + # "application/json; charset=UTF-8" + # to + # "application/json" + if (grepl(";", content_type, fixed = TRUE)) { + content_type <- strsplit(content_type, ";")[[1]][1] + } + parser <- parsers[[content_type]] + + # return known parser + if (!is.null(parser)) { + return(parser) + } + + # return text parser + if (stri_startswith_fixed(content_type, "text/")) { + # text parser + return(parsers$text) + } + + # query string + if (is.null(filename)) { + return(parsers$query) + } + + # octect + parsers$octet +} -#' Plumber Parsers -#' -#' Parsers are used in Plumber to transform the raw body content received -#' by a request to the API. -#' @name parsers -#' @rdname parsers -NULL #' Add a Parsers #' #' A parser is responsible for decoding the raw body content of a request into #' a list of arguments that can be mapped to endpoint function arguments. #' For instance, the \code{parser_json} parser content-type `application/json`. -#' The list of available parsers in plumber is global. #' -#' @param name The name of the parser (character string) -#' @param parser The parser to be added. -#' @param pattern A pattern to match against the content-type of each part of -#' the request body. +#' @param content_type A string to match against the content-type of each part of +#' the request body +#' @param parser The parser function to be added. This function should possibly +#' accept `value` and the named parameters `content_type` and `filename`. +#' Other parameters may be provided from [webutils::parse_multipart()]. +#' To be safe, add a `...` to your function signature. +#' @param verbose Logical value which determines if a warning should be +#' displayed when patterns are overwritten. #' -#' @details For instance, the \code{parser_json} pattern is `application/json`. -#' If `pattern` is not provided, will be set to `application/{name}`. -#' Detection is done assuming content-type starts with pattern and is -#' case sensitive. +#' @details #' #' Parser function structure is something like below. Available parameters #' to build parser are `value`, `content_type` and `filename` (only available #' in `multipart-form` body). #' ```r -#' parser <- function() { -#' function(value, content_type = "ct", filename, ...) { -#' # do something with raw value -#' } +#' parser <- function(value, content_type = "ct", filename, ...) { +#' # do something with raw value #' } #' ``` #' @@ -94,94 +102,139 @@ NULL #' plumber endpoint function args. #' #' @examples -#' parser_json <- function() { -#' function(value, content_type = "application/json", ...) { -#' charset <- getCharacterSet(content_type) -#' value <- rawToChar(value) -#' Encoding(value) <- charset -#' jsonlite::fromJSON(value) -#' } +#' parser_dcf <- function(value, content_type = "text/x-dcf", ...) { +#' charset <- getCharacterSet(content_type) +#' value <- rawToChar(value) +#' Encoding(value) <- charset +#' read.dcf(value) #' } -#' @md #' @export -addParser <- function(name, parser, pattern = NULL) { - if (is.null(.globals$parsers)) { - .globals$parsers <- list() - } - if (!is.null(.globals$parsers$func[[name]])) { - stop("Already have a parser by the name of ", name) - } - if (is.null(pattern)) { - pattern <- paste0("application/", name) +add_parser <- function(content_type, parser, verbose = TRUE) { + + if (!is.null(.globals$parsers[[content_type]])) { + if (isTRUE(verbose)) { + warning("Overwriting parser: ", content_type) + } } - .globals$parsers$func[[name]] <- parser - .globals$parsers$pattern[[name]] <- pattern -} + stopifnot(is.function(parser)) + + .globals$parsers[[content_type]] <- parser + + invisible(.globals$parsers) +} -#' JSON -#' @rdname parsers +#' Plumber Parsers +#' +#' Parsers are used in Plumber to transform the raw body content received +#' by a request to the API. Extra parameters may be provided to parser +#' functions when adding the parser to plumber. This will allow for +#' non-default behavior. +#' +#' @param ... parameters supplied to the appropriate internal function +#' @describeIn parsers Query string parser +#' @examples +#' \dontrun{ +#' # Overwrite `text/json` parsing behavior to not allow JSON vectors to be simplified +#' add_parser("text/json", parser_json(simplifyVector = FALSE)) +#' } #' @export -parser_json <- function() { - function(value, content_type = NULL, ...) { - charset <- getCharacterSet(content_type) - value <- rawToChar(value) - Encoding(value) <- charset - safeFromJSON(value) - } +parser_query <- function() { + parser_text(parseQS) } +#' @describeIn parsers JSON parser +#' @export +parser_json <- function(...) { + parser_text(function(value) { + safeFromJSON(value, ...) + }) +} -#' QUERY STRING -#' @rdname parsers +#' @describeIn parsers Helper parser to parse plain text +#' @param parse_fn function to further decode a text string into an object #' @export -parser_query <- function() { +parser_text <- function(parse_fn = identity) { + stopifnot(is.function(parse_fn)) function(value, content_type = NULL, ...) { charset <- getCharacterSet(content_type) value <- rawToChar(value) Encoding(value) <- charset - parseQS(value) + parse_fn(value) } } +#' @describeIn parsers CSV parser +#' @export +parser_csv <- function(...) { + parser_text(function(val) { + utils::read.csv(val, ...) + }) +} -#' TEXT -#' @rdname parsers +#' @describeIn parsers TSV parser #' @export -parser_text <- function() { - function(value, content_type = NULL, ...) { - charset <- getCharacterSet(content_type) - value <- rawToChar(value) - Encoding(value) <- charset - value - } +parser_tsv <- function(...) { + parser_text(function(val) { + utils::read.delim(val, ...) + }) } +#' @describeIn parsers YAML parser +#' @export +parser_yaml <- function(...) { + parser_text(function(val) { + if (!requireNamespace("yaml", quietly = TRUE)) { + stop("yaml must be installed for the yaml parser to work") + } + yaml::yaml.load(val, ...) + }) +} - -#" RDS -#' @rdname parsers +#' @describeIn parsers Helper parser that writes the binary post body to a file and reads it back again using `read_fn`. +#' This parser should be used when reading from a file is required. +#' @param read_fn function used to read a the content of a file. Ex: [readRDS()] #' @export -parser_rds <- function() { +parser_read_file <- function(read_fn = readLines) { + stopifnot(is.function(read_fn)) function(value, filename, ...) { tmp <- tempfile("plumb", fileext = paste0("_", basename(filename))) - on.exit(file.remove(tmp), add = TRUE) + on.exit({ + file.remove(tmp) + }, add = TRUE) writeBin(value, tmp) - readRDS(tmp) + read_fn(tmp) } } +#' @describeIn parsers RDS parser +#' @export +parser_rds <- function(...) { + parser_read_file(function(value) { + readRDS(value, ...) + }) +} + -#" MULTI -#' @rdname parsers +#' @describeIn parsers Octet stream parser +#' @export +parser_octet <- function() { + function(value, filename = NULL, ...) { + attr(value, "filename") <- filename + value + } +} + + +#' @describeIn parsers Multi part parser. This parser will then parse each individual body with its respective parser #' @export #' @importFrom webutils parse_multipart parser_multi <- function() { @@ -195,51 +248,45 @@ parser_multi <- function() { if (!is.null(x$filename)) { x$content_type <- getContentType(tools::file_ext(x$filename)) } - parseRaw(x) + parse_raw(x) }) } } +add_parsers_onLoad <- function() { -#' OCTET -#' @rdname parsers -#' @export -parser_octet <- function() { - function(value, filename = NULL, ...) { - attr(value, "filename") <- filename - return(value) - } -} + # add both `application/XYZ` and `text/XYZ` parsers + for (type in c("application", "text")) { + mime_type <- function(x) { + paste0(type, "/", x) + } + add_parser(mime_type("json"), parser_json()) + add_parser(mime_type("csv"), parser_csv()) + add_parser(mime_type("x-csv"), parser_csv()) + add_parser(mime_type("yaml"), parser_yaml()) + add_parser(mime_type("x-yaml"), parser_yaml()) + + add_parser(mime_type("tab-separated-values"), parser_tsv()) -#' YAML -#' @rdname parsers -#' @export -parser_yaml <- function() { - if (!requireNamespace("yaml", quietly = TRUE)) { - stop("yaml must be installed for the yaml parser to work") - } - function(value, content_type = NULL, ...) { - charset <- getCharacterSet(content_type) - value <- rawToChar(value) - Encoding(value) <- charset - yaml::yaml.load(value) } -} + # only one form of these parsers + add_parser("application/x-www-form-urlencoded", parser_query()) + add_parser("application/rds", parser_rds()) + add_parser("multipart/form-data", parser_multi()) + add_parser("application/octet", parser_octet()) + add_parser("text/plain", parser_text()) -addParsers_onLoad <- function() { - addParser("json", parser_json, "application/json") - addParser("query", parser_query, "application/x-www-form-urlencoded") - addParser("text", parser_text, "text/") - addParser("rds", parser_rds, "application/rds") - addParser("multi", parser_multi, "multipart/form-data") - addParser("octet", parser_octet, "application/octet") - addParser("yaml", parser_yaml, "application/x-yaml") + # shorthand names for parser_picker + add_parser("text", parser_text()) + add_parser("query", parser_query()) + add_parser("octet", parser_octet()) + add_parser("json", parser_json()) } diff --git a/R/zzz.R b/R/zzz.R index c1d80042d..89424545b 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -3,6 +3,6 @@ addApiInfo_onLoad() - addParsers_onLoad() + add_parsers_onLoad() } diff --git a/man/addParser.Rd b/man/addParser.Rd deleted file mode 100644 index ed9bdb5e1..000000000 --- a/man/addParser.Rd +++ /dev/null @@ -1,50 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/parse-body.R -\name{addParser} -\alias{addParser} -\title{Add a Parsers} -\usage{ -addParser(name, parser, pattern = NULL) -} -\arguments{ -\item{name}{The name of the parser (character string)} - -\item{parser}{The parser to be added.} - -\item{pattern}{A pattern to match against the content-type of each part of -the request body.} -} -\description{ -A parser is responsible for decoding the raw body content of a request into -a list of arguments that can be mapped to endpoint function arguments. -For instance, the \code{parser_json} parser content-type \code{application/json}. -The list of available parsers in plumber is global. -} -\details{ -For instance, the \code{parser_json} pattern is \code{application/json}. -If \code{pattern} is not provided, will be set to \code{application/{name}}. -Detection is done assuming content-type starts with pattern and is -case sensitive. - -Parser function structure is something like below. Available parameters -to build parser are \code{value}, \code{content_type} and \code{filename} (only available -in \code{multipart-form} body).\if{html}{\out{
}}\preformatted{parser <- function() \{ - function(value, content_type = "ct", filename, ...) \{ - # do something with raw value - \} -\} -}\if{html}{\out{
}} - -It should return a named list if you want values to map to -plumber endpoint function args. -} -\examples{ -parser_json <- function() { - function(value, content_type = "application/json", ...) { - charset <- getCharacterSet(content_type) - value <- rawToChar(value) - Encoding(value) <- charset - jsonlite::fromJSON(value) - } -} -} diff --git a/man/add_parser.Rd b/man/add_parser.Rd new file mode 100644 index 000000000..53102ceee --- /dev/null +++ b/man/add_parser.Rd @@ -0,0 +1,44 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/parse-body.R +\name{add_parser} +\alias{add_parser} +\title{Add a Parsers} +\usage{ +add_parser(content_type, parser, verbose = TRUE) +} +\arguments{ +\item{content_type}{A string to match against the content-type of each part of +the request body} + +\item{parser}{The parser function to be added. This function should possibly +accept \code{value} and the named parameters \code{content_type} and \code{filename}. +Other parameters may be provided from \code{\link[webutils:parse_multipart]{webutils::parse_multipart()}}. +To be safe, add a \code{...} to your function signature.} + +\item{verbose}{Logical value which determines if a warning should be +displayed when patterns are overwritten.} +} +\description{ +A parser is responsible for decoding the raw body content of a request into +a list of arguments that can be mapped to endpoint function arguments. +For instance, the \code{parser_json} parser content-type \code{application/json}. +} +\details{ +Parser function structure is something like below. Available parameters +to build parser are \code{value}, \code{content_type} and \code{filename} (only available +in \code{multipart-form} body).\if{html}{\out{
}}\preformatted{parser <- function(value, content_type = "ct", filename, ...) \{ + # do something with raw value +\} +}\if{html}{\out{
}} + +It should return a named list if you want values to map to +plumber endpoint function args. +} +\examples{ +parser_dcf <- function(value, content_type = "text/x-dcf", ...) { + charset <- getCharacterSet(content_type) + value <- rawToChar(value) + Encoding(value) <- charset + read.dcf(value) +} +} diff --git a/man/parsers.Rd b/man/parsers.Rd index 69ed120ad..efbf325fa 100644 --- a/man/parsers.Rd +++ b/man/parsers.Rd @@ -1,31 +1,78 @@ % Generated by roxygen2: do not edit by hand % Please edit documentation in R/parse-body.R -\name{parsers} -\alias{parsers} -\alias{parser_json} +\name{parser_query} \alias{parser_query} +\alias{parser_json} \alias{parser_text} +\alias{parser_csv} +\alias{parser_tsv} +\alias{parser_yaml} +\alias{parser_read_file} \alias{parser_rds} -\alias{parser_multi} \alias{parser_octet} -\alias{parser_yaml} +\alias{parser_multi} \title{Plumber Parsers} \usage{ -parser_json() - parser_query() -parser_text() +parser_json(...) -parser_rds() +parser_text(parse_fn = identity) -parser_multi() +parser_csv(...) + +parser_tsv(...) + +parser_yaml(...) + +parser_read_file(read_fn = readLines) + +parser_rds(...) parser_octet() -parser_yaml() +parser_multi() +} +\arguments{ +\item{...}{parameters supplied to the appropriate internal function} + +\item{parse_fn}{function to further decode a text string into an object} + +\item{read_fn}{function used to read a the content of a file. Ex: \code{\link[=readRDS]{readRDS()}}} } \description{ Parsers are used in Plumber to transform the raw body content received -by a request to the API. +by a request to the API. Extra parameters may be provided to parser +functions when adding the parser to plumber. This will allow for +non-default behavior. +} +\section{Functions}{ +\itemize{ +\item \code{parser_query}: Query string parser + +\item \code{parser_json}: JSON parser + +\item \code{parser_text}: Helper parser to parse plain text + +\item \code{parser_csv}: CSV parser + +\item \code{parser_tsv}: TSV parser + +\item \code{parser_yaml}: YAML parser + +\item \code{parser_read_file}: Helper parser that writes the binary post body to a file and reads it back again using \code{read_fn}. +This parser should be used when reading from a file is required. + +\item \code{parser_rds}: RDS parser + +\item \code{parser_octet}: Octet stream parser + +\item \code{parser_multi}: Multi part parser. This parser will then parse each individual body with its respective parser +}} + +\examples{ +\dontrun{ +# Overwrite `text/json` parsing behavior to not allow JSON vectors to be simplified +add_parser("text/json", parser_json(simplifyVector = FALSE)) +} } diff --git a/tests/testthat/test-postbody.R b/tests/testthat/test-postbody.R index 1cce315b9..017584914 100644 --- a/tests/testthat/test-postbody.R +++ b/tests/testthat/test-postbody.R @@ -1,21 +1,21 @@ context("POST body") test_that("JSON is consumed on POST", { - expect_equal(parseBody('{"a":"1"}', content_type = NULL), list(a = "1")) + expect_equal(parse_body('{"a":"1"}', content_type = NULL), list(a = "1")) }) test_that("ending in `==` does not produce a unexpected key", { # See https://github.com/rstudio/plumber/issues/463 - expect_equal(parseBody("randomcharshere==", content_type = NULL), list()) + expect_equal(parse_body("randomcharshere==", content_type = NULL), list()) }) test_that("Query strings on post are handled correctly", { - expect_equivalent(parseBody("a="), list()) # It's technically a named list() - expect_equal(parseBody("a=1&b=&c&d=1", content_type = NULL), list(a="1", d="1")) + expect_equivalent(parse_body("a="), list()) # It's technically a named list() + expect_equal(parse_body("a=1&b=&c&d=1", content_type = NULL), list(a="1", d="1")) }) test_that("Able to handle UTF-8", { - expect_equal(parseBody('{"text":"élise"}', content_type = "application/json; charset=UTF-8")$text, "élise") + expect_equal(parse_body('{"text":"élise"}', content_type = "application/json; charset=UTF-8")$text, "élise") }) #charset moved to part parsing @@ -35,7 +35,7 @@ test_that("filter passes on content-type", { args = c() ) with_mock( - parseBody = function(body, content_type = "unknown") { + parse_body = function(body, content_type = "unknown") { print(content_type) body }, @@ -46,21 +46,21 @@ test_that("filter passes on content-type", { # parsers test_that("Test text parser", { - expect_equal(parseBody("Ceci est un texte.", "text/html"), "Ceci est un texte.") + expect_equal(parse_body("Ceci est un texte.", "text/html"), "Ceci est un texte.") }) test_that("Test yaml parser", { skip_if_not_installed("yaml") r_object <- list(a=1,b=list(c=2,d=list(e=3,f=4:6))) - expect_equal(parseBody(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml"), r_object) + expect_equal(parse_body(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml"), r_object) }) test_that("Test multipart parser", { bin_file <- test_path("files/multipart-form.bin") body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) - parsed_body <- parseBody(body, "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ") + parsed_body <- parse_body(body, "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ") expect_equal(names(parsed_body), c("json", "img1", "img2", "rds")) expect_equal(parsed_body[["rds"]], women) From dee07fab998dd633e4b4cebcc07ad39cb476c0fe Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 13:20:53 -0400 Subject: [PATCH 07/71] use stringi methods --- R/parse-body.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 9cd89b08a..74adb1247 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -45,8 +45,8 @@ parser_picker <- function(content_type, first_byte, filename = NULL) { # "application/json; charset=UTF-8" # to # "application/json" - if (grepl(";", content_type, fixed = TRUE)) { - content_type <- strsplit(content_type, ";")[[1]][1] + if (stri_detect_fixed(content_type, ";")) { + content_type <- stri_split_fixed(content_type, ";")[[1]][1] } parser <- parsers[[content_type]] From 0f27683c599dd11762f9413c01a9439cba64bab8 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 13:26:36 -0400 Subject: [PATCH 08/71] Add comments as to why the content type is being altered --- R/parse-body.R | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index 74adb1247..c8b3ab14d 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -245,7 +245,12 @@ parser_multi <- function() { toparse <- parse_multipart(value, boundary) # content-type detection lapply(toparse, function(x) { - if (is.null(x$content_type) || isTRUE(x$content_type == "application/octet-stream")) { + if ( + is.null(x$content_type) || + # allows for files to be shipped as octect, but parsed using the matching value in `knownContentTypes` + # (Ex: `.rds` files -> `application/rds` which has a proper RDS parser) + isTRUE(stri_detect_fixed(x$content_type, "application/octet-stream")) + ) { if (!is.null(x$filename)) { # Guess content-type from file extension x$content_type <- getContentType(tools::file_ext(x$filename)) From 30ca5ad97adf7f282e19ad0f346faa184498afa5 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 13:26:58 -0400 Subject: [PATCH 09/71] Merged name change --- tests/testthat/test-postbody.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-postbody.R b/tests/testthat/test-postbody.R index a06296808..33fcbdd56 100644 --- a/tests/testthat/test-postbody.R +++ b/tests/testthat/test-postbody.R @@ -71,6 +71,6 @@ test_that("Test multipart parser", { test_that("Test multipart respect content-type", { bin_file <- test_path("files/multipart-ctype.bin") body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) - parsed_body <- parseBody(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272") + parsed_body <- parse_body(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272") expect_equal(class(parsed_body$file), "character") }) From ddaa3c56da78dcbc27491d7870a6ac408b3db7e6 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 13:44:51 -0400 Subject: [PATCH 10/71] Add a default file name in case one isn't provided --- R/parse-body.R | 40 +++++++++++++++++++--------------------- man/parsers.Rd | 20 ++++++++++---------- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index c8b3ab14d..53cfffc9a 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -168,24 +168,6 @@ parser_text <- function(parse_fn = identity) { } -#' @describeIn parsers CSV parser -#' @export -parser_csv <- function(...) { - parser_text(function(val) { - utils::read.csv(val, ...) - }) -} - - -#' @describeIn parsers TSV parser -#' @export -parser_tsv <- function(...) { - parser_text(function(val) { - utils::read.delim(val, ...) - }) -} - - #' @describeIn parsers YAML parser #' @export parser_yaml <- function(...) { @@ -203,7 +185,7 @@ parser_yaml <- function(...) { #' @export parser_read_file <- function(read_fn = readLines) { stopifnot(is.function(read_fn)) - function(value, filename, ...) { + function(value, filename = "", ...) { tmp <- tempfile("plumb", fileext = paste0("_", basename(filename))) on.exit({ file.remove(tmp) @@ -213,6 +195,24 @@ parser_read_file <- function(read_fn = readLines) { } } +#' @describeIn parsers CSV parser +#' @export +parser_csv <- function(...) { + parser_read_file(function(val) { + utils::read.csv(val, ...) + }) +} + + +#' @describeIn parsers TSV parser +#' @export +parser_tsv <- function(...) { + parser_read_file(function(val) { + utils::read.delim(val, ...) + }) +} + + #' @describeIn parsers RDS parser #' @export parser_rds <- function(...) { @@ -222,8 +222,6 @@ parser_rds <- function(...) { } - - #' @describeIn parsers Octet stream parser #' @export parser_octet <- function() { diff --git a/man/parsers.Rd b/man/parsers.Rd index efbf325fa..15258cdac 100644 --- a/man/parsers.Rd +++ b/man/parsers.Rd @@ -4,10 +4,10 @@ \alias{parser_query} \alias{parser_json} \alias{parser_text} -\alias{parser_csv} -\alias{parser_tsv} \alias{parser_yaml} \alias{parser_read_file} +\alias{parser_csv} +\alias{parser_tsv} \alias{parser_rds} \alias{parser_octet} \alias{parser_multi} @@ -19,14 +19,14 @@ parser_json(...) parser_text(parse_fn = identity) -parser_csv(...) - -parser_tsv(...) - parser_yaml(...) parser_read_file(read_fn = readLines) +parser_csv(...) + +parser_tsv(...) + parser_rds(...) parser_octet() @@ -54,15 +54,15 @@ non-default behavior. \item \code{parser_text}: Helper parser to parse plain text -\item \code{parser_csv}: CSV parser - -\item \code{parser_tsv}: TSV parser - \item \code{parser_yaml}: YAML parser \item \code{parser_read_file}: Helper parser that writes the binary post body to a file and reads it back again using \code{read_fn}. This parser should be used when reading from a file is required. +\item \code{parser_csv}: CSV parser + +\item \code{parser_tsv}: TSV parser + \item \code{parser_rds}: RDS parser \item \code{parser_octet}: Octet stream parser From 11d459c5bedbaab06db409c616486cf6d0f08ad4 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 13:45:15 -0400 Subject: [PATCH 11/71] Add tsv and csv tests. Rename test file. --- .../{test-postbody.R => test-parse-body.R} | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) rename tests/testthat/{test-postbody.R => test-parse-body.R} (79%) diff --git a/tests/testthat/test-postbody.R b/tests/testthat/test-parse-body.R similarity index 79% rename from tests/testthat/test-postbody.R rename to tests/testthat/test-parse-body.R index 33fcbdd56..e10c24b19 100644 --- a/tests/testthat/test-postbody.R +++ b/tests/testthat/test-parse-body.R @@ -56,7 +56,32 @@ test_that("Test yaml parser", { expect_equal(parse_body(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml"), r_object) }) +test_that("Test csv parser", { + tmp <- tempfile() + on.exit({ + file.remove(tmp) + }, add = TRUE) + + r_object <- cars + write.csv(r_object, tmp, row.names = FALSE) + val <- readBin(tmp, "raw", 1000) + expect_equal(parse_body(val, "application/csv"), r_object) +}) + +test_that("Test tsv parser", { + tmp <- tempfile() + on.exit({ + file.remove(tmp) + }, add = TRUE) + + r_object <- cars + write.table(r_object, tmp, sep = "\t", row.names = FALSE) + val <- readBin(tmp, "raw", 1000) + expect_equal(parse_body(val, "application/tab-separated-values"), r_object) +}) + test_that("Test multipart parser", { + # also tests rds and the octet -> content type conversion bin_file <- test_path("files/multipart-form.bin") body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) @@ -68,9 +93,10 @@ test_that("Test multipart parser", { expect_equal(parsed_body[["json"]], list(a=2,b=4,c=list(w=3,t=5))) }) + test_that("Test multipart respect content-type", { bin_file <- test_path("files/multipart-ctype.bin") body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) parsed_body <- parse_body(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272") - expect_equal(class(parsed_body$file), "character") + expect_s3_class(parsed_body$file, "data.frame") }) From 48e78097a1152d0273394c86a366ff0dd048c822 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 14:04:49 -0400 Subject: [PATCH 12/71] add news item --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 02c7843b7..bf2b50846 100644 --- a/NEWS.md +++ b/NEWS.md @@ -52,6 +52,8 @@ plumber 1.0.0 * Added yaml support, serializer and parser. (@meztez, #556) +* Added csv and tsv parsers (#584) + * Added csv serializer (@pachamaltese, #520) * Added svg serializer (@pachamaltese, #398) From 79c2972a1b74b36c65bdd30c64f92973f2181257 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 14:07:24 -0400 Subject: [PATCH 13/71] fix topic names for pkgdown --- pkgdown/_pkgdown.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index d58bc188f..f811d6c9a 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -46,7 +46,7 @@ reference: - 'PlumberEndpoint' - 'PlumberStatic' - 'PlumberStep' - - 'addParser' + - 'add_parser' - 'addSerializer' - 'do_configure_https' - 'do_deploy_api' @@ -59,7 +59,7 @@ reference: - 'hookable' - 'include_file' - 'options_plumber' - - 'parsers' + - 'parser_query' - 'plumb' - 'randomCookieKey' - 'serializer_json' From 10199b5ce025a869978b2f85aaa32549bcba25ec Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 15:05:16 -0400 Subject: [PATCH 14/71] Add octet-stream --- R/parse-body.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/parse-body.R b/R/parse-body.R index 53cfffc9a..eca534486 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -286,6 +286,7 @@ add_parsers_onLoad <- function() { add_parser("application/rds", parser_rds()) add_parser("multipart/form-data", parser_multi()) add_parser("application/octet", parser_octet()) + add_parser("application/octet-stream", parser_octet()) add_parser("text/plain", parser_text()) From 61fec39615838e7d7dfee5d2d8f8b271614fb635 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 3 Jul 2020 15:14:17 -0400 Subject: [PATCH 15/71] Remove invalid content type --- R/parse-body.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index eca534486..2b296e2c9 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -285,7 +285,6 @@ add_parsers_onLoad <- function() { add_parser("application/x-www-form-urlencoded", parser_query()) add_parser("application/rds", parser_rds()) add_parser("multipart/form-data", parser_multi()) - add_parser("application/octet", parser_octet()) add_parser("application/octet-stream", parser_octet()) add_parser("text/plain", parser_text()) From 0d3b795949b892b2f5efadc6abf5d8f59a96f105 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 7 Jul 2020 16:33:17 -0400 Subject: [PATCH 16/71] Do not eval any yaml --- R/parse-body.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index 2b296e2c9..599826163 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -175,7 +175,7 @@ parser_yaml <- function(...) { if (!requireNamespace("yaml", quietly = TRUE)) { stop("yaml must be installed for the yaml parser to work") } - yaml::yaml.load(val, ...) + yaml::yaml.load(val, ..., eval.expr = FALSE) }) } From 8110b910425277e6c6791be0d4b93a9f62c7da5f Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Wed, 8 Jul 2020 22:24:01 -0400 Subject: [PATCH 17/71] parser plumber tag, parser builder functions return a named list of mime-type = parser --- R/parse-body.R | 143 +++++++++++++++++++++++++++-------------------- R/plumb-block.R | 35 +++++++++++- R/plumber-step.R | 8 ++- R/plumber.R | 27 +++++++-- 4 files changed, 144 insertions(+), 69 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 2b296e2c9..7a229c9da 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -2,11 +2,7 @@ postBodyFilter <- function(req){ handled <- req$.internal$postBodyHandled if (is.null(handled) || handled != TRUE) { # This will return raw bytes - body <- req$rook.input$read() - type <- req$HTTP_CONTENT_TYPE - args <- parse_body(body, type) - req$args <- c(req$args, args) - req$postBodyRaw <- body + req$postBodyRaw <- req$rook.input$read() if (isTRUE(getOption("plumber.postBody", TRUE))) { req$rook.input$rewind() req$postBody <- paste0(req$rook.input$read_lines(), collapse = "\n") @@ -16,20 +12,29 @@ postBodyFilter <- function(req){ forward() } -parse_body <- function(body, content_type = NULL) { +postbody_parser <- function(req, parsers = NULL) { + type <- req$HTTP_CONTENT_TYPE + parse_body(req$postBodyRaw, type, parsers) +} + +parse_body <- function(body, content_type = NULL, parsers = NULL) { if (!is.raw(body)) {body <- charToRaw(body)} toparse <- list(value = body, content_type = content_type) parse_raw(toparse) } -parse_raw <- function(toparse) { +parse_raw <- function(toparse, parsers = NULL) { if (length(toparse$value) == 0L) return(list()) - parser <- parser_picker(toparse$content_type, toparse$value[1], toparse$filename) - do.call(parser, toparse) + parser <- parser_picker(toparse$content_type, toparse$value[1], toparse$filename, parsers) + if (!is.null(parser)) { + return(do.call(parser, toparse)) + } else { + warning("No suitable parser found to handle request body.") + return(list()) + } } -parser_picker <- function(content_type, first_byte, filename = NULL) { - parsers <- .globals$parsers +parser_picker <- function(content_type, first_byte, filename = NULL, parsers = NULL) { # parse as a query string if (is.null(content_type)) { @@ -67,7 +72,7 @@ parser_picker <- function(content_type, first_byte, filename = NULL) { return(parsers$query) } - # octect + # octet parsers$octet } @@ -78,8 +83,7 @@ parser_picker <- function(content_type, first_byte, filename = NULL) { #' a list of arguments that can be mapped to endpoint function arguments. #' For instance, the \code{parser_json} parser content-type `application/json`. #' -#' @param content_type A string to match against the content-type of each part of -#' the request body +#' @param alias Short name to map parser #' @param parser The parser function to be added. This function should possibly #' accept `value` and the named parameters `content_type` and `filename`. #' Other parameters may be provided from [webutils::parse_multipart()]. @@ -109,17 +113,17 @@ parser_picker <- function(content_type, first_byte, filename = NULL) { #' read.dcf(value) #' } #' @export -add_parser <- function(content_type, parser, verbose = TRUE) { +add_parser <- function(alias, parser, verbose = TRUE) { - if (!is.null(.globals$parsers[[content_type]])) { + if (!is.null(.globals$parsers[[alias]])) { if (isTRUE(verbose)) { - warning("Overwriting parser: ", content_type) + warning("Overwriting parser: ", alias) } } stopifnot(is.function(parser)) - .globals$parsers[[content_type]] <- parser + .globals$parsers[[alias]] <- parser invisible(.globals$parsers) } @@ -141,16 +145,25 @@ add_parser <- function(content_type, parser, verbose = TRUE) { #' } #' @export parser_query <- function() { - parser_text(parseQS) + parse_func <- parser_text(parseQS) + return(invisible( + list("application/x-www-form-urlencoded" = parse_func, + "query" = parse_func) + )) } #' @describeIn parsers JSON parser #' @export parser_json <- function(...) { - parser_text(function(value) { + parse_func <- parser_text(function(value) { safeFromJSON(value, ...) - }) + })[[1]] + return(invisible( + list("application/json" = parse_func, + "text/json" = parse_func, + "json" = parse_func) + )) } @@ -159,24 +172,34 @@ parser_json <- function(...) { #' @export parser_text <- function(parse_fn = identity) { stopifnot(is.function(parse_fn)) - function(value, content_type = NULL, ...) { + parse_func <- function(value, content_type = NULL, ...) { charset <- getCharacterSet(content_type) value <- rawToChar(value) Encoding(value) <- charset parse_fn(value) } + return(invisible( + list("text/plain" = parse_func, + "text" = parse_func) + )) } #' @describeIn parsers YAML parser #' @export parser_yaml <- function(...) { - parser_text(function(val) { + parse_func <- parser_text(function(val) { if (!requireNamespace("yaml", quietly = TRUE)) { stop("yaml must be installed for the yaml parser to work") } yaml::yaml.load(val, ...) - }) + })[[1]] + return(invisible( + list("application/yaml" = parse_func, + "application/x-yaml" = parse_func, + "text/yaml" = parse_func, + "text/x-yaml" = parse_func) + )) } #' @describeIn parsers Helper parser that writes the binary post body to a file and reads it back again using `read_fn`. @@ -198,37 +221,54 @@ parser_read_file <- function(read_fn = readLines) { #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { - parser_read_file(function(val) { + parse_func <- parser_read_file(function(val) { utils::read.csv(val, ...) }) + return(invisible( + list("application/csv" = parse_func, + "application/x-csv" = parse_func, + "text/csv" = parse_func, + "text/x-csv" = parse_func) + )) } #' @describeIn parsers TSV parser #' @export parser_tsv <- function(...) { - parser_read_file(function(val) { + parse_func <- parser_read_file(function(val) { utils::read.delim(val, ...) }) + return(invisible( + list("application/tab-separated-values" = parse_func, + "text/tab-separated-values" = parse_func) + )) } #' @describeIn parsers RDS parser #' @export parser_rds <- function(...) { - parser_read_file(function(value) { + parse_func <- parser_read_file(function(value) { readRDS(value, ...) }) + return(invisible( + list("application/rds" = parse_func) + )) } #' @describeIn parsers Octet stream parser #' @export parser_octet <- function() { - function(value, filename = NULL, ...) { + parse_func <- function(value, filename = NULL, ...) { attr(value, "filename") <- filename value } + return(invisible( + list("application/octet-stream" = parse_func, + "octet" = parse_func) + )) } @@ -236,7 +276,7 @@ parser_octet <- function() { #' @export #' @importFrom webutils parse_multipart parser_multi <- function() { - function(value, content_type, ...) { + parse_func <- function(value, content_type, ...) { if (!stri_detect_fixed(content_type, "boundary=", case_insensitive = TRUE)) stop("No boundary found in multipart content-type header: ", content_type) boundary <- stri_match_first_regex(content_type, "boundary=([^; ]{2,})", case_insensitive = TRUE)[,2] @@ -257,42 +297,23 @@ parser_multi <- function() { parse_raw(x) }) } + return(invisible( + list("multipart/form-data" = parse_func) + )) } add_parsers_onLoad <- function() { - # add both `application/XYZ` and `text/XYZ` parsers - for (type in c("application", "text")) { - mime_type <- function(x) { - paste0(type, "/", x) - } - - add_parser(mime_type("json"), parser_json()) - - add_parser(mime_type("csv"), parser_csv()) - add_parser(mime_type("x-csv"), parser_csv()) - - add_parser(mime_type("yaml"), parser_yaml()) - add_parser(mime_type("x-yaml"), parser_yaml()) - - add_parser(mime_type("tab-separated-values"), parser_tsv()) - - } - - # only one form of these parsers - add_parser("application/x-www-form-urlencoded", parser_query()) - add_parser("application/rds", parser_rds()) - add_parser("multipart/form-data", parser_multi()) - add_parser("application/octet-stream", parser_octet()) - - add_parser("text/plain", parser_text()) - - - # shorthand names for parser_picker - add_parser("text", parser_text()) - add_parser("query", parser_query()) - add_parser("octet", parser_octet()) - add_parser("json", parser_json()) + # shorthand names for parser plumbing + add_parser("csv", parser_csv) + add_parser("json", parser_json) + add_parser("multi", parser_multi) + add_parser("octet", parser_octet) + add_parser("query", parser_query) + add_parser("rds", parser_rds) + add_parser("text", parser_text) + add_parser("tsv", parser_tsv) + add_parser("yaml", parser_yaml) } diff --git a/R/plumb-block.R b/R/plumb-block.R index 1f5102d18..5e122b7ab 100644 --- a/R/plumb-block.R +++ b/R/plumb-block.R @@ -19,6 +19,7 @@ plumbBlock <- function(lineNum, file){ image <- NULL imageAttr <- NULL serializer <- NULL + parsers <- NULL assets <- NULL params <- NULL comments <- "" @@ -102,7 +103,7 @@ plumbBlock <- function(lineNum, file){ } if (!s %in% names(.globals$serializers)){ - stop("No such @serializer registered: ", s) + stopOnLine(lineNum, line, paste0("No such @serializer registered: ", s)) } ser <- .globals$serializers[[s]] @@ -130,7 +131,7 @@ plumbBlock <- function(lineNum, file){ } if (!is.na(s) && !s %in% names(.globals$serializers)){ - stop("No such @serializer registered: ", s) + stopOnLine(lineNum, line, paste0("No such @serializer registered: ", s)) } shortSerAttr <- trimws(shortSerMat[1,3]) if(!identical(shortSerAttr, "") && !grepl("^\\(.*\\)$", shortSerAttr)){ @@ -151,6 +152,33 @@ plumbBlock <- function(lineNum, file){ } + parsersMat <- stri_match(line, regex="^#['\\*]\\s*@parser(\\s+([^\\s]+)\\s*(.*)\\s*$)?") + if (!is.na(parsersMat[1,1])){ + parser_alias <- stri_trim_both(parsersMat[1,3]) + if (is.na(parser_alias) || parser_alias == ""){ + stopOnLine(lineNum, line, "No @parser specified") + } + + if (!parser_alias %in% names(.globals$parsers)){ + stopOnLine(lineNum, line, paste0("No such @parser registered: ", parser_alias)) + } + + parser_builder <- .globals$parsers[[parser_alias]] + + if (!is.na(parsersMat[1, 4]) && parsersMat[1,4] != ""){ + # We have an arg to pass in to the parser + argList <- eval(parse(text=parsersMat[1,4])) + } else { + argList <- list() + } + tryCatch({ + parsers <- c(parsers, do.call(parser_builder, argList)) + }, error = function(e) { + stopOnLine(lineNum, line, paste0("Error creating parser: ", parser_alias, "\n", e)) + }) + + } + imageMat <- stri_match(line, regex="^#['\\*]\\s*@(jpeg|png|svg)([\\s\\(].*)?\\s*$") if (!is.na(imageMat[1,1])){ if (!is.null(image)){ @@ -223,6 +251,7 @@ plumbBlock <- function(lineNum, file){ image = image, imageAttr = imageAttr, serializer = serializer, + parsers = parsers, assets = assets, params = rev(params), comments = comments, @@ -247,7 +276,7 @@ evaluateBlock <- function(srcref, file, expr, envir, addEndpoint, addFilter, pr) # ALL if statements possibilities must eventually call eval(expr, envir) if (!is.null(block$paths)){ lapply(block$paths, function(p){ - ep <- PlumberEndpoint$new(p$verb, p$path, expr, envir, block$serializer, srcref, block$params, block$comments, block$responses, block$tags) + ep <- PlumberEndpoint$new(p$verb, p$path, expr, envir, block$serializer, block$parsers, srcref, block$params, block$comments, block$responses, block$tags) if (!is.null(block$image)){ # Arguments to pass in to the image serializer diff --git a/R/plumber-step.R b/R/plumber-step.R index ff88ab60e..9fa38c551 100644 --- a/R/plumber-step.R +++ b/R/plumber-step.R @@ -158,6 +158,8 @@ PlumberEndpoint <- R6Class( params = NA, #' @field tags endpoint tags tags = NA, + #' @field parsers step allowed parsers + parsers = NULL, #' @description ability to serve request #' @param req a request object #' @return a logical. `TRUE` when endpoint can serve request. @@ -170,6 +172,7 @@ PlumberEndpoint <- R6Class( #' @param expr endpoint expr #' @param envir endpoint environment #' @param serializer endpoint serializer + #' @param parsers endpoint parsers #' @param lines endpoint block #' @param params endpoint params #' @param comments endpoint comments @@ -178,7 +181,7 @@ PlumberEndpoint <- R6Class( #' @details Parameters values are obtained from parsing blocks of lines in a plumber file. #' They can also be provided manually for historical reasons. #' @return A new `PlumberEndpoint` object - initialize = function(verbs, path, expr, envir, serializer, lines, params, comments, responses, tags){ + initialize = function(verbs, path, expr, envir, serializer, parsers, lines, params, comments, responses, tags){ self$verbs <- verbs self$path <- path @@ -195,6 +198,9 @@ PlumberEndpoint <- R6Class( if (!missing(serializer) && !is.null(serializer)){ self$serializer <- serializer } + if (!missing(parsers) && !is.null(parsers)){ + self$parsers <- parsers + } if (!missing(lines)){ self$lines <- lines } diff --git a/R/plumber.R b/R/plumber.R index 16798a85c..d1f3cfc3d 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -220,6 +220,7 @@ plumber <- R6Class( # Initialize private$serializer <- serializer_json() + private$parsers <- c(parser_json(), parser_query(), parser_text(), parser_octet(), parser_multi()) private$errorHandler <- defaultErrorHandler() private$notFoundHandler <- default404Handler private$maxSize <- getOption('plumber.maxRequestSize', 0) #0 Unlimited @@ -480,8 +481,8 @@ plumber <- R6Class( #' "

Programmatic Plumber!

" #' }, serializer=plumber::serializer_html()) #' } - handle = function(methods, path, handler, preempt, serializer, endpoint, ...){ - epdef <- !missing(methods) || !missing(path) || !missing(handler) || !missing(serializer) + handle = function(methods, path, handler, preempt, serializer, parsers, endpoint, ...){ + epdef <- !missing(methods) || !missing(path) || !missing(handler) || !missing(serializer) || !missing(parsers) if (!missing(endpoint) && epdef){ stop("You must provide either the components for an endpoint (handler and serializer) OR provide the endpoint yourself. You cannot do both.") } @@ -490,8 +491,11 @@ plumber <- R6Class( if (missing(serializer)){ serializer <- private$serializer } + if (missing(parsers)){ + parsers <- private$parsers + } - endpoint <- PlumberEndpoint$new(methods, path, handler, private$envir, serializer, ...) + endpoint <- PlumberEndpoint$new(methods, path, handler, private$envir, serializer, parsers, ...) } private$addEndpointInternal(endpoint, preempt) }, @@ -729,7 +733,16 @@ plumber <- R6Class( if (!is.null(h$serializer)) { res$serializer <- h$serializer } - req$args <- c(h$getPathParams(path), req$args) + if (!is.null(h$parsers)) { + parsers <- h$parsers + } else { + parsers <- private$parsers + } + req$args <- c( + h$getPathParams(path), + req$args, + postbody_parser(req, parsers) + ) return(do.call(h$exec, req$args)) } } @@ -871,6 +884,11 @@ plumber <- R6Class( setSerializer = function(serializer){ private$serializer <- serializer }, + #' @details Sets the default parsers of the router. + #' @param parsers a named list of parsers + setParsers = function(parsers){ + private$parsers <- parsers + }, #' @details Sets the handler that gets called if an #' incoming request can’t be served by any filter, endpoint, or sub-router. #' @param fun a handler function. @@ -1047,6 +1065,7 @@ plumber <- R6Class( } ), private = list( serializer = NULL, # The default serializer for the router + parsers = NULL, # The default parsers for the router ends = list(), # List of endpoints indexed by their pre-empted filter. filts = NULL, # Array of filters From b2c058294911402814a3fbeaae35c7a817cf450a Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Wed, 8 Jul 2020 23:25:40 -0400 Subject: [PATCH 18/71] passing current tests --- R/parse-body.R | 20 +++++++++++------- tests/testthat/test-parse-body.R | 36 +++++++++++++------------------- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 7a229c9da..bfa9419b0 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -14,22 +14,27 @@ postBodyFilter <- function(req){ postbody_parser <- function(req, parsers = NULL) { type <- req$HTTP_CONTENT_TYPE - parse_body(req$postBodyRaw, type, parsers) + body <- req$postBodyRaw + if (length(body)>1) { + parse_body(body, type, parsers) + } else { + list() + } } parse_body <- function(body, content_type = NULL, parsers = NULL) { if (!is.raw(body)) {body <- charToRaw(body)} - toparse <- list(value = body, content_type = content_type) + toparse <- list(value = body, content_type = content_type, parsers = parsers) parse_raw(toparse) } -parse_raw <- function(toparse, parsers = NULL) { +parse_raw <- function(toparse) { if (length(toparse$value) == 0L) return(list()) - parser <- parser_picker(toparse$content_type, toparse$value[1], toparse$filename, parsers) + parser <- parser_picker(toparse$content_type, toparse$value[1], toparse$filename, toparse$parsers) if (!is.null(parser)) { return(do.call(parser, toparse)) } else { - warning("No suitable parser found to handle request body.") + warning("No suitable parser found to handle request body type ", toparse$content_type, ".") return(list()) } } @@ -145,7 +150,7 @@ add_parser <- function(alias, parser, verbose = TRUE) { #' } #' @export parser_query <- function() { - parse_func <- parser_text(parseQS) + parse_func <- parser_text(parseQS)[[1]] return(invisible( list("application/x-www-form-urlencoded" = parse_func, "query" = parse_func) @@ -276,7 +281,7 @@ parser_octet <- function() { #' @export #' @importFrom webutils parse_multipart parser_multi <- function() { - parse_func <- function(value, content_type, ...) { + parse_func <- function(value, content_type, parsers, ...) { if (!stri_detect_fixed(content_type, "boundary=", case_insensitive = TRUE)) stop("No boundary found in multipart content-type header: ", content_type) boundary <- stri_match_first_regex(content_type, "boundary=([^; ]{2,})", case_insensitive = TRUE)[,2] @@ -294,6 +299,7 @@ parser_multi <- function() { x$content_type <- getContentType(tools::file_ext(x$filename)) } } + x$parsers <- parsers parse_raw(x) }) } diff --git a/tests/testthat/test-parse-body.R b/tests/testthat/test-parse-body.R index e10c24b19..d1beb2b04 100644 --- a/tests/testthat/test-parse-body.R +++ b/tests/testthat/test-parse-body.R @@ -1,59 +1,51 @@ context("POST body") test_that("JSON is consumed on POST", { - expect_equal(parse_body('{"a":"1"}', content_type = NULL), list(a = "1")) + expect_equal(parse_body('{"a":"1"}', content_type = NULL, parsers = parser_json()), list(a = "1")) }) test_that("ending in `==` does not produce a unexpected key", { # See https://github.com/rstudio/plumber/issues/463 - expect_equal(parse_body("randomcharshere==", content_type = NULL), list()) + expect_equal(parse_body("randomcharshere==", content_type = NULL, parsers = parser_query()), list()) }) test_that("Query strings on post are handled correctly", { - expect_equivalent(parse_body("a="), list()) # It's technically a named list() - expect_equal(parse_body("a=1&b=&c&d=1", content_type = NULL), list(a="1", d="1")) + expect_equivalent(parse_body("a=", parsers = parser_query()), list()) # It's technically a named list() + expect_equal(parse_body("a=1&b=&c&d=1", content_type = NULL, parser_query()), list(a="1", d="1")) }) test_that("Able to handle UTF-8", { - expect_equal(parse_body('{"text":"élise"}', content_type = "application/json; charset=UTF-8")$text, "élise") + expect_equal(parse_body('{"text":"élise"}', content_type = "application/json; charset=UTF-8", parsers = parser_json())$text, "élise") }) #charset moved to part parsing test_that("filter passes on content-type", { content_type_passed <- "" req <- list( - .internal = list(postBodyHandled = FALSE), - rook.input = list( - read = function() { - called <- TRUE - return(charToRaw("this is a body")) - }, - rewind = function() {}, - read_lines = function() {return("this is a body")} - ), + postBodyRaw = charToRaw("this is a body"), HTTP_CONTENT_TYPE = "text/html; charset=testset", args = c() ) with_mock( - parse_body = function(body, content_type = "unknown") { + parse_body = function(body, content_type = "unknown", parsers = NULL) { print(content_type) body }, - expect_output(postBodyFilter(req), "text/html; charset=testset"), + expect_output(postbody_parser(req, ), "text/html; charset=testset"), .env = "plumber" ) }) # parsers test_that("Test text parser", { - expect_equal(parse_body("Ceci est un texte.", "text/html"), "Ceci est un texte.") + expect_equal(parse_body("Ceci est un texte.", "text/html", parser_text()), "Ceci est un texte.") }) test_that("Test yaml parser", { skip_if_not_installed("yaml") r_object <- list(a=1,b=list(c=2,d=list(e=3,f=4:6))) - expect_equal(parse_body(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml"), r_object) + expect_equal(parse_body(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml", parser_yaml()), r_object) }) test_that("Test csv parser", { @@ -65,7 +57,7 @@ test_that("Test csv parser", { r_object <- cars write.csv(r_object, tmp, row.names = FALSE) val <- readBin(tmp, "raw", 1000) - expect_equal(parse_body(val, "application/csv"), r_object) + expect_equal(parse_body(val, "application/csv", parser_csv()), r_object) }) test_that("Test tsv parser", { @@ -77,7 +69,7 @@ test_that("Test tsv parser", { r_object <- cars write.table(r_object, tmp, sep = "\t", row.names = FALSE) val <- readBin(tmp, "raw", 1000) - expect_equal(parse_body(val, "application/tab-separated-values"), r_object) + expect_equal(parse_body(val, "application/tab-separated-values", parser_tsv()), r_object) }) test_that("Test multipart parser", { @@ -85,7 +77,7 @@ test_that("Test multipart parser", { bin_file <- test_path("files/multipart-form.bin") body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) - parsed_body <- parse_body(body, "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ") + parsed_body <- parse_body(body, "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ", c(parser_multi(), parser_json(), parser_rds(), parser_octet())) expect_equal(names(parsed_body), c("json", "img1", "img2", "rds")) expect_equal(parsed_body[["rds"]], women) @@ -97,6 +89,6 @@ test_that("Test multipart parser", { test_that("Test multipart respect content-type", { bin_file <- test_path("files/multipart-ctype.bin") body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) - parsed_body <- parse_body(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272") + parsed_body <- parse_body(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272", c(parser_multi(), parser_tsv())) expect_s3_class(parsed_body$file, "data.frame") }) From 70d3691213e4e93634fbcb551db912738c338a6c Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Thu, 9 Jul 2020 00:11:30 -0400 Subject: [PATCH 19/71] change default parsers on router --- R/parse-body.R | 5 ++++- R/plumb-block.R | 3 ++- R/plumber.R | 3 ++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index bfa9419b0..9b53fc202 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -34,7 +34,7 @@ parse_raw <- function(toparse) { if (!is.null(parser)) { return(do.call(parser, toparse)) } else { - warning("No suitable parser found to handle request body type ", toparse$content_type, ".") + message("No suitable parser found to handle request body type ", toparse$content_type, ".") return(list()) } } @@ -141,6 +141,9 @@ add_parser <- function(alias, parser, verbose = TRUE) { #' functions when adding the parser to plumber. This will allow for #' non-default behavior. #' +#' User should be aware that `rds` parsing should only be done from a +#' trusted source. Do not accept `rds` files blindly. +#' #' @param ... parameters supplied to the appropriate internal function #' @describeIn parsers Query string parser #' @examples diff --git a/R/plumb-block.R b/R/plumb-block.R index 5e122b7ab..0bc6a619a 100644 --- a/R/plumb-block.R +++ b/R/plumb-block.R @@ -172,7 +172,8 @@ plumbBlock <- function(lineNum, file){ argList <- list() } tryCatch({ - parsers <- c(parsers, do.call(parser_builder, argList)) + # Use modifyList instead of c to avoid duplicated parsers name + parsers <- utils::modifyList(parsers, do.call(parser_builder, argList)) }, error = function(e) { stopOnLine(lineNum, line, paste0("Error creating parser: ", parser_alias, "\n", e)) }) diff --git a/R/plumber.R b/R/plumber.R index d1f3cfc3d..d6187afb4 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -220,7 +220,8 @@ plumber <- R6Class( # Initialize private$serializer <- serializer_json() - private$parsers <- c(parser_json(), parser_query(), parser_text(), parser_octet(), parser_multi()) + # Default parsers to maintain legacy features + private$parsers <- c(parser_json(), parser_query()) private$errorHandler <- defaultErrorHandler() private$notFoundHandler <- default404Handler private$maxSize <- getOption('plumber.maxRequestSize', 0) #0 Unlimited From bd0a46f98f6bbae1155f6c75041808201d3e8d3b Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Thu, 9 Jul 2020 10:42:55 -0400 Subject: [PATCH 20/71] adding @parser none and @parser all, update parsers doc. --- R/parse-body.R | 90 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 27 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 9b53fc202..48d63f110 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -13,13 +13,11 @@ postBodyFilter <- function(req){ } postbody_parser <- function(req, parsers = NULL) { + if (length(parsers) == 0) {return(list())} type <- req$HTTP_CONTENT_TYPE body <- req$postBodyRaw - if (length(body)>1) { - parse_body(body, type, parsers) - } else { - list() - } + if (is.null(body)) {return(list())} + parse_body(body, type, parsers) } parse_body <- function(body, content_type = NULL, parsers = NULL) { @@ -86,37 +84,42 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' #' A parser is responsible for decoding the raw body content of a request into #' a list of arguments that can be mapped to endpoint function arguments. -#' For instance, the \code{parser_json} parser content-type `application/json`. +#' For instance, \code{parser_json} parse content-type `application/json`. #' -#' @param alias Short name to map parser -#' @param parser The parser function to be added. This function should possibly -#' accept `value` and the named parameters `content_type` and `filename`. -#' Other parameters may be provided from [webutils::parse_multipart()]. -#' To be safe, add a `...` to your function signature. +#' @param alias Short name to map parser from the `@parser` plumber tag. +#' @param parser The parser function to be added. This build the parser function. #' @param verbose Logical value which determines if a warning should be -#' displayed when patterns are overwritten. +#' displayed when alias in map are overwritten. #' #' @details -#' -#' Parser function structure is something like below. Available parameters -#' to build parser are `value`, `content_type` and `filename` (only available -#' in `multipart-form` body). +#' When `parser` is evaluated, it should return a named list of functions. +#' Content-types/Mime-types are used as the list names and will be matched to +#' corresponding parsing function. +#' Functions signature in the list should include `value`, `...` and +#' possibly `content_type`, `filename`. Other parameters may be provided +#' if you want to use the headers from [webutils::parse_multipart()]. +#' Parser function structure is something like below. #' ```r -#' parser <- function(value, content_type = "ct", filename, ...) { +#' parser <- () { +#' f <- function(value, ...) { #' # do something with raw value +#' } +#' list("ct" = f) #' } #' ``` #' -#' It should return a named list if you want values to map to -#' plumber endpoint function args. -#' #' @examples -#' parser_dcf <- function(value, content_type = "text/x-dcf", ...) { -#' charset <- getCharacterSet(content_type) -#' value <- rawToChar(value) -#' Encoding(value) <- charset -#' read.dcf(value) +#' # Content-type header is mostly used to look up charset and adjust encoding +#' parser_dcf <- function() { +#' parse_func <- function(value, content_type = "text/x-dcf", ...) { +#' charset <- getCharacterSet(content_type) +#' value <- rawToChar(value) +#' Encoding(value) <- charset +#' read.dcf(value) +#' } +#' return(invisible(list("text/x-dcf" = parse_func))) #' } +#' add_parser("dcf", parser_dcf) #' @export add_parser <- function(alias, parser, verbose = TRUE) { @@ -130,9 +133,14 @@ add_parser <- function(alias, parser, verbose = TRUE) { .globals$parsers[[alias]] <- parser - invisible(.globals$parsers) + invisible(list_parsers()) } +#' @export +#' @describeIn add_parser List currently registered parsers +list_parsers <- function() { + .globals$parsers +} #' Plumber Parsers #' @@ -141,15 +149,27 @@ add_parser <- function(alias, parser, verbose = TRUE) { #' functions when adding the parser to plumber. This will allow for #' non-default behavior. #' +#' Parsers are optional. When unspecified, only the [parser_json()] and +#' [parser_query()] are available. You can use `@parser parser` tag to +#' activate parsers per endpoint. Multiple parsers can be activated for +#' the same endpoint using multiple `@parser parser` tags. +#' #' User should be aware that `rds` parsing should only be done from a #' trusted source. Do not accept `rds` files blindly. #' +#' See [list_parsers()] for a list of registered parsers. +#' #' @param ... parameters supplied to the appropriate internal function #' @describeIn parsers Query string parser #' @examples #' \dontrun{ #' # Overwrite `text/json` parsing behavior to not allow JSON vectors to be simplified -#' add_parser("text/json", parser_json(simplifyVector = FALSE)) +#' #* @parser json simplifyVector = FALSE +#' # Activate `rds` parser in a multipart request +#' #* @parser multi +#' #* @parser rds +#' pr <- plumber$new() +#' pr$handle("GET", "/upload", function(rds) {rds}, parsers = c(parser_multi(), parser_rds())) #' } #' @export parser_query <- function() { @@ -311,7 +331,21 @@ parser_multi <- function() { )) } +#' @describeIn parsers All parsers +#' @export +parser_all <- function() { + return(invisible( + Reduce(function(a, b) {c(a, b())}, .globals$parsers, init = list()) + )) +} +#' @describeIn parsers No parser +#' @export +parser_none <- function() { + return(invisible( + list() + )) +} add_parsers_onLoad <- function() { @@ -325,4 +359,6 @@ add_parsers_onLoad <- function() { add_parser("text", parser_text) add_parser("tsv", parser_tsv) add_parser("yaml", parser_yaml) + add_parser("all", parser_all) + add_parser("none", parser_none) } From 7aa66f6f693ab17ee70f3f6f0732ca0ed670e8e7 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Thu, 9 Jul 2020 11:00:54 -0400 Subject: [PATCH 21/71] Apply suggestions from code review Add more default parsers Co-authored-by: Barret Schloerke --- R/plumber.R | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/R/plumber.R b/R/plumber.R index d6187afb4..263e0f885 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -221,7 +221,13 @@ plumber <- R6Class( # Initialize private$serializer <- serializer_json() # Default parsers to maintain legacy features - private$parsers <- c(parser_json(), parser_query()) + private$parsers <- c( + parser_json(), + parser_query(), + parser_text(), + parser_octet(), + parser_multi() + ) private$errorHandler <- defaultErrorHandler() private$notFoundHandler <- default404Handler private$maxSize <- getOption('plumber.maxRequestSize', 0) #0 Unlimited From 0479637c158dfec9186d1c65948d3a92708c639b Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Thu, 9 Jul 2020 11:21:40 -0400 Subject: [PATCH 22/71] update doc, avoid recursively calling parser_all --- NAMESPACE | 3 +++ R/parse-body.R | 4 +++- R/plumber.R | 3 ++- man/PlumberEndpoint.Rd | 5 ++++ man/PlumberStatic.Rd | 1 + man/add_parser.Rd | 53 ++++++++++++++++++++++++++---------------- man/parsers.Rd | 28 +++++++++++++++++++++- man/plumber.Rd | 34 ++++++++++++++++++++++++++- 8 files changed, 107 insertions(+), 24 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index a7502d9dd..5318fb125 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -16,10 +16,13 @@ export(include_file) export(include_html) export(include_md) export(include_rmd) +export(list_parsers) export(options_plumber) +export(parser_all) export(parser_csv) export(parser_json) export(parser_multi) +export(parser_none) export(parser_octet) export(parser_query) export(parser_rds) diff --git a/R/parse-body.R b/R/parse-body.R index 48d63f110..74225de12 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -334,8 +334,10 @@ parser_multi <- function() { #' @describeIn parsers All parsers #' @export parser_all <- function() { + parsers <- .globals$parsers + parsers$all <- NULL return(invisible( - Reduce(function(a, b) {c(a, b())}, .globals$parsers, init = list()) + Reduce(function(a, b) {c(a, b())}, parsers, init = list()) )) } diff --git a/R/plumber.R b/R/plumber.R index 263e0f885..07c176812 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -222,7 +222,7 @@ plumber <- R6Class( private$serializer <- serializer_json() # Default parsers to maintain legacy features private$parsers <- c( - parser_json(), + parser_json(), parser_query(), parser_text(), parser_octet(), @@ -473,6 +473,7 @@ plumber <- R6Class( #' @param handler a handler function. #' @param preempt a preempt function. #' @param serializer a serializer function. + #' @param parsers a named list of parsers. #' @param endpoint a `PlumberEndpoint` object. #' @param ... additional arguments for `PlumberEndpoint` creation #' @details The “handler” functions that you define in these handle calls diff --git a/man/PlumberEndpoint.Rd b/man/PlumberEndpoint.Rd index fd7d751fb..effcc6ef5 100644 --- a/man/PlumberEndpoint.Rd +++ b/man/PlumberEndpoint.Rd @@ -33,6 +33,8 @@ each separate verb/path into its own endpoint, so we just do that.} \item{\code{params}}{endpoint parameters} \item{\code{tags}}{endpoint tags} + +\item{\code{parsers}}{step allowed parsers} } \if{html}{\out{}} } @@ -98,6 +100,7 @@ Create a new \code{PlumberEndpoint} object expr, envir, serializer, + parsers, lines, params, comments, @@ -119,6 +122,8 @@ Create a new \code{PlumberEndpoint} object \item{\code{serializer}}{endpoint serializer} +\item{\code{parsers}}{endpoint parsers} + \item{\code{lines}}{endpoint block} \item{\code{params}}{endpoint params} diff --git a/man/PlumberStatic.Rd b/man/PlumberStatic.Rd index 41fd31488..528967e4f 100644 --- a/man/PlumberStatic.Rd +++ b/man/PlumberStatic.Rd @@ -44,6 +44,7 @@ Creates a router that is backed by a directory of files on disk. \item \out{}\href{../../plumber/html/plumber.html#method-serve}{\code{plumber::plumber$serve()}}\out{} \item \out{}\href{../../plumber/html/plumber.html#method-set404Handler}{\code{plumber::plumber$set404Handler()}}\out{} \item \out{}\href{../../plumber/html/plumber.html#method-setErrorHandler}{\code{plumber::plumber$setErrorHandler()}}\out{} +\item \out{}\href{../../plumber/html/plumber.html#method-setParsers}{\code{plumber::plumber$setParsers()}}\out{} \item \out{}\href{../../plumber/html/plumber.html#method-setSerializer}{\code{plumber::plumber$setSerializer()}}\out{} \item \out{}\href{../../plumber/html/plumber.html#method-swaggerFile}{\code{plumber::plumber$swaggerFile()}}\out{} } diff --git a/man/add_parser.Rd b/man/add_parser.Rd index 53102ceee..eec11a6f9 100644 --- a/man/add_parser.Rd +++ b/man/add_parser.Rd @@ -2,43 +2,56 @@ % Please edit documentation in R/parse-body.R \name{add_parser} \alias{add_parser} +\alias{list_parsers} \title{Add a Parsers} \usage{ -add_parser(content_type, parser, verbose = TRUE) +add_parser(alias, parser, verbose = TRUE) + +list_parsers() } \arguments{ -\item{content_type}{A string to match against the content-type of each part of -the request body} +\item{alias}{Short name to map parser from the \verb{@parser} plumber tag.} -\item{parser}{The parser function to be added. This function should possibly -accept \code{value} and the named parameters \code{content_type} and \code{filename}. -Other parameters may be provided from \code{\link[webutils:parse_multipart]{webutils::parse_multipart()}}. -To be safe, add a \code{...} to your function signature.} +\item{parser}{The parser function to be added. This build the parser function.} \item{verbose}{Logical value which determines if a warning should be -displayed when patterns are overwritten.} +displayed when alias in map are overwritten.} } \description{ A parser is responsible for decoding the raw body content of a request into a list of arguments that can be mapped to endpoint function arguments. -For instance, the \code{parser_json} parser content-type \code{application/json}. +For instance, \code{parser_json} parse content-type \code{application/json}. } \details{ -Parser function structure is something like below. Available parameters -to build parser are \code{value}, \code{content_type} and \code{filename} (only available -in \code{multipart-form} body).\if{html}{\out{
}}\preformatted{parser <- function(value, content_type = "ct", filename, ...) \{ +When \code{parser} is evaluated, it should return a named list of functions. +Content-types/Mime-types are used as the list names and will be matched to +corresponding parsing function. +Functions signature in the list should include \code{value}, \code{...} and +possibly \code{content_type}, \code{filename}. Other parameters may be provided +if you want to use the headers from \code{\link[webutils:parse_multipart]{webutils::parse_multipart()}}. +Parser function structure is something like below.\if{html}{\out{
}}\preformatted{parser <- () \{ + f <- function(value, ...) \{ # do something with raw value + \} + list("ct" = f) \} }\if{html}{\out{
}} - -It should return a named list if you want values to map to -plumber endpoint function args. } +\section{Functions}{ +\itemize{ +\item \code{list_parsers}: List currently registered parsers +}} + \examples{ -parser_dcf <- function(value, content_type = "text/x-dcf", ...) { - charset <- getCharacterSet(content_type) - value <- rawToChar(value) - Encoding(value) <- charset - read.dcf(value) +# Content-type header is mostly used to look up charset and adjust encoding +parser_dcf <- function() { + parse_func <- function(value, content_type = "text/x-dcf", ...) { + charset <- getCharacterSet(content_type) + value <- rawToChar(value) + Encoding(value) <- charset + read.dcf(value) + } + return(invisible(list("text/x-dcf" = parse_func))) } +add_parser("dcf", parser_dcf) } diff --git a/man/parsers.Rd b/man/parsers.Rd index 15258cdac..67544c9ff 100644 --- a/man/parsers.Rd +++ b/man/parsers.Rd @@ -11,6 +11,8 @@ \alias{parser_rds} \alias{parser_octet} \alias{parser_multi} +\alias{parser_all} +\alias{parser_none} \title{Plumber Parsers} \usage{ parser_query() @@ -32,6 +34,10 @@ parser_rds(...) parser_octet() parser_multi() + +parser_all() + +parser_none() } \arguments{ \item{...}{parameters supplied to the appropriate internal function} @@ -46,6 +52,17 @@ by a request to the API. Extra parameters may be provided to parser functions when adding the parser to plumber. This will allow for non-default behavior. } +\details{ +Parsers are optional. When unspecified, only the \code{\link[=parser_json]{parser_json()}} and +\code{\link[=parser_query]{parser_query()}} are available. You can use \verb{@parser parser} tag to +activate parsers per endpoint. Multiple parsers can be activated for +the same endpoint using multiple \verb{@parser parser} tags. + +User should be aware that \code{rds} parsing should only be done from a +trusted source. Do not accept \code{rds} files blindly. + +See \code{\link[=list_parsers]{list_parsers()}} for a list of registered parsers. +} \section{Functions}{ \itemize{ \item \code{parser_query}: Query string parser @@ -68,11 +85,20 @@ This parser should be used when reading from a file is required. \item \code{parser_octet}: Octet stream parser \item \code{parser_multi}: Multi part parser. This parser will then parse each individual body with its respective parser + +\item \code{parser_all}: All parsers + +\item \code{parser_none}: No parser }} \examples{ \dontrun{ # Overwrite `text/json` parsing behavior to not allow JSON vectors to be simplified -add_parser("text/json", parser_json(simplifyVector = FALSE)) +#* @parser json simplifyVector = FALSE +# Activate `rds` parser in a multipart request +#* @parser multi +#* @parser rds +pr <- plumber$new() +pr$handle("GET", "/upload", function(rds) {rds}, parsers = c(parser_multi(), parser_rds())) } } diff --git a/man/plumber.Rd b/man/plumber.Rd index 485b1d529..68999570c 100644 --- a/man/plumber.Rd +++ b/man/plumber.Rd @@ -144,6 +144,7 @@ pr$setErrorHandler(function(req, res) {cat(res$body)}) \item \href{#method-onHeaders}{\code{plumber$onHeaders()}} \item \href{#method-onWSOpen}{\code{plumber$onWSOpen()}} \item \href{#method-setSerializer}{\code{plumber$setSerializer()}} +\item \href{#method-setParsers}{\code{plumber$setParsers()}} \item \href{#method-set404Handler}{\code{plumber$set404Handler()}} \item \href{#method-setErrorHandler}{\code{plumber$setErrorHandler()}} \item \href{#method-filter}{\code{plumber$filter()}} @@ -382,7 +383,16 @@ pr$handle("GET", "/", function(){ 123 }) \subsection{Method \code{handle()}}{ Define endpoints \subsection{Usage}{ -\if{html}{\out{
}}\preformatted{plumber$handle(methods, path, handler, preempt, serializer, endpoint, ...)}\if{html}{\out{
}} +\if{html}{\out{
}}\preformatted{plumber$handle( + methods, + path, + handler, + preempt, + serializer, + parsers, + endpoint, + ... +)}\if{html}{\out{
}} } \subsection{Arguments}{ @@ -398,6 +408,8 @@ Define endpoints \item{\code{serializer}}{a serializer function.} +\item{\code{parsers}}{a named list of parsers.} + \item{\code{endpoint}}{a \code{PlumberEndpoint} object.} \item{\code{...}}{additional arguments for \code{PlumberEndpoint} creation} @@ -572,6 +584,26 @@ required for httpuv interface Sets the default serializer of the router. } +} +\if{html}{\out{
}} +\if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-setParsers}{}}} +\subsection{Method \code{setParsers()}}{ +\subsection{Usage}{ +\if{html}{\out{
}}\preformatted{plumber$setParsers(parsers)}\if{html}{\out{
}} +} + +\subsection{Arguments}{ +\if{html}{\out{
}} +\describe{ +\item{\code{parsers}}{a named list of parsers} +} +\if{html}{\out{
}} +} +\subsection{Details}{ +Sets the default parsers of the router. +} + } \if{html}{\out{
}} \if{html}{\out{}} From 290fbf9b83dc3fb53a02eae0270d2e485f2a78a1 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Thu, 9 Jul 2020 12:21:34 -0400 Subject: [PATCH 23/71] re-added parser to content-type test --- tests/testthat/test-parse-body.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-parse-body.R b/tests/testthat/test-parse-body.R index d1beb2b04..8c0460298 100644 --- a/tests/testthat/test-parse-body.R +++ b/tests/testthat/test-parse-body.R @@ -31,7 +31,7 @@ test_that("filter passes on content-type", { print(content_type) body }, - expect_output(postbody_parser(req, ), "text/html; charset=testset"), + expect_output(postbody_parser(req, parser_text()), "text/html; charset=testset"), .env = "plumber" ) }) From 4c578a185d7538b92d7b3c107af2389c0e9a56fb Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Thu, 9 Jul 2020 14:03:08 -0400 Subject: [PATCH 24/71] Adding test for the parser tag --- R/plumb-block.R | 6 +++- tests/testthat/files/parsers.R | 54 +++++++++++++++++++++++++++++++ tests/testthat/test-parse-block.R | 23 +++++++++++++ tests/testthat/test-parser.R | 18 +++++++++++ 4 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/files/parsers.R create mode 100644 tests/testthat/test-parser.R diff --git a/R/plumb-block.R b/R/plumb-block.R index 0bc6a619a..788ebe6c7 100644 --- a/R/plumb-block.R +++ b/R/plumb-block.R @@ -173,7 +173,11 @@ plumbBlock <- function(lineNum, file){ } tryCatch({ # Use modifyList instead of c to avoid duplicated parsers name - parsers <- utils::modifyList(parsers, do.call(parser_builder, argList)) + if (is.null(parsers)) { + parsers <- do.call(parser_builder, argList) + } else { + parsers <- utils::modifyList(parsers, do.call(parser_builder, argList)) + } }, error = function(e) { stopOnLine(lineNum, line, paste0("Error creating parser: ", parser_alias, "\n", e)) }) diff --git a/tests/testthat/files/parsers.R b/tests/testthat/files/parsers.R new file mode 100644 index 000000000..6856692c5 --- /dev/null +++ b/tests/testthat/files/parsers.R @@ -0,0 +1,54 @@ +#* @post /none +#* @parser none +function(...){ + ret <- list(...) + ret$req <- NULL + ret$res <- NULL + ret +} + +#* @post /all +#* @parser all +function(...){ + ret <- list(...) + ret$req <- NULL + ret$res <- NULL + ret +} + +#* @post /default +function(...){ + ret <- list(...) + ret$req <- NULL + ret$res <- NULL + ret +} + +#* @post /json +#* @parser json +function(...){ + ret <- list(...) + ret$req <- NULL + ret$res <- NULL + ret +} + +#* @post /mixed +#* @parser json +#* @parser query +function(...){ + ret <- list(...) + ret$req <- NULL + ret$res <- NULL + ret +} + +#* @post /repeated +#* @parser json +#* @parser json +function(...){ + ret <- list(...) + ret$req <- NULL + ret$res <- NULL + ret +} diff --git a/tests/testthat/test-parse-block.R b/tests/testthat/test-parse-block.R index 5a8985bb0..89372e779 100644 --- a/tests/testthat/test-parse-block.R +++ b/tests/testthat/test-parse-block.R @@ -160,4 +160,27 @@ test_that("@html parameters produce an error", { expect_block_error("#' @html (key = \"val\")", "unused argument") }) +test_that("@parser parameters produce an error or not", { + # due to covr changing some code, the return answer is very strange + testthat::skip_on_covr() + + expect_block_fn <- function(lines, fn) { + b <- plumber:::plumbBlock(length(lines), lines) + expect_equal_functions(b$parsers, fn) + } + expect_block_error <- function(lines, ...) { + expect_error({ + plumbBlock(length(lines), lines) + }, ...) + } + + expect_block_fn("#' @parser octet", parser_octet()) + + expect_block_fn("#' @parser octet list()", parser_octet()) + expect_block_fn("#' @parser octet list( )", parser_octet()) + expect_block_fn("#' @parser octet list ( ) ", parser_octet()) + + expect_block_error("#' @parser octet list(key = \"val\")", "unused argument") +}) + # TODO: more testing around filter, assets, endpoint, etc. diff --git a/tests/testthat/test-parser.R b/tests/testthat/test-parser.R new file mode 100644 index 000000000..496d84bd9 --- /dev/null +++ b/tests/testthat/test-parser.R @@ -0,0 +1,18 @@ +context("Parsers tag") + +test_that("parsers work", { + r <- plumber$new(test_path("files/parsers.R")) + res <- PlumberResponse$new() + expect_identical(r$route(make_req("POST", "/none", body='{"a":1}'), res), structure(list(), names = character())) + expect_identical(r$route(make_req("POST", "/all", body='{"a":1}'), res), structure(list(1L), names = "a")) + expect_identical(r$route(make_req("POST", "/default", body='{"a":1}'), res), structure(list(1L), names = "a")) + bin_file <- test_path("files/multipart-ctype.bin") + bin_body <- readBin(bin_file, "raw", file.info(bin_file)$size) + expect_identical(r$route(make_req("POST", "/none", body=rawToChar(bin_body)), res), structure(list(), names = character())) + expect_message(r$route(make_req("POST", "/json", body=rawToChar(bin_body)), res), "No suitable parser found") + expect_equal(r$routes$none$parsers, list()) + expect_equal(r$routes$all$parsers, parser_all()) + expect_equal(r$routes$default$parsers, NULL) + expect_equal(r$routes$json$parsers, parser_json()) + expect_equal(r$routes$repeated$parsers, parser_json()) +}) From b5efad460296eeccecdc433a2ce64bd3bbb95d4b Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Thu, 9 Jul 2020 14:30:48 -0400 Subject: [PATCH 25/71] parse_json instead of fromJSON --- NAMESPACE | 1 - R/json.R | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index 5318fb125..4cbe80c4e 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -46,7 +46,6 @@ import(R6) import(crayon) import(promises) import(stringi) -importFrom(jsonlite,fromJSON) importFrom(jsonlite,toJSON) importFrom(jsonlite,validate) importFrom(stats,runif) diff --git a/R/json.R b/R/json.R index dd391f88f..88e9b2874 100644 --- a/R/json.R +++ b/R/json.R @@ -1,8 +1,8 @@ -#' @importFrom jsonlite validate fromJSON toJSON +#' @importFrom jsonlite validate toJSON #' @noRd -safeFromJSON <- function(txt, ...) { +safeFromJSON <- function(txt, simplifyVector = TRUE, ...) { if (!validate(txt)) { stop("Argument 'txt' is not a valid JSON string.") } - fromJSON(txt, ...) + jsonlite::parse_json(txt, simplifyVector, ...) } From 8955d8fe2f5647fdd43bd857784266f7456813b0 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Thu, 9 Jul 2020 20:40:35 -0400 Subject: [PATCH 26/71] Rework parser matching to also do regex on top of (after) fixed match + lower case content-type Added helper function make_parsers to transform into named list. Fix order of plumbed parsers to match order in plumbed file. --- NAMESPACE | 1 + R/parse-body.R | 143 ++++++++++++++++++++--------------- R/plumb-block.R | 4 +- man/add_parser.Rd | 15 +++- tests/testthat/test-parser.R | 1 + 5 files changed, 99 insertions(+), 65 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index 4cbe80c4e..fc98ad7e0 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -17,6 +17,7 @@ export(include_html) export(include_md) export(include_rmd) export(list_parsers) +export(make_parsers) export(options_plumber) export(parser_all) export(parser_csv) diff --git a/R/parse-body.R b/R/parse-body.R index da212a843..e765a5cf1 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -28,7 +28,12 @@ parse_body <- function(body, content_type = NULL, parsers = NULL) { parse_raw <- function(toparse) { if (length(toparse$value) == 0L) return(list()) - parser <- parser_picker(toparse$content_type, toparse$value[1], toparse$filename, toparse$parsers) + parser <- parser_picker( + # Lower case content_type for parser matching + tolower(toparse$content_type), + toparse$value[1], + toparse$filename, + toparse$parsers) if (!is.null(parser)) { return(do.call(parser, toparse)) } else { @@ -40,13 +45,13 @@ parse_raw <- function(toparse) { parser_picker <- function(content_type, first_byte, filename = NULL, parsers = NULL) { # parse as a query string - if (is.null(content_type)) { + if (length(content_type) == 0) { # fast default to json when first byte is 7b (ascii {) if (first_byte == as.raw(123L)) { - return(parsers$json) + return(parsers$json_) } - return(parsers$query) + return(parsers$query_) } # remove trailing content type information @@ -59,24 +64,29 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N parser <- parsers[[content_type]] - # return known parser + # return known parser (exact match) if (!is.null(parser)) { return(parser) } - # return text parser - if (stri_startswith_fixed(content_type, "text/")) { - # text parser - return(parsers$text) + fpm <- stri_detect_regex( + content_type, + names(parsers), + max_count = 1) + fpm[is.na(fpm)] <- FALSE + + # return known parser (first pattern match) + if (any(fpm)) { + return(parsers[[which(fpm)]]) } # query string if (is.null(filename)) { - return(parsers$query) + return(parsers$query_) } # octet - parsers$octet + parsers$octet_ } @@ -111,13 +121,14 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' @examples #' # Content-type header is mostly used to look up charset and adjust encoding #' parser_dcf <- function() { -#' parse_func <- function(value, content_type = "text/x-dcf", ...) { +#' f <- function(value, content_type = "text/x-dcf", ...) { #' charset <- getCharacterSet(content_type) #' value <- rawToChar(value) #' Encoding(value) <- charset #' read.dcf(value) #' } -#' return(invisible(list("text/x-dcf" = parse_func))) +#' ct <- "text/x-dcf" +#' return(make_parsers(f, ct)) #' } #' add_parser("dcf", parser_dcf) #' @export @@ -142,6 +153,22 @@ list_parsers <- function() { .globals$parsers } +#' @export +#' @describeIn add_parser Make named list. Mapping content-type with parser. +#' @param parser_function A single functions to map to one or more Content-Type. +#' @param content_type A vector of Content-Type or regex to be matched against a request +#' Content-Type. +make_parsers <- function(parser_function, content_type) { + invisible( + structure( + replicate( + length(content_type), + parser_function), + names = content_type + ) + ) +} + #' Plumber Parsers #' #' Parsers are used in Plumber to transform the raw body content received @@ -173,25 +200,24 @@ list_parsers <- function() { #' } #' @export parser_query <- function() { - parse_func <- parser_text(parseQS)[[1]] - return(invisible( - list("application/x-www-form-urlencoded" = parse_func, - "query" = parse_func) - )) + f <- parser_text(parseQS)[[1]] + ct <- c("application/x-www-form-urlencoded", + "query_") + return(make_parsers(f, ct)) } #' @describeIn parsers JSON parser #' @export parser_json <- function(...) { - parse_func <- parser_text(function(value) { + f <- parser_text(function(value) { safeFromJSON(value, ...) })[[1]] - return(invisible( - list("application/json" = parse_func, - "text/json" = parse_func, - "json" = parse_func) - )) + ct <- c("application/json", + "text/json", + "json$", + "json_") + return(make_parsers(f, ct)) } @@ -200,34 +226,32 @@ parser_json <- function(...) { #' @export parser_text <- function(parse_fn = identity) { stopifnot(is.function(parse_fn)) - parse_func <- function(value, content_type = NULL, ...) { + f <- function(value, content_type = NULL, ...) { charset <- getCharacterSet(content_type) value <- rawToChar(value) Encoding(value) <- charset parse_fn(value) } - return(invisible( - list("text/plain" = parse_func, - "text" = parse_func) - )) + ct <- c("text/plain", + "^text/") + return(make_parsers(f, ct)) } #' @describeIn parsers YAML parser #' @export parser_yaml <- function(...) { - parse_func <- parser_text(function(val) { + f <- parser_text(function(val) { if (!requireNamespace("yaml", quietly = TRUE)) { stop("yaml must be installed for the yaml parser to work") } yaml::yaml.load(val, ..., eval.expr = FALSE) })[[1]] - return(invisible( - list("application/yaml" = parse_func, - "application/x-yaml" = parse_func, - "text/yaml" = parse_func, - "text/x-yaml" = parse_func) - )) + ct <- c("application/yaml", + "application/x-yaml", + "text/yaml", + "text/x-yaml") + return(make_parsers(f, ct)) } #' @describeIn parsers Helper parser that writes the binary post body to a file and reads it back again using `read_fn`. @@ -249,54 +273,50 @@ parser_read_file <- function(read_fn = readLines) { #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { - parse_func <- parser_read_file(function(val) { + f <- parser_read_file(function(val) { utils::read.csv(val, ...) }) - return(invisible( - list("application/csv" = parse_func, - "application/x-csv" = parse_func, - "text/csv" = parse_func, - "text/x-csv" = parse_func) - )) + ct <- c("application/csv", + "application/x-csv", + "text/csv", + "text/x-csv") + return(make_parsers(f, ct)) } #' @describeIn parsers TSV parser #' @export parser_tsv <- function(...) { - parse_func <- parser_read_file(function(val) { + f <- parser_read_file(function(val) { utils::read.delim(val, ...) }) - return(invisible( - list("application/tab-separated-values" = parse_func, - "text/tab-separated-values" = parse_func) - )) + ct <- c("application/tab-separated-values", + "text/tab-separated-values") + return(make_parsers(f, ct)) } #' @describeIn parsers RDS parser #' @export parser_rds <- function(...) { - parse_func <- parser_read_file(function(value) { + f <- parser_read_file(function(value) { readRDS(value, ...) }) - return(invisible( - list("application/rds" = parse_func) - )) + ct <- "application/rds" + return(make_parsers(f, ct)) } #' @describeIn parsers Octet stream parser #' @export parser_octet <- function() { - parse_func <- function(value, filename = NULL, ...) { + f <- function(value, filename = NULL, ...) { attr(value, "filename") <- filename value } - return(invisible( - list("application/octet-stream" = parse_func, - "octet" = parse_func) - )) + ct <- c("application/octet-stream", + "octet_") + return(make_parsers(f, ct)) } @@ -304,7 +324,7 @@ parser_octet <- function() { #' @export #' @importFrom webutils parse_multipart parser_multi <- function() { - parse_func <- function(value, content_type, parsers, ...) { + f <- function(value, content_type, parsers, ...) { if (!stri_detect_fixed(content_type, "boundary=", case_insensitive = TRUE)) stop("No boundary found in multipart content-type header: ", content_type) boundary <- stri_match_first_regex(content_type, "boundary=([^; ]{2,})", case_insensitive = TRUE)[,2] @@ -326,9 +346,8 @@ parser_multi <- function() { parse_raw(x) }) } - return(invisible( - list("multipart/form-data" = parse_func) - )) + ct <- "multipart/form-data" + return(make_parsers(f, ct)) } #' @describeIn parsers All parsers @@ -337,7 +356,7 @@ parser_all <- function() { parsers <- .globals$parsers parsers$all <- NULL return(invisible( - Reduce(function(a, b) {c(a, b())}, parsers, init = list()) + Reduce(function(l, p) {c(l, p())}, parsers, init = list()) )) } diff --git a/R/plumb-block.R b/R/plumb-block.R index 788ebe6c7..1e0a631ee 100644 --- a/R/plumb-block.R +++ b/R/plumb-block.R @@ -176,7 +176,9 @@ plumbBlock <- function(lineNum, file){ if (is.null(parsers)) { parsers <- do.call(parser_builder, argList) } else { - parsers <- utils::modifyList(parsers, do.call(parser_builder, argList)) + # Since we plumb from bottom to top, put currently plumbed parsers in front + # Parsers will be added in the order the appear in the plumbed file + parsers <- utils::modifyList(do.call(parser_builder, argList), parsers) } }, error = function(e) { stopOnLine(lineNum, line, paste0("Error creating parser: ", parser_alias, "\n", e)) diff --git a/man/add_parser.Rd b/man/add_parser.Rd index eec11a6f9..ef739349c 100644 --- a/man/add_parser.Rd +++ b/man/add_parser.Rd @@ -3,11 +3,14 @@ \name{add_parser} \alias{add_parser} \alias{list_parsers} +\alias{make_parsers} \title{Add a Parsers} \usage{ add_parser(alias, parser, verbose = TRUE) list_parsers() + +make_parsers(parser_function, content_type) } \arguments{ \item{alias}{Short name to map parser from the \verb{@parser} plumber tag.} @@ -16,6 +19,11 @@ list_parsers() \item{verbose}{Logical value which determines if a warning should be displayed when alias in map are overwritten.} + +\item{parser_function}{A single functions to map to one or more Content-Type.} + +\item{content_type}{A vector of Content-Type or regex to be matched against a request +Content-Type.} } \description{ A parser is responsible for decoding the raw body content of a request into @@ -40,18 +48,21 @@ Parser function structure is something like below.\if{html}{\out{
\section{Functions}{ \itemize{ \item \code{list_parsers}: List currently registered parsers + +\item \code{make_parsers}: Make named list. Mapping content-type with parser. }} \examples{ # Content-type header is mostly used to look up charset and adjust encoding parser_dcf <- function() { - parse_func <- function(value, content_type = "text/x-dcf", ...) { + f <- function(value, content_type = "text/x-dcf", ...) { charset <- getCharacterSet(content_type) value <- rawToChar(value) Encoding(value) <- charset read.dcf(value) } - return(invisible(list("text/x-dcf" = parse_func))) + ct <- "text/x-dcf" + return(make_parsers(f, ct)) } add_parser("dcf", parser_dcf) } diff --git a/tests/testthat/test-parser.R b/tests/testthat/test-parser.R index 496d84bd9..7600db547 100644 --- a/tests/testthat/test-parser.R +++ b/tests/testthat/test-parser.R @@ -14,5 +14,6 @@ test_that("parsers work", { expect_equal(r$routes$all$parsers, parser_all()) expect_equal(r$routes$default$parsers, NULL) expect_equal(r$routes$json$parsers, parser_json()) + expect_equal(r$routes$mixed$parsers, c(parser_json(), parser_query())) expect_equal(r$routes$repeated$parsers, parser_json()) }) From bc0264c25057ccdba6531e2d14eb7cfc5ce31e70 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Sun, 19 Jul 2020 21:14:56 -0400 Subject: [PATCH 27/71] modify make_parsers to handle shortname, fixed and regex for Content-Type match --- DESCRIPTION | 2 +- R/parse-body.R | 86 ++++++++++++++------------------ R/plumber.R | 12 ++--- man/add_parser.Rd | 13 +++-- tests/testthat/test-parse-body.R | 8 ++- tests/testthat/test-parser.R | 2 +- 6 files changed, 58 insertions(+), 65 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 367389a3b..ae876a6fb 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -81,4 +81,4 @@ Collate: 'session-cookie.R' 'utf8.R' 'zzz.R' -RoxygenNote: 7.1.0 +RoxygenNote: 7.1.1 diff --git a/R/parse-body.R b/R/parse-body.R index e765a5cf1..83c156b4e 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -48,10 +48,10 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N if (length(content_type) == 0) { # fast default to json when first byte is 7b (ascii {) if (first_byte == as.raw(123L)) { - return(parsers$json_) + return(parsers$json) } - return(parsers$query_) + return(parsers$query) } # remove trailing content type information @@ -62,7 +62,7 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N content_type <- stri_split_fixed(content_type, ";")[[1]][1] } - parser <- parsers[[content_type]] + parser <- parsers$fixed[[content_type]] # return known parser (exact match) if (!is.null(parser)) { @@ -71,22 +71,22 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N fpm <- stri_detect_regex( content_type, - names(parsers), + names(parsers$regex), max_count = 1) fpm[is.na(fpm)] <- FALSE - # return known parser (first pattern match) + # return known parser (first regex pattern match) if (any(fpm)) { - return(parsers[[which(fpm)]]) + return(parsers$regex[[which(fpm)]]) } # query string if (is.null(filename)) { - return(parsers$query_) + return(parsers$query) } # octet - parsers$octet_ + parsers$octet } @@ -114,7 +114,7 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' f <- function(value, ...) { #' # do something with raw value #' } -#' list("ct" = f) +#' make_parsers(f, fixed = "ct") #' } #' ``` #' @@ -127,8 +127,7 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' Encoding(value) <- charset #' read.dcf(value) #' } -#' ct <- "text/x-dcf" -#' return(make_parsers(f, ct)) +#' return(make_parsers(f, fixed = "text/x-dcf")) #' } #' add_parser("dcf", parser_dcf) #' @export @@ -156,17 +155,27 @@ list_parsers <- function() { #' @export #' @describeIn add_parser Make named list. Mapping content-type with parser. #' @param parser_function A single functions to map to one or more Content-Type. -#' @param content_type A vector of Content-Type or regex to be matched against a request +#' @param fixed A character vector of fixed string to be matched against a request Content-Type. +#' @param regex A character vector of [regex] string to be matched against a request Content-Type. +#' @param shortname A character value to reference a parser by a shortname. #' Content-Type. -make_parsers <- function(parser_function, content_type) { - invisible( +make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname = NULL) { + m <- function(n) { structure( replicate( - length(content_type), + length(n), parser_function), - names = content_type + names = n ) - ) + } + parsers <- m(shortname) + if (length(fixed) > 0) { + parsers <- c(parsers, list("fixed" = m(fixed))) + } + if (length(regex) > 0) { + parsers <- c(parsers, list("regex" = m(regex))) + } + invisible(parsers) } #' Plumber Parsers @@ -201,9 +210,7 @@ make_parsers <- function(parser_function, content_type) { #' @export parser_query <- function() { f <- parser_text(parseQS)[[1]] - ct <- c("application/x-www-form-urlencoded", - "query_") - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = "application/x-www-form-urlencoded", shortname = "query")) } @@ -213,11 +220,7 @@ parser_json <- function(...) { f <- parser_text(function(value) { safeFromJSON(value, ...) })[[1]] - ct <- c("application/json", - "text/json", - "json$", - "json_") - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = c("application/json", "text/json"), regex = "json$", shortname = "json")) } @@ -232,9 +235,7 @@ parser_text <- function(parse_fn = identity) { Encoding(value) <- charset parse_fn(value) } - ct <- c("text/plain", - "^text/") - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = "text/plain", regex = "^text/")) } @@ -247,11 +248,7 @@ parser_yaml <- function(...) { } yaml::yaml.load(val, ..., eval.expr = FALSE) })[[1]] - ct <- c("application/yaml", - "application/x-yaml", - "text/yaml", - "text/x-yaml") - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = c("application/yaml", "application/x-yaml", "text/yaml", "text/x-yaml"))) } #' @describeIn parsers Helper parser that writes the binary post body to a file and reads it back again using `read_fn`. @@ -276,11 +273,7 @@ parser_csv <- function(...) { f <- parser_read_file(function(val) { utils::read.csv(val, ...) }) - ct <- c("application/csv", - "application/x-csv", - "text/csv", - "text/x-csv") - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = c("application/csv", "application/x-csv", "text/csv", "text/x-csv"))) } @@ -290,9 +283,7 @@ parser_tsv <- function(...) { f <- parser_read_file(function(val) { utils::read.delim(val, ...) }) - ct <- c("application/tab-separated-values", - "text/tab-separated-values") - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = c("application/tab-separated-values", "text/tab-separated-values"))) } @@ -302,8 +293,7 @@ parser_rds <- function(...) { f <- parser_read_file(function(value) { readRDS(value, ...) }) - ct <- "application/rds" - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = "application/rds")) } @@ -314,9 +304,7 @@ parser_octet <- function() { attr(value, "filename") <- filename value } - ct <- c("application/octet-stream", - "octet_") - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = "application/octet-stream", shortname = "octet")) } @@ -346,8 +334,7 @@ parser_multi <- function() { parse_raw(x) }) } - ct <- "multipart/form-data" - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = "multipart/form-data")) } #' @describeIn parsers All parsers @@ -356,7 +343,8 @@ parser_all <- function() { parsers <- .globals$parsers parsers$all <- NULL return(invisible( - Reduce(function(l, p) {c(l, p())}, parsers, init = list()) + # Lambda function to get each parser `p()` list + Reduce(function(l, p) {utils::modifyList(p(), l)}, parsers, init = list()) )) } diff --git a/R/plumber.R b/R/plumber.R index 07c176812..db47ff9c6 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -221,13 +221,11 @@ plumber <- R6Class( # Initialize private$serializer <- serializer_json() # Default parsers to maintain legacy features - private$parsers <- c( - parser_json(), - parser_query(), - parser_text(), - parser_octet(), - parser_multi() - ) + private$parsers <- Reduce(utils::modifyList, list(parser_json(), + parser_query(), + parser_text(), + parser_octet(), + parser_multi())) private$errorHandler <- defaultErrorHandler() private$notFoundHandler <- default404Handler private$maxSize <- getOption('plumber.maxRequestSize', 0) #0 Unlimited diff --git a/man/add_parser.Rd b/man/add_parser.Rd index ef739349c..c26d59e05 100644 --- a/man/add_parser.Rd +++ b/man/add_parser.Rd @@ -10,7 +10,7 @@ add_parser(alias, parser, verbose = TRUE) list_parsers() -make_parsers(parser_function, content_type) +make_parsers(parser_function, fixed = NULL, regex = NULL, shortname = NULL) } \arguments{ \item{alias}{Short name to map parser from the \verb{@parser} plumber tag.} @@ -22,7 +22,11 @@ displayed when alias in map are overwritten.} \item{parser_function}{A single functions to map to one or more Content-Type.} -\item{content_type}{A vector of Content-Type or regex to be matched against a request +\item{fixed}{A character vector of fixed string to be matched against a request Content-Type.} + +\item{regex}{A character vector of \link{regex} string to be matched against a request Content-Type.} + +\item{shortname}{A character value to reference a parser by a shortname. Content-Type.} } \description{ @@ -41,7 +45,7 @@ Parser function structure is something like below.\if{html}{\out{
f <- function(value, ...) \{ # do something with raw value \} - list("ct" = f) + make_parsers(f, fixed = "ct") \} }\if{html}{\out{
}} } @@ -61,8 +65,7 @@ parser_dcf <- function() { Encoding(value) <- charset read.dcf(value) } - ct <- "text/x-dcf" - return(make_parsers(f, ct)) + return(make_parsers(f, fixed = "text/x-dcf")) } add_parser("dcf", parser_dcf) } diff --git a/tests/testthat/test-parse-body.R b/tests/testthat/test-parse-body.R index 8c0460298..19ccf625e 100644 --- a/tests/testthat/test-parse-body.R +++ b/tests/testthat/test-parse-body.R @@ -77,7 +77,9 @@ test_that("Test multipart parser", { bin_file <- test_path("files/multipart-form.bin") body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) - parsed_body <- parse_body(body, "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ", c(parser_multi(), parser_json(), parser_rds(), parser_octet())) + parsed_body <- parse_body(body, + "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ", + Reduce(utils::modifyList, list(parser_multi(), parser_json(), parser_rds(), parser_octet()))) expect_equal(names(parsed_body), c("json", "img1", "img2", "rds")) expect_equal(parsed_body[["rds"]], women) @@ -89,6 +91,8 @@ test_that("Test multipart parser", { test_that("Test multipart respect content-type", { bin_file <- test_path("files/multipart-ctype.bin") body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) - parsed_body <- parse_body(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272", c(parser_multi(), parser_tsv())) + parsed_body <- parse_body(body, + "multipart/form-data; boundary=---------------------------90908882332870323642673870272", + Reduce(utils::modifyList, list(parser_multi(), parser_tsv()))) expect_s3_class(parsed_body$file, "data.frame") }) diff --git a/tests/testthat/test-parser.R b/tests/testthat/test-parser.R index 7600db547..24de4c46c 100644 --- a/tests/testthat/test-parser.R +++ b/tests/testthat/test-parser.R @@ -14,6 +14,6 @@ test_that("parsers work", { expect_equal(r$routes$all$parsers, parser_all()) expect_equal(r$routes$default$parsers, NULL) expect_equal(r$routes$json$parsers, parser_json()) - expect_equal(r$routes$mixed$parsers, c(parser_json(), parser_query())) + expect_equal(r$routes$mixed$parsers, Reduce(utils::modifyList, list(parser_json(), parser_query()))) expect_equal(r$routes$repeated$parsers, parser_json()) }) From 730c4bacac7faea6200ec3fedabcac9d18d0ac99 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Sun, 19 Jul 2020 21:36:12 -0400 Subject: [PATCH 28/71] slight change to parser_text reference, use shortname instead of [[1]] --- R/parse-body.R | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 83c156b4e..580d89c4c 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -160,6 +160,9 @@ list_parsers <- function() { #' @param shortname A character value to reference a parser by a shortname. #' Content-Type. make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname = NULL) { + if (any(shortname %in% c("fixed", "regex"))) { + stop("Shortnames `fixed` and `regex` are reserved for internal use.") + } m <- function(n) { structure( replicate( @@ -209,7 +212,7 @@ make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname #' } #' @export parser_query <- function() { - f <- parser_text(parseQS)[[1]] + f <- parser_text(parseQS)$text return(make_parsers(f, fixed = "application/x-www-form-urlencoded", shortname = "query")) } @@ -219,7 +222,7 @@ parser_query <- function() { parser_json <- function(...) { f <- parser_text(function(value) { safeFromJSON(value, ...) - })[[1]] + })$text return(make_parsers(f, fixed = c("application/json", "text/json"), regex = "json$", shortname = "json")) } @@ -235,7 +238,7 @@ parser_text <- function(parse_fn = identity) { Encoding(value) <- charset parse_fn(value) } - return(make_parsers(f, fixed = "text/plain", regex = "^text/")) + return(make_parsers(f, fixed = "text/plain", regex = "^text/", shortname = "text")) } @@ -247,7 +250,7 @@ parser_yaml <- function(...) { stop("yaml must be installed for the yaml parser to work") } yaml::yaml.load(val, ..., eval.expr = FALSE) - })[[1]] + })$text return(make_parsers(f, fixed = c("application/yaml", "application/x-yaml", "text/yaml", "text/x-yaml"))) } From 6ae2da5c28bdb977645bd47787a55a87bbedc6c0 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Mon, 20 Jul 2020 11:38:41 -0400 Subject: [PATCH 29/71] Update R/parse-body.R Co-authored-by: Barret Schloerke --- R/parse-body.R | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 580d89c4c..410dea79a 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -163,20 +163,20 @@ make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname if (any(shortname %in% c("fixed", "regex"))) { stop("Shortnames `fixed` and `regex` are reserved for internal use.") } - m <- function(n) { - structure( + create_list <- function(names) { + stats::setNames( replicate( - length(n), + length(names), parser_function), - names = n + names ) } - parsers <- m(shortname) + parsers <- create_list(shortname) if (length(fixed) > 0) { - parsers <- c(parsers, list("fixed" = m(fixed))) + parsers$fixed <- create_list(fixed) } if (length(regex) > 0) { - parsers <- c(parsers, list("regex" = m(regex))) + parsers$regex <- create_list(regex) } invisible(parsers) } From beb12a250b68e6ee2f4cea465580b03b4498007a Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Mon, 20 Jul 2020 11:39:10 -0400 Subject: [PATCH 30/71] Update R/parse-body.R Co-authored-by: Barret Schloerke --- R/parse-body.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index 410dea79a..c46632df3 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -157,7 +157,7 @@ list_parsers <- function() { #' @param parser_function A single functions to map to one or more Content-Type. #' @param fixed A character vector of fixed string to be matched against a request Content-Type. #' @param regex A character vector of [regex] string to be matched against a request Content-Type. -#' @param shortname A character value to reference a parser by a shortname. +#' @param shortname A character value to reference a parser by a shortname. (For internal use only) #' Content-Type. make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname = NULL) { if (any(shortname %in% c("fixed", "regex"))) { From de672af6fdd5bd29e3c5e9566b785070c7a1ad76 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Mon, 20 Jul 2020 11:45:09 -0400 Subject: [PATCH 31/71] Update R/parse-body.R Co-authored-by: Barret Schloerke --- R/parse-body.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index c46632df3..3eb1b9a7e 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -361,7 +361,7 @@ parser_none <- function() { add_parsers_onLoad <- function() { - # shorthand names for parser plumbing + # parser alias names for plumbing add_parser("csv", parser_csv) add_parser("json", parser_json) add_parser("multi", parser_multi) From edc2f12c8c22f239dd79fc819f3dc5e17a686a1f Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Mon, 20 Jul 2020 11:49:18 -0400 Subject: [PATCH 32/71] Update R/parse-body.R Co-authored-by: Barret Schloerke --- R/parse-body.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index 3eb1b9a7e..7a3ff269b 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -171,7 +171,10 @@ make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname names ) } - parsers <- create_list(shortname) + parsers <- list() + if (length(shortname) > 0) { + parsers$shortname <- create_list(shortname) + } if (length(fixed) > 0) { parsers$fixed <- create_list(fixed) } From 83effa41a347d2188acbf665b451fbbcea4d48ef Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Mon, 20 Jul 2020 11:50:11 -0400 Subject: [PATCH 33/71] Update R/parse-body.R Co-authored-by: Barret Schloerke --- R/parse-body.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/parse-body.R b/R/parse-body.R index 7a3ff269b..291b7163c 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -347,6 +347,7 @@ parser_multi <- function() { #' @export parser_all <- function() { parsers <- .globals$parsers + # remove to avoid infinite recursion parsers$all <- NULL return(invisible( # Lambda function to get each parser `p()` list From cbe8ff9fbb34e7db47752ea6c73860708ed981c3 Mon Sep 17 00:00:00 2001 From: Bruno Tremblay Date: Mon, 20 Jul 2020 16:22:46 -0400 Subject: [PATCH 34/71] Move make_parsers to add_parser and associated impact fix --- NAMESPACE | 2 +- R/parse-body.R | 188 ++++++++++++++++-------------- R/plumb-block.R | 6 +- R/plumber.R | 6 +- man/add_parser.Rd | 59 ++++++---- man/parsers.Rd | 12 +- tests/testthat/test-parse-block.R | 8 +- tests/testthat/test-parse-body.R | 24 ++-- tests/testthat/test-parser.R | 6 +- 9 files changed, 162 insertions(+), 149 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index fc98ad7e0..66ce8c9e0 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -17,7 +17,6 @@ export(include_html) export(include_md) export(include_rmd) export(list_parsers) -export(make_parsers) export(options_plumber) export(parser_all) export(parser_csv) @@ -34,6 +33,7 @@ export(parser_yaml) export(plumb) export(plumber) export(randomCookieKey) +export(select_parsers) export(serializer_content_type) export(serializer_csv) export(serializer_html) diff --git a/R/parse-body.R b/R/parse-body.R index 291b7163c..1bef0478a 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -90,48 +90,55 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N } -#' Add a Parsers +#' Manage parsers #' #' A parser is responsible for decoding the raw body content of a request into #' a list of arguments that can be mapped to endpoint function arguments. -#' For instance, \code{parser_json} parse content-type `application/json`. +#' For instance, [parser_json()] parse content-type `application/json`. #' -#' @param alias Short name to map parser from the `@parser` plumber tag. +#' @param alias An alias to map parser from the `@parser` plumber tag to the global parsers list. #' @param parser The parser function to be added. This build the parser function. +#' @param fixed A character vector of fixed string to be matched against a request `content-type` to use `parser`. +#' @param regex A character vector of [regex] string to be matched against a request `content-type` to use `parser`. +#' @param shortname A character value to reference a parser by a shortname. (For internal use only) #' @param verbose Logical value which determines if a warning should be -#' displayed when alias in map are overwritten. +#' displayed when alias in map are overwritten. #' #' @details -#' When `parser` is evaluated, it should return a named list of functions. -#' Content-types/Mime-types are used as the list names and will be matched to -#' corresponding parsing function. -#' Functions signature in the list should include `value`, `...` and +#' When `parser` is evaluated, it should return a parser function. +#' Parser matching is done first by `content-type` header matching on `fixed` then by using a +#' regular expressions on `regex`. Note that plumber strip the header from `; charset*` to +#' perform matching. +#' +#' There is a special case when no `content-type` header is +#' provided that will use a [parser_json()] when it detects a `json` string. +#' +#' Functions signature should include `value`, `...` and #' possibly `content_type`, `filename`. Other parameters may be provided #' if you want to use the headers from [webutils::parse_multipart()]. +#' #' Parser function structure is something like below. #' ```r #' parser <- () { -#' f <- function(value, ...) { +#' function(value, ...) { #' # do something with raw value #' } -#' make_parsers(f, fixed = "ct") #' } #' ``` #' #' @examples -#' # Content-type header is mostly used to look up charset and adjust encoding +#' # `content-type` header is mostly used to look up charset and adjust encoding #' parser_dcf <- function() { -#' f <- function(value, content_type = "text/x-dcf", ...) { +#' function(value, content_type = "text/x-dcf", ...) { #' charset <- getCharacterSet(content_type) #' value <- rawToChar(value) #' Encoding(value) <- charset #' read.dcf(value) #' } -#' return(make_parsers(f, fixed = "text/x-dcf")) #' } -#' add_parser("dcf", parser_dcf) +#' add_parser("dcf", parser_dcf, fixed = "text/x-dcf") #' @export -add_parser <- function(alias, parser, verbose = TRUE) { +add_parser <- function(alias, parser, fixed = NULL, regex = NULL, shortname = NULL, verbose = TRUE) { if (!is.null(.globals$parsers[[alias]])) { if (isTRUE(verbose)) { @@ -141,7 +148,44 @@ add_parser <- function(alias, parser, verbose = TRUE) { stopifnot(is.function(parser)) - .globals$parsers[[alias]] <- parser + if (length(c(fixed, regex, shortname))) { + + parsers_list <- function(...) { + + parser_function <- do.call(parser, list(...)) + + create_list <- function(names) { + stats::setNames( + replicate( + length(names), + parser_function), + names + ) + } + + parsers <- list() + + if (length(shortname) > 0) { + parsers[[shortname]] <- parser_function + } + if (length(fixed) > 0) { + parsers$fixed <- create_list(fixed) + } + if (length(regex) > 0) { + parsers$regex <- create_list(regex) + } + + return(parsers) + + } + + } else { + + parsers_list = parser + + } + + .globals$parsers[[alias]] <- parsers_list invisible(list_parsers()) } @@ -152,36 +196,17 @@ list_parsers <- function() { .globals$parsers } +#' @describeIn add_parser Select from global parsers and create +#' a formatted parsers list for programmatic use. #' @export -#' @describeIn add_parser Make named list. Mapping content-type with parser. -#' @param parser_function A single functions to map to one or more Content-Type. -#' @param fixed A character vector of fixed string to be matched against a request Content-Type. -#' @param regex A character vector of [regex] string to be matched against a request Content-Type. -#' @param shortname A character value to reference a parser by a shortname. (For internal use only) -#' Content-Type. -make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname = NULL) { - if (any(shortname %in% c("fixed", "regex"))) { - stop("Shortnames `fixed` and `regex` are reserved for internal use.") - } - create_list <- function(names) { - stats::setNames( - replicate( - length(names), - parser_function), - names - ) - } - parsers <- list() - if (length(shortname) > 0) { - parsers$shortname <- create_list(shortname) - } - if (length(fixed) > 0) { - parsers$fixed <- create_list(fixed) - } - if (length(regex) > 0) { - parsers$regex <- create_list(regex) - } - invisible(parsers) +select_parsers <- function(alias = character()) { + parsers <- .globals$parsers[alias] + # remove to avoid infinite recursion + parsers$all <- NULL + return(invisible( + # Lambda function to get each parser `p()` list + Reduce(function(l, p) {utils::modifyList(l, p())}, parsers, init = list()) + )) } #' Plumber Parsers @@ -191,10 +216,10 @@ make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname #' functions when adding the parser to plumber. This will allow for #' non-default behavior. #' -#' Parsers are optional. When unspecified, only the [parser_json()] and -#' [parser_query()] are available. You can use `@parser parser` tag to -#' activate parsers per endpoint. Multiple parsers can be activated for -#' the same endpoint using multiple `@parser parser` tags. +#' Parsers are optional. When unspecified, only the [parser_json()], +#' [parser_octet()], [parser_query()] and [parser_text()] are available. +#' You can use `@parser parser` tag to activate parsers per endpoint. +#' Multiple parsers can be activated for the same endpoint using multiple `@parser parser` tags. #' #' User should be aware that `rds` parsing should only be done from a #' trusted source. Do not accept `rds` files blindly. @@ -215,18 +240,16 @@ make_parsers <- function(parser_function, fixed = NULL, regex = NULL, shortname #' } #' @export parser_query <- function() { - f <- parser_text(parseQS)$text - return(make_parsers(f, fixed = "application/x-www-form-urlencoded", shortname = "query")) + parser_text(parseQS) } #' @describeIn parsers JSON parser #' @export parser_json <- function(...) { - f <- parser_text(function(value) { + parser_text(function(value) { safeFromJSON(value, ...) - })$text - return(make_parsers(f, fixed = c("application/json", "text/json"), regex = "json$", shortname = "json")) + }) } @@ -235,26 +258,24 @@ parser_json <- function(...) { #' @export parser_text <- function(parse_fn = identity) { stopifnot(is.function(parse_fn)) - f <- function(value, content_type = NULL, ...) { + function(value, content_type = NULL, ...) { charset <- getCharacterSet(content_type) value <- rawToChar(value) Encoding(value) <- charset parse_fn(value) } - return(make_parsers(f, fixed = "text/plain", regex = "^text/", shortname = "text")) } #' @describeIn parsers YAML parser #' @export parser_yaml <- function(...) { - f <- parser_text(function(val) { + parser_text(function(val) { if (!requireNamespace("yaml", quietly = TRUE)) { stop("yaml must be installed for the yaml parser to work") } yaml::yaml.load(val, ..., eval.expr = FALSE) - })$text - return(make_parsers(f, fixed = c("application/yaml", "application/x-yaml", "text/yaml", "text/x-yaml"))) + }) } #' @describeIn parsers Helper parser that writes the binary post body to a file and reads it back again using `read_fn`. @@ -276,41 +297,37 @@ parser_read_file <- function(read_fn = readLines) { #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { - f <- parser_read_file(function(val) { + parser_read_file(function(val) { utils::read.csv(val, ...) }) - return(make_parsers(f, fixed = c("application/csv", "application/x-csv", "text/csv", "text/x-csv"))) } #' @describeIn parsers TSV parser #' @export parser_tsv <- function(...) { - f <- parser_read_file(function(val) { + parser_read_file(function(val) { utils::read.delim(val, ...) }) - return(make_parsers(f, fixed = c("application/tab-separated-values", "text/tab-separated-values"))) } #' @describeIn parsers RDS parser #' @export parser_rds <- function(...) { - f <- parser_read_file(function(value) { + parser_read_file(function(value) { readRDS(value, ...) }) - return(make_parsers(f, fixed = "application/rds")) } #' @describeIn parsers Octet stream parser #' @export parser_octet <- function() { - f <- function(value, filename = NULL, ...) { + function(value, filename = NULL, ...) { attr(value, "filename") <- filename value } - return(make_parsers(f, fixed = "application/octet-stream", shortname = "octet")) } @@ -318,7 +335,7 @@ parser_octet <- function() { #' @export #' @importFrom webutils parse_multipart parser_multi <- function() { - f <- function(value, content_type, parsers, ...) { + function(value, content_type, parsers, ...) { if (!stri_detect_fixed(content_type, "boundary=", case_insensitive = TRUE)) stop("No boundary found in multipart content-type header: ", content_type) boundary <- stri_match_first_regex(content_type, "boundary=([^; ]{2,})", case_insensitive = TRUE)[,2] @@ -340,41 +357,32 @@ parser_multi <- function() { parse_raw(x) }) } - return(make_parsers(f, fixed = "multipart/form-data")) } -#' @describeIn parsers All parsers +#' @describeIn parsers All parsers (For internal use only) #' @export parser_all <- function() { - parsers <- .globals$parsers - # remove to avoid infinite recursion - parsers$all <- NULL - return(invisible( - # Lambda function to get each parser `p()` list - Reduce(function(l, p) {utils::modifyList(p(), l)}, parsers, init = list()) - )) + select_parsers(names(.globals$parsers)) } -#' @describeIn parsers No parser +#' @describeIn parsers No parser (For internal use only) #' @export parser_none <- function() { - return(invisible( - list() - )) + select_parsers() } add_parsers_onLoad <- function() { # parser alias names for plumbing - add_parser("csv", parser_csv) - add_parser("json", parser_json) - add_parser("multi", parser_multi) - add_parser("octet", parser_octet) - add_parser("query", parser_query) - add_parser("rds", parser_rds) - add_parser("text", parser_text) - add_parser("tsv", parser_tsv) - add_parser("yaml", parser_yaml) + add_parser("csv", parser_csv, fixed = c("application/csv", "application/x-csv", "text/csv", "text/x-csv")) + add_parser("json", parser_json, fixed = c("application/json", "text/json"), regex = "json$", shortname = "json") + add_parser("multi", parser_multi, fixed = "multipart/form-data") + add_parser("octet", parser_octet, fixed = "application/octet-stream", shortname = "octet") + add_parser("query", parser_query, fixed = "application/x-www-form-urlencoded", shortname = "query") + add_parser("rds", parser_rds, fixed = "application/rds") + add_parser("text", parser_text, fixed = "text/plain", regex = "^text/", shortname = "text") + add_parser("tsv", parser_tsv, fixed = c("application/tab-separated-values", "text/tab-separated-values")) + add_parser("yaml", parser_yaml, fixed = c("application/yaml", "application/x-yaml", "text/yaml", "text/x-yaml")) add_parser("all", parser_all) add_parser("none", parser_none) } diff --git a/R/plumb-block.R b/R/plumb-block.R index 1e0a631ee..6c04beb20 100644 --- a/R/plumb-block.R +++ b/R/plumb-block.R @@ -163,7 +163,7 @@ plumbBlock <- function(lineNum, file){ stopOnLine(lineNum, line, paste0("No such @parser registered: ", parser_alias)) } - parser_builder <- .globals$parsers[[parser_alias]] + parser <- .globals$parsers[[parser_alias]] if (!is.na(parsersMat[1, 4]) && parsersMat[1,4] != ""){ # We have an arg to pass in to the parser @@ -174,11 +174,11 @@ plumbBlock <- function(lineNum, file){ tryCatch({ # Use modifyList instead of c to avoid duplicated parsers name if (is.null(parsers)) { - parsers <- do.call(parser_builder, argList) + parsers <- do.call(parser, argList) } else { # Since we plumb from bottom to top, put currently plumbed parsers in front # Parsers will be added in the order the appear in the plumbed file - parsers <- utils::modifyList(do.call(parser_builder, argList), parsers) + parsers <- utils::modifyList(do.call(parser, argList), parsers) } }, error = function(e) { stopOnLine(lineNum, line, paste0("Error creating parser: ", parser_alias, "\n", e)) diff --git a/R/plumber.R b/R/plumber.R index db47ff9c6..7e108abb1 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -221,11 +221,7 @@ plumber <- R6Class( # Initialize private$serializer <- serializer_json() # Default parsers to maintain legacy features - private$parsers <- Reduce(utils::modifyList, list(parser_json(), - parser_query(), - parser_text(), - parser_octet(), - parser_multi())) + private$parsers <- select_parsers(c("json", "query", "text", "octet", "multi")) private$errorHandler <- defaultErrorHandler() private$notFoundHandler <- default404Handler private$maxSize <- getOption('plumber.maxRequestSize', 0) #0 Unlimited diff --git a/man/add_parser.Rd b/man/add_parser.Rd index c26d59e05..a4ac40977 100644 --- a/man/add_parser.Rd +++ b/man/add_parser.Rd @@ -3,49 +3,58 @@ \name{add_parser} \alias{add_parser} \alias{list_parsers} -\alias{make_parsers} -\title{Add a Parsers} +\alias{select_parsers} +\title{Manage parsers} \usage{ -add_parser(alias, parser, verbose = TRUE) +add_parser( + alias, + parser, + fixed = NULL, + regex = NULL, + shortname = NULL, + verbose = TRUE +) list_parsers() -make_parsers(parser_function, fixed = NULL, regex = NULL, shortname = NULL) +select_parsers(alias = character()) } \arguments{ -\item{alias}{Short name to map parser from the \verb{@parser} plumber tag.} +\item{alias}{An alias to map parser from the \verb{@parser} plumber tag to the global parsers list.} \item{parser}{The parser function to be added. This build the parser function.} -\item{verbose}{Logical value which determines if a warning should be -displayed when alias in map are overwritten.} +\item{fixed}{A character vector of fixed string to be matched against a request \code{content-type} to use \code{parser}.} -\item{parser_function}{A single functions to map to one or more Content-Type.} +\item{regex}{A character vector of \link{regex} string to be matched against a request \code{content-type} to use \code{parser}.} -\item{fixed}{A character vector of fixed string to be matched against a request Content-Type.} +\item{shortname}{A character value to reference a parser by a shortname. (For internal use only)} -\item{regex}{A character vector of \link{regex} string to be matched against a request Content-Type.} - -\item{shortname}{A character value to reference a parser by a shortname. -Content-Type.} +\item{verbose}{Logical value which determines if a warning should be +displayed when alias in map are overwritten.} } \description{ A parser is responsible for decoding the raw body content of a request into a list of arguments that can be mapped to endpoint function arguments. -For instance, \code{parser_json} parse content-type \code{application/json}. +For instance, \code{\link[=parser_json]{parser_json()}} parse content-type \code{application/json}. } \details{ -When \code{parser} is evaluated, it should return a named list of functions. -Content-types/Mime-types are used as the list names and will be matched to -corresponding parsing function. -Functions signature in the list should include \code{value}, \code{...} and +When \code{parser} is evaluated, it should return a parser function. +Parser matching is done first by \code{content-type} header matching on \code{fixed} then by using a +regular expressions on \code{regex}. Note that plumber strip the header from \verb{; charset*} to +perform matching. + +There is a special case when no \code{content-type} header is +provided that will use a \code{\link[=parser_json]{parser_json()}} when it detects a \code{json} string. + +Functions signature should include \code{value}, \code{...} and possibly \code{content_type}, \code{filename}. Other parameters may be provided if you want to use the headers from \code{\link[webutils:parse_multipart]{webutils::parse_multipart()}}. + Parser function structure is something like below.\if{html}{\out{
}}\preformatted{parser <- () \{ - f <- function(value, ...) \{ + function(value, ...) \{ # do something with raw value \} - make_parsers(f, fixed = "ct") \} }\if{html}{\out{
}} } @@ -53,19 +62,19 @@ Parser function structure is something like below.\if{html}{\out{
\itemize{ \item \code{list_parsers}: List currently registered parsers -\item \code{make_parsers}: Make named list. Mapping content-type with parser. +\item \code{select_parsers}: Select from global parsers and create +a formatted parsers list for programmatic use. }} \examples{ -# Content-type header is mostly used to look up charset and adjust encoding +# `content-type` header is mostly used to look up charset and adjust encoding parser_dcf <- function() { - f <- function(value, content_type = "text/x-dcf", ...) { + function(value, content_type = "text/x-dcf", ...) { charset <- getCharacterSet(content_type) value <- rawToChar(value) Encoding(value) <- charset read.dcf(value) } - return(make_parsers(f, fixed = "text/x-dcf")) } -add_parser("dcf", parser_dcf) +add_parser("dcf", parser_dcf, fixed = "text/x-dcf") } diff --git a/man/parsers.Rd b/man/parsers.Rd index 67544c9ff..7bc5e1c45 100644 --- a/man/parsers.Rd +++ b/man/parsers.Rd @@ -53,10 +53,10 @@ functions when adding the parser to plumber. This will allow for non-default behavior. } \details{ -Parsers are optional. When unspecified, only the \code{\link[=parser_json]{parser_json()}} and -\code{\link[=parser_query]{parser_query()}} are available. You can use \verb{@parser parser} tag to -activate parsers per endpoint. Multiple parsers can be activated for -the same endpoint using multiple \verb{@parser parser} tags. +Parsers are optional. When unspecified, only the \code{\link[=parser_json]{parser_json()}}, +\code{\link[=parser_octet]{parser_octet()}}, \code{\link[=parser_query]{parser_query()}} and \code{\link[=parser_text]{parser_text()}} are available. +You can use \verb{@parser parser} tag to activate parsers per endpoint. +Multiple parsers can be activated for the same endpoint using multiple \verb{@parser parser} tags. User should be aware that \code{rds} parsing should only be done from a trusted source. Do not accept \code{rds} files blindly. @@ -86,9 +86,9 @@ This parser should be used when reading from a file is required. \item \code{parser_multi}: Multi part parser. This parser will then parse each individual body with its respective parser -\item \code{parser_all}: All parsers +\item \code{parser_all}: All parsers (For internal use only) -\item \code{parser_none}: No parser +\item \code{parser_none}: No parser (For internal use only) }} \examples{ diff --git a/tests/testthat/test-parse-block.R b/tests/testthat/test-parse-block.R index 89372e779..a97f20d1a 100644 --- a/tests/testthat/test-parse-block.R +++ b/tests/testthat/test-parse-block.R @@ -174,11 +174,11 @@ test_that("@parser parameters produce an error or not", { }, ...) } - expect_block_fn("#' @parser octet", parser_octet()) + expect_block_fn("#' @parser octet", select_parsers("octet")) - expect_block_fn("#' @parser octet list()", parser_octet()) - expect_block_fn("#' @parser octet list( )", parser_octet()) - expect_block_fn("#' @parser octet list ( ) ", parser_octet()) + expect_block_fn("#' @parser octet list()", select_parsers("octet")) + expect_block_fn("#' @parser octet list( )", select_parsers("octet")) + expect_block_fn("#' @parser octet list ( ) ", select_parsers("octet")) expect_block_error("#' @parser octet list(key = \"val\")", "unused argument") }) diff --git a/tests/testthat/test-parse-body.R b/tests/testthat/test-parse-body.R index 19ccf625e..56720ad55 100644 --- a/tests/testthat/test-parse-body.R +++ b/tests/testthat/test-parse-body.R @@ -1,21 +1,21 @@ context("POST body") test_that("JSON is consumed on POST", { - expect_equal(parse_body('{"a":"1"}', content_type = NULL, parsers = parser_json()), list(a = "1")) + expect_equal(parse_body('{"a":"1"}', content_type = NULL, parsers = select_parsers("json")), list(a = "1")) }) test_that("ending in `==` does not produce a unexpected key", { # See https://github.com/rstudio/plumber/issues/463 - expect_equal(parse_body("randomcharshere==", content_type = NULL, parsers = parser_query()), list()) + expect_equal(parse_body("randomcharshere==", content_type = NULL, parsers = select_parsers("query")), list()) }) test_that("Query strings on post are handled correctly", { - expect_equivalent(parse_body("a=", parsers = parser_query()), list()) # It's technically a named list() - expect_equal(parse_body("a=1&b=&c&d=1", content_type = NULL, parser_query()), list(a="1", d="1")) + expect_equivalent(parse_body("a=", parsers = select_parsers("query")), list()) # It's technically a named list() + expect_equal(parse_body("a=1&b=&c&d=1", content_type = NULL, select_parsers("query")), list(a="1", d="1")) }) test_that("Able to handle UTF-8", { - expect_equal(parse_body('{"text":"élise"}', content_type = "application/json; charset=UTF-8", parsers = parser_json())$text, "élise") + expect_equal(parse_body('{"text":"élise"}', content_type = "application/json; charset=UTF-8", parsers = select_parsers("json"))$text, "élise") }) #charset moved to part parsing @@ -31,21 +31,21 @@ test_that("filter passes on content-type", { print(content_type) body }, - expect_output(postbody_parser(req, parser_text()), "text/html; charset=testset"), + expect_output(postbody_parser(req, select_parsers("text")), "text/html; charset=testset"), .env = "plumber" ) }) # parsers test_that("Test text parser", { - expect_equal(parse_body("Ceci est un texte.", "text/html", parser_text()), "Ceci est un texte.") + expect_equal(parse_body("Ceci est un texte.", "text/html", select_parsers("text")), "Ceci est un texte.") }) test_that("Test yaml parser", { skip_if_not_installed("yaml") r_object <- list(a=1,b=list(c=2,d=list(e=3,f=4:6))) - expect_equal(parse_body(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml", parser_yaml()), r_object) + expect_equal(parse_body(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml", select_parsers("yaml")), r_object) }) test_that("Test csv parser", { @@ -57,7 +57,7 @@ test_that("Test csv parser", { r_object <- cars write.csv(r_object, tmp, row.names = FALSE) val <- readBin(tmp, "raw", 1000) - expect_equal(parse_body(val, "application/csv", parser_csv()), r_object) + expect_equal(parse_body(val, "application/csv", select_parsers("csv")), r_object) }) test_that("Test tsv parser", { @@ -69,7 +69,7 @@ test_that("Test tsv parser", { r_object <- cars write.table(r_object, tmp, sep = "\t", row.names = FALSE) val <- readBin(tmp, "raw", 1000) - expect_equal(parse_body(val, "application/tab-separated-values", parser_tsv()), r_object) + expect_equal(parse_body(val, "application/tab-separated-values", select_parsers("tsv")), r_object) }) test_that("Test multipart parser", { @@ -79,7 +79,7 @@ test_that("Test multipart parser", { body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) parsed_body <- parse_body(body, "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ", - Reduce(utils::modifyList, list(parser_multi(), parser_json(), parser_rds(), parser_octet()))) + select_parsers(c("multi", "json", "rds", "octet"))) expect_equal(names(parsed_body), c("json", "img1", "img2", "rds")) expect_equal(parsed_body[["rds"]], women) @@ -93,6 +93,6 @@ test_that("Test multipart respect content-type", { body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) parsed_body <- parse_body(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272", - Reduce(utils::modifyList, list(parser_multi(), parser_tsv()))) + select_parsers(c("multi", "tsv"))) expect_s3_class(parsed_body$file, "data.frame") }) diff --git a/tests/testthat/test-parser.R b/tests/testthat/test-parser.R index 24de4c46c..fbcd5d5a8 100644 --- a/tests/testthat/test-parser.R +++ b/tests/testthat/test-parser.R @@ -13,7 +13,7 @@ test_that("parsers work", { expect_equal(r$routes$none$parsers, list()) expect_equal(r$routes$all$parsers, parser_all()) expect_equal(r$routes$default$parsers, NULL) - expect_equal(r$routes$json$parsers, parser_json()) - expect_equal(r$routes$mixed$parsers, Reduce(utils::modifyList, list(parser_json(), parser_query()))) - expect_equal(r$routes$repeated$parsers, parser_json()) + expect_equal(r$routes$json$parsers, select_parsers("json")) + expect_equal(r$routes$mixed$parsers, select_parsers(c("json", "query"))) + expect_equal(r$routes$repeated$parsers, select_parsers("json")) }) From 1357e253665be49ecdb74907efbcf8114441362a Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 21 Jul 2020 09:52:59 -0400 Subject: [PATCH 35/71] Make arg required --- R/parse-body.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index 1bef0478a..6700c48a9 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -199,7 +199,7 @@ list_parsers <- function() { #' @describeIn add_parser Select from global parsers and create #' a formatted parsers list for programmatic use. #' @export -select_parsers <- function(alias = character()) { +select_parsers <- function(alias) { parsers <- .globals$parsers[alias] # remove to avoid infinite recursion parsers$all <- NULL From 1b4b0e592cdd3056bfb000782eca6a72fe1b5674 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 21 Jul 2020 09:57:33 -0400 Subject: [PATCH 36/71] document --- man/add_parser.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/add_parser.Rd b/man/add_parser.Rd index a4ac40977..ad220c54a 100644 --- a/man/add_parser.Rd +++ b/man/add_parser.Rd @@ -17,7 +17,7 @@ add_parser( list_parsers() -select_parsers(alias = character()) +select_parsers(alias) } \arguments{ \item{alias}{An alias to map parser from the \verb{@parser} plumber tag to the global parsers list.} From 4023650b8e9b5cd43018019ee8d883b8e8b484f6 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 21 Jul 2020 09:57:46 -0400 Subject: [PATCH 37/71] display value when selecting --- R/parse-body.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 6700c48a9..6fcd96759 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -203,10 +203,10 @@ select_parsers <- function(alias) { parsers <- .globals$parsers[alias] # remove to avoid infinite recursion parsers$all <- NULL - return(invisible( + return( # Lambda function to get each parser `p()` list Reduce(function(l, p) {utils::modifyList(l, p())}, parsers, init = list()) - )) + ) } #' Plumber Parsers From 05d680d76cca25880eaabc1f146d37c33f142ae2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 21 Jul 2020 09:58:00 -0400 Subject: [PATCH 38/71] parser_none should ask for no alias values --- R/parse-body.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index 6fcd96759..60337a054 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -368,7 +368,8 @@ parser_all <- function() { #' @describeIn parsers No parser (For internal use only) #' @export parser_none <- function() { - select_parsers() + # do not select any parsers + select_parsers(character()) } add_parsers_onLoad <- function() { From e00fd23adca1cb8d92cb8c42e3409e20ae6ed61d Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 21 Jul 2020 12:24:48 -0400 Subject: [PATCH 39/71] have all endpoint parsers run through `select_parsers()`. Fixes #612 --- R/plumber-step.R | 2 +- R/plumber.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/plumber-step.R b/R/plumber-step.R index 9fa38c551..0e122ca00 100644 --- a/R/plumber-step.R +++ b/R/plumber-step.R @@ -199,7 +199,7 @@ PlumberEndpoint <- R6Class( self$serializer <- serializer } if (!missing(parsers) && !is.null(parsers)){ - self$parsers <- parsers + self$parsers <- select_parsers(parsers) } if (!missing(lines)){ self$lines <- lines diff --git a/R/plumber.R b/R/plumber.R index 7e108abb1..c793ba279 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -221,7 +221,7 @@ plumber <- R6Class( # Initialize private$serializer <- serializer_json() # Default parsers to maintain legacy features - private$parsers <- select_parsers(c("json", "query", "text", "octet", "multi")) + private$parsers <- c("json", "query", "text", "octet", "multi") private$errorHandler <- defaultErrorHandler() private$notFoundHandler <- default404Handler private$maxSize <- getOption('plumber.maxRequestSize', 0) #0 Unlimited From 92362418cc8c0b285176f40c6f4bbd276d18aab6 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Tue, 21 Jul 2020 13:06:02 -0500 Subject: [PATCH 40/71] Nitpick --- R/parse-body.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 60337a054..3cfd0daea 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -33,13 +33,13 @@ parse_raw <- function(toparse) { tolower(toparse$content_type), toparse$value[1], toparse$filename, - toparse$parsers) - if (!is.null(parser)) { - return(do.call(parser, toparse)) - } else { + toparse$parsers + ) + if (is.null(parser)) { message("No suitable parser found to handle request body type ", toparse$content_type, ".") return(list()) } + do.call(parser, toparse) } parser_picker <- function(content_type, first_byte, filename = NULL, parsers = NULL) { From d178b97d1af7a2f16947445748ae94d37383592d Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 22 Jul 2020 13:46:23 -0400 Subject: [PATCH 41/71] Apply suggestions from code review Co-authored-by: Carson Sievert --- R/parse-body.R | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 3cfd0daea..9fdaccacd 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -72,12 +72,12 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N fpm <- stri_detect_regex( content_type, names(parsers$regex), - max_count = 1) - fpm[is.na(fpm)] <- FALSE + max_count = 1 + ) # return known parser (first regex pattern match) if (any(fpm)) { - return(parsers$regex[[which(fpm)]]) + return(parsers$regex[[which(fpm)[1]]]) } # query string @@ -199,7 +199,7 @@ list_parsers <- function() { #' @describeIn add_parser Select from global parsers and create #' a formatted parsers list for programmatic use. #' @export -select_parsers <- function(alias) { +get_parsers <- function(aliases) { parsers <- .globals$parsers[alias] # remove to avoid infinite recursion parsers$all <- NULL @@ -270,10 +270,10 @@ parser_text <- function(parse_fn = identity) { #' @describeIn parsers YAML parser #' @export parser_yaml <- function(...) { + if (!requireNamespace("yaml", quietly = TRUE)) { + stop("yaml must be installed for the yaml parser to work") + } parser_text(function(val) { - if (!requireNamespace("yaml", quietly = TRUE)) { - stop("yaml must be installed for the yaml parser to work") - } yaml::yaml.load(val, ..., eval.expr = FALSE) }) } @@ -297,8 +297,8 @@ parser_read_file <- function(read_fn = readLines) { #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { - parser_read_file(function(val) { - utils::read.csv(val, ...) + parser_read_file(function(tmpfile) { + utils::read.csv(tmpfile, ...) }) } @@ -306,8 +306,8 @@ parser_csv <- function(...) { #' @describeIn parsers TSV parser #' @export parser_tsv <- function(...) { - parser_read_file(function(val) { - utils::read.delim(val, ...) + parser_read_file(function(tmpfile) { + utils::read.delim(tmpfile, ...) }) } @@ -315,8 +315,8 @@ parser_tsv <- function(...) { #' @describeIn parsers RDS parser #' @export parser_rds <- function(...) { - parser_read_file(function(value) { - readRDS(value, ...) + parser_read_file(function(tmpfile) { + readRDS(tmpfile, ...) }) } @@ -376,7 +376,7 @@ add_parsers_onLoad <- function() { # parser alias names for plumbing add_parser("csv", parser_csv, fixed = c("application/csv", "application/x-csv", "text/csv", "text/x-csv")) - add_parser("json", parser_json, fixed = c("application/json", "text/json"), regex = "json$", shortname = "json") + add_parser("json", parser_json, fixed = c("application/json", "text/json"), shortname = "json") add_parser("multi", parser_multi, fixed = "multipart/form-data") add_parser("octet", parser_octet, fixed = "application/octet-stream", shortname = "octet") add_parser("query", parser_query, fixed = "application/x-www-form-urlencoded", shortname = "query") From 1ae78d4771587ecb1dd2704dd41cf55126f7ed39 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Thu, 23 Jul 2020 12:31:43 -0400 Subject: [PATCH 42/71] Rename pr private$parsers to private$default_parsers --- R/plumber.R | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/R/plumber.R b/R/plumber.R index c8c13d602..e609c6059 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -217,7 +217,7 @@ plumber <- R6Class( # Initialize private$serializer <- serializer_json() # Default parsers to maintain legacy features - private$parsers <- c("json", "query", "text", "octet", "multi") + private$default_parsers <- c("json", "query", "text", "octet", "multi") private$errorHandler <- defaultErrorHandler() private$notFoundHandler <- default404Handler private$maxSize <- getOption('plumber.maxRequestSize', 0) #0 Unlimited @@ -487,17 +487,17 @@ plumber <- R6Class( #' "

Programmatic Plumber!

" #' }, serializer=plumber::serializer_html()) #' } - handle = function(methods, path, handler, preempt, serializer, parsers, endpoint, ...){ + handle = function(methods, path, handler, preempt, serializer, parsers, endpoint, ...) { epdef <- !missing(methods) || !missing(path) || !missing(handler) || !missing(serializer) || !missing(parsers) if (!missing(endpoint) && epdef){ stop("You must provide either the components for an endpoint (handler and serializer) OR provide the endpoint yourself. You cannot do both.") } - if (epdef){ - if (missing(serializer)){ + if (epdef) { + if (missing(serializer)) { serializer <- private$serializer } - if (missing(parsers)){ + if (missing(parsers)) { parsers <- private$parsers } @@ -893,8 +893,8 @@ plumber <- R6Class( }, #' @details Sets the default parsers of the router. #' @param parsers a named list of parsers - setParsers = function(parsers){ - private$parsers <- parsers + setParsers = function(parsers) { + private$default_parsers <- parsers }, #' @details Sets the handler that gets called if an #' incoming request can’t be served by any filter, endpoint, or sub-router. @@ -1072,7 +1072,7 @@ plumber <- R6Class( } ), private = list( serializer = NULL, # The default serializer for the router - parsers = NULL, # The default parsers for the router + default_parsers = NULL, # The default parsers for the router ends = list(), # List of endpoints indexed by their pre-empted filter. filts = NULL, # Array of filters From 12fb373649a8df67293c2da9cffa6594168fa09b Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Thu, 23 Jul 2020 12:33:18 -0400 Subject: [PATCH 43/71] Rename add_parser to register_parser. Allow for TRUE, character vect, and named list of parser to arguments --- R/parse-body.R | 72 ++++++++++++----------- man/{add_parser.Rd => register_parser.Rd} | 37 ++++++------ 2 files changed, 54 insertions(+), 55 deletions(-) rename man/{add_parser.Rd => register_parser.Rd} (72%) diff --git a/R/parse-body.R b/R/parse-body.R index 9fdaccacd..2ba081cd2 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -100,7 +100,6 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' @param parser The parser function to be added. This build the parser function. #' @param fixed A character vector of fixed string to be matched against a request `content-type` to use `parser`. #' @param regex A character vector of [regex] string to be matched against a request `content-type` to use `parser`. -#' @param shortname A character value to reference a parser by a shortname. (For internal use only) #' @param verbose Logical value which determines if a warning should be #' displayed when alias in map are overwritten. #' @@ -136,9 +135,15 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' read.dcf(value) #' } #' } -#' add_parser("dcf", parser_dcf, fixed = "text/x-dcf") +#' register_parser("dcf", parser_dcf, fixed = "text/x-dcf") #' @export -add_parser <- function(alias, parser, fixed = NULL, regex = NULL, shortname = NULL, verbose = TRUE) { +register_parser <- function( + alias, + parser, + fixed = NULL, + regex = NULL, + verbose = TRUE +) { if (!is.null(.globals$parsers[[alias]])) { if (isTRUE(verbose)) { @@ -148,46 +153,43 @@ add_parser <- function(alias, parser, fixed = NULL, regex = NULL, shortname = NU stopifnot(is.function(parser)) - if (length(c(fixed, regex, shortname))) { - - parsers_list <- function(...) { - - parser_function <- do.call(parser, list(...)) - - create_list <- function(names) { - stats::setNames( - replicate( - length(names), - parser_function), - names - ) - } - - parsers <- list() - - if (length(shortname) > 0) { - parsers[[shortname]] <- parser_function - } - if (length(fixed) > 0) { - parsers$fixed <- create_list(fixed) - } - if (length(regex) > 0) { - parsers$regex <- create_list(regex) - } + if (length(c(fixed, regex)) == 0) { + stop("At least one value of `fixed` and `regex` is required to register a parser") + } - return(parsers) + # Init the parser function with outside arguments + init_parser_func <- function(...) { + parser_function <- do.call(parser, list(...)) + parser_formals <- formals(parser_function) + if (!("..." %in% names(parser_formals))) { + stop("For parser '", alias, "', please add a `...` argument to the returned function for possible future parameter expansion") } - } else { + create_list <- function(names) { + stats::setNames( + replicate( + length(names), + parser_function), + names + ) + } - parsers_list = parser + parser_info <- list( + alias = create_list(alias) + ) + if (length(fixed) > 0) { + parser_info$fixed <- create_list(fixed) + } + if (length(regex) > 0) { + parser_info$regex <- create_list(regex) + } + return(parser_info) } - .globals$parsers[[alias]] <- parsers_list - - invisible(list_parsers()) + .globals$parsers[[alias]] <- init_parser_func + invisible(.globals$parsers) } #' @export diff --git a/man/add_parser.Rd b/man/register_parser.Rd similarity index 72% rename from man/add_parser.Rd rename to man/register_parser.Rd index c576b4a01..8073fb504 100644 --- a/man/add_parser.Rd +++ b/man/register_parser.Rd @@ -1,23 +1,16 @@ % Generated by roxygen2: do not edit by hand % Please edit documentation in R/parse-body.R -\name{add_parser} -\alias{add_parser} -\alias{list_parsers} -\alias{get_parsers} +\name{register_parser} +\alias{register_parser} +\alias{registered_parsers} +\alias{combine_parsers} \title{Manage parsers} \usage{ -add_parser( - alias, - parser, - fixed = NULL, - regex = NULL, - shortname = NULL, - verbose = TRUE -) +register_parser(alias, parser, fixed = NULL, regex = NULL, verbose = TRUE) -list_parsers() +registered_parsers() -get_parsers(aliases) +combine_parsers(aliases) } \arguments{ \item{alias}{An alias to map parser from the \verb{@parser} plumber tag to the global parsers list.} @@ -28,10 +21,15 @@ get_parsers(aliases) \item{regex}{A character vector of \link{regex} string to be matched against a request \code{content-type} to use \code{parser}.} -\item{shortname}{A character value to reference a parser by a shortname. (For internal use only)} - \item{verbose}{Logical value which determines if a warning should be displayed when alias in map are overwritten.} + +\item{aliases}{Can be one of: +\itemize{ +\item A character vector of \code{alias} names. +\item A named \code{list()} whose keysare \code{alias} names and values are arguments to be applied with \code{\link[=do.call]{do.call()}} +\item A \code{TRUE} value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes. +}} } \description{ A parser is responsible for decoding the raw body content of a request into @@ -60,10 +58,9 @@ Parser function structure is something like below.\if{html}{\out{
} \section{Functions}{ \itemize{ -\item \code{list_parsers}: List currently registered parsers +\item \code{registered_parsers}: Return all registered parsers -\item \code{get_parsers}: Select from global parsers and create -a formatted parsers list for programmatic use. +\item \code{combine_parsers}: Select from global parsers and create a combined parsers list for programmatic use. }} \examples{ @@ -76,5 +73,5 @@ parser_dcf <- function() { read.dcf(value) } } -add_parser("dcf", parser_dcf, fixed = "text/x-dcf") +register_parser("dcf", parser_dcf, fixed = "text/x-dcf") } From 2b950d2871dbdf828162f306c7d7e6fe26ca2646 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 10:20:03 -0400 Subject: [PATCH 44/71] `get_parsers()` -> `make_parser()`; `add_parser()` -> `register_parser()` Allow for many different types of inputs. Which allows for arguments for each particular parser to be passed through. --- R/json.R | 4 +- R/parse-body.R | 169 ++++++++++++++++++++++++++++++++++++------------- 2 files changed, 127 insertions(+), 46 deletions(-) diff --git a/R/json.R b/R/json.R index 09c256825..573386b22 100644 --- a/R/json.R +++ b/R/json.R @@ -1,5 +1,5 @@ #' @importFrom jsonlite parse_json #' @noRd -safeFromJSON <- function(txt) { - parse_json(txt, simplifyVector = TRUE) +safeFromJSON <- function(txt, simplifyVector = TRUE, ...) { + parse_json(txt, simplifyVector = simplifyVector, ...) } diff --git a/R/parse-body.R b/R/parse-body.R index 2ba081cd2..6695f694e 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -48,10 +48,10 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N if (length(content_type) == 0) { # fast default to json when first byte is 7b (ascii {) if (first_byte == as.raw(123L)) { - return(parsers$json) + return(parsers$alias$json) } - return(parsers$query) + return(parsers$alias$query) } # remove trailing content type information @@ -82,11 +82,11 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N # query string if (is.null(filename)) { - return(parsers$query) + return(parsers$alias$query) } # octet - parsers$octet + parsers$alias$octet } @@ -97,7 +97,7 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' For instance, [parser_json()] parse content-type `application/json`. #' #' @param alias An alias to map parser from the `@parser` plumber tag to the global parsers list. -#' @param parser The parser function to be added. This build the parser function. +#' @param parser The parser function to be added. This build the parser function. See Details for more information. #' @param fixed A character vector of fixed string to be matched against a request `content-type` to use `parser`. #' @param regex A character vector of [regex] string to be matched against a request `content-type` to use `parser`. #' @param verbose Logical value which determines if a warning should be @@ -192,23 +192,87 @@ register_parser <- function( invisible(.globals$parsers) } +#' @describeIn register_parser Return all registered parsers #' @export -#' @describeIn add_parser List currently registered parsers -list_parsers <- function() { - .globals$parsers +registered_parsers <- function() { + names(.globals$parsers) } -#' @describeIn add_parser Select from global parsers and create -#' a formatted parsers list for programmatic use. +#' @describeIn register_parser Select from global parsers and create a combined parser list for programmatic use. +#' @param aliases Can be one of: +#' * A character vector of `alias` names. +#' * A named `list()` whose keysare `alias` names and values are arguments to be applied with [do.call()] +#' * A `TRUE` value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes. +#' * Already combined parsers. (Will be returned immediately.) #' @export -get_parsers <- function(aliases) { - parsers <- .globals$parsers[alias] - # remove to avoid infinite recursion - parsers$all <- NULL - return( - # Lambda function to get each parser `p()` list - Reduce(function(l, p) {utils::modifyList(l, p())}, parsers, init = list()) - ) +make_parser <- function(aliases) { + if (inherits(aliases, "plumber_parsed_parsers")) { + return(aliases) + } + if (isTRUE(aliases)) { + # use all available parsers except ("none") + aliases <- setdiff(registered_parsers(), c("all", "none")) + } + if (is.character(aliases)) { + if (any(is.na(aliases))) { + stop("aliases can not be `NA` values") + } + # turn aliases into a named list with empty values + aliases <- setNames( + replicate(length(aliases), {list()}), + aliases + ) + } + + stopifnot(is.list(aliases)) + if (is.null(names(aliases))) { + stop("aliases must be able to convert to a named list") + } + + local({ + aliases_not_found <- !(names(aliases) %in% registered_parsers()) + if (any(aliases_not_found)) { + missing_aliases <- names(aliases)[aliases_not_found] + stop("Aliases not available: ", paste0(missing_aliases, collapse = ", "), ". See: registered_parsers()") + } + }) + + # if "all" is found, add all remaining registered parsers (except 'none') to the `aliases` list + if ("all" %in% names(aliases)) { + all_parser_names <- setdiff(registered_parsers(), c("all", "none")) + # remove to avoid infinite recursion + aliases$all <- NULL + names_to_add <- setdiff(all_parser_names, names(aliases)) + if (length(names_to_add)) { + aliases[names_to_add] <- replicate(length(names_to_add), list()) + } + } + + + # convert parser functions into initialized information + parser_infos <- + lapply( + names(aliases), + function(alias) { + # get init function + init_parser_func <- .globals$parsers[[alias]] + # call outer parser function to init the params for inner function + do.call(init_parser_func, aliases[[alias]]) + } + ) + + # combine information into a single list + combined_parser_info <- + Reduce( + function(cur_parser_info, parser_info) { + utils::modifyList(cur_parser_info, parser_info) + }, + parser_infos, + init = list() + ) + + class(combined_parser_info) <- c("plumber_parsed_parsers", class(combined_parser_info)) + combined_parser_info } #' Plumber Parsers @@ -249,8 +313,8 @@ parser_query <- function() { #' @describeIn parsers JSON parser #' @export parser_json <- function(...) { - parser_text(function(value) { - safeFromJSON(value, ...) + parser_text(function(txt_value) { + safeFromJSON(txt_value, ...) }) } @@ -262,9 +326,9 @@ parser_text <- function(parse_fn = identity) { stopifnot(is.function(parse_fn)) function(value, content_type = NULL, ...) { charset <- getCharacterSet(content_type) - value <- rawToChar(value) - Encoding(value) <- charset - parse_fn(value) + txt_value <- rawToChar(value) + Encoding(txt_value) <- charset + parse_fn(txt_value) } } @@ -299,8 +363,11 @@ parser_read_file <- function(read_fn = readLines) { #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { + if (!requireNamespace("readr", quietly = TRUE)) { + stop("`readr` must be installed for `parser_csv` to work") + } parser_read_file(function(tmpfile) { - utils::read.csv(tmpfile, ...) + readr::read_csv(tmpfile, ...) }) } @@ -308,8 +375,11 @@ parser_csv <- function(...) { #' @describeIn parsers TSV parser #' @export parser_tsv <- function(...) { + if (!requireNamespace("readr", quietly = TRUE)) { + stop("`readr` must be installed for `parser_tsv` to work") + } parser_read_file(function(tmpfile) { - utils::read.delim(tmpfile, ...) + readr::read_tsv(tmpfile, ...) }) } @@ -323,7 +393,7 @@ parser_rds <- function(...) { } -#' @describeIn parsers Octet stream parser +#' @describeIn parsers Octet stream parser. Will add a filename attribute if the filename exists #' @export parser_octet <- function() { function(value, filename = NULL, ...) { @@ -361,31 +431,42 @@ parser_multi <- function() { } } -#' @describeIn parsers All parsers (For internal use only) +#' @describeIn parsers Enable all parsers. Not recommended due to security concerns. #' @export parser_all <- function() { - select_parsers(names(.globals$parsers)) + function(value, content_type, filename, ...) { + # re-perform parse_raw(), but provide all parsers + parse_raw( + # mimic the shape of the output of `webutils::parse_multipart` + parsers + list( + content_type = content_type, + value = value, + filename = filename, + parsers = .globals$parsers + ) + ) + } } -#' @describeIn parsers No parser (For internal use only) +#' @describeIn parsers No parser. Will not process the postBody. #' @export parser_none <- function() { - # do not select any parsers - select_parsers(character()) + function(value, ...) { + value + } } -add_parsers_onLoad <- function() { - +register_parsers_onLoad <- function() { # parser alias names for plumbing - add_parser("csv", parser_csv, fixed = c("application/csv", "application/x-csv", "text/csv", "text/x-csv")) - add_parser("json", parser_json, fixed = c("application/json", "text/json"), shortname = "json") - add_parser("multi", parser_multi, fixed = "multipart/form-data") - add_parser("octet", parser_octet, fixed = "application/octet-stream", shortname = "octet") - add_parser("query", parser_query, fixed = "application/x-www-form-urlencoded", shortname = "query") - add_parser("rds", parser_rds, fixed = "application/rds") - add_parser("text", parser_text, fixed = "text/plain", regex = "^text/", shortname = "text") - add_parser("tsv", parser_tsv, fixed = c("application/tab-separated-values", "text/tab-separated-values")) - add_parser("yaml", parser_yaml, fixed = c("application/yaml", "application/x-yaml", "text/yaml", "text/x-yaml")) - add_parser("all", parser_all) - add_parser("none", parser_none) + register_parser("csv", parser_csv, fixed = c("application/csv", "application/x-csv", "text/csv", "text/x-csv")) + register_parser("json", parser_json, fixed = c("application/json", "text/json")) + register_parser("multi", parser_multi, fixed = "multipart/form-data") + register_parser("octet", parser_octet, fixed = "application/octet-stream") + register_parser("query", parser_query, fixed = "application/x-www-form-urlencoded") + register_parser("rds", parser_rds, fixed = "application/rds") + register_parser("text", parser_text, fixed = "text/plain", regex = "^text/") + register_parser("tsv", parser_tsv, fixed = c("application/tab-separated-values", "text/tab-separated-values")) + register_parser("yaml", parser_yaml, fixed = c("application/yaml", "application/x-yaml", "text/yaml", "text/x-yaml")) + register_parser("all", parser_all, regex = "*") + register_parser("none", parser_none, regex = "*") } From 7f0991574742474df59153dbeb0d78a803501c72 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 10:21:05 -0400 Subject: [PATCH 45/71] plumb block should set up a named list of parser to arguments structure to later be processed by plumber endpoint --- R/plumb-block.R | 38 ++++++++++++++++++++------------------ R/plumber-step.R | 6 +++--- R/plumber.R | 11 ++++++----- R/zzz.R | 2 +- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/R/plumb-block.R b/R/plumb-block.R index 6c04beb20..7d21bdd66 100644 --- a/R/plumb-block.R +++ b/R/plumb-block.R @@ -159,30 +159,20 @@ plumbBlock <- function(lineNum, file){ stopOnLine(lineNum, line, "No @parser specified") } - if (!parser_alias %in% names(.globals$parsers)){ + if (!parser_alias %in% registered_parsers()){ stopOnLine(lineNum, line, paste0("No such @parser registered: ", parser_alias)) } - parser <- .globals$parsers[[parser_alias]] - if (!is.na(parsersMat[1, 4]) && parsersMat[1,4] != ""){ # We have an arg to pass in to the parser - argList <- eval(parse(text=parsersMat[1,4])) + arg_list <- eval(parse(text=parsersMat[1,4])) } else { - argList <- list() + arg_list <- list() } - tryCatch({ - # Use modifyList instead of c to avoid duplicated parsers name - if (is.null(parsers)) { - parsers <- do.call(parser, argList) - } else { - # Since we plumb from bottom to top, put currently plumbed parsers in front - # Parsers will be added in the order the appear in the plumbed file - parsers <- utils::modifyList(do.call(parser, argList), parsers) - } - }, error = function(e) { - stopOnLine(lineNum, line, paste0("Error creating parser: ", parser_alias, "\n", e)) - }) + if (is.null(parsers)) { + parsers <- list() + } + parsers[[parser_alias]] <- arg_list } @@ -283,7 +273,19 @@ evaluateBlock <- function(srcref, file, expr, envir, addEndpoint, addFilter, pr) # ALL if statements possibilities must eventually call eval(expr, envir) if (!is.null(block$paths)){ lapply(block$paths, function(p){ - ep <- PlumberEndpoint$new(p$verb, p$path, expr, envir, block$serializer, block$parsers, srcref, block$params, block$comments, block$responses, block$tags) + ep <- PlumberEndpoint$new( + verbs = p$verb, + path = p$path, + expr = expr, + envir = envir, + serializer = block$serializer, + parsers = block$parsers, + lines = srcref, + params = block$params, + comments = block$comments, + responses = block$responses, + tags = block$tags + ) if (!is.null(block$image)){ # Arguments to pass in to the image serializer diff --git a/R/plumber-step.R b/R/plumber-step.R index ea5ba14e5..b8ae91dcf 100644 --- a/R/plumber-step.R +++ b/R/plumber-step.R @@ -178,7 +178,7 @@ PlumberEndpoint <- R6Class( #' @param expr endpoint expr #' @param envir endpoint environment #' @param serializer endpoint serializer - #' @param parsers endpoint parsers + #' @param parsers endpoint parsers. If provided, the value will be sent processed by [make_parser()] #' @param lines endpoint block #' @param params endpoint params #' @param comments endpoint comments @@ -204,8 +204,8 @@ PlumberEndpoint <- R6Class( if (!missing(serializer) && !is.null(serializer)){ self$serializer <- serializer } - if (!missing(parsers) && !is.null(parsers)){ - self$parsers <- select_parsers(parsers) + if (!missing(parsers) && !is.null(parsers)) { + self$parsers <- make_parser(parsers) } if (!missing(lines)){ self$lines <- lines diff --git a/R/plumber.R b/R/plumber.R index e609c6059..937d8665d 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -739,11 +739,12 @@ plumber <- R6Class( if (!is.null(h$serializer)) { res$serializer <- h$serializer } - if (!is.null(h$parsers)) { - parsers <- h$parsers - } else { - parsers <- private$parsers - } + parsers <- + if (!is.null(h$parsers)) { + h$parsers + } else { + make_parser(private$default_parsers) + } req$args <- c( h$getPathParams(path), req$args, diff --git a/R/zzz.R b/R/zzz.R index 58ab85074..27a905dc5 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -3,7 +3,7 @@ addApiInfo_onLoad() - add_parsers_onLoad() + register_parsers_onLoad() add_serializers_onLoad() From 779b18158fd4cd56578afe944997195312c9b345 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 10:21:45 -0400 Subject: [PATCH 46/71] Update and add more tests for parsers --- tests/testthat/files/parsers.R | 50 +++++++++---------------------- tests/testthat/test-parse-block.R | 28 ++++++++++++----- tests/testthat/test-parse-body.R | 36 ++++++++++++++-------- tests/testthat/test-parser.R | 50 +++++++++++++++++++++++++++---- 4 files changed, 102 insertions(+), 62 deletions(-) diff --git a/tests/testthat/files/parsers.R b/tests/testthat/files/parsers.R index 6856692c5..003af7d02 100644 --- a/tests/testthat/files/parsers.R +++ b/tests/testthat/files/parsers.R @@ -1,54 +1,32 @@ -#* @post /none -#* @parser none -function(...){ +return_inputs <- function(...) { ret <- list(...) ret$req <- NULL ret$res <- NULL ret } -#* @post /all -#* @parser all -function(...){ - ret <- list(...) - ret$req <- NULL - ret$res <- NULL - ret -} #* @post /default -function(...){ - ret <- list(...) - ret$req <- NULL - ret$res <- NULL - ret -} +return_inputs #* @post /json #* @parser json -function(...){ - ret <- list(...) - ret$req <- NULL - ret$res <- NULL - ret -} +return_inputs #* @post /mixed -#* @parser json #* @parser query -function(...){ - ret <- list(...) - ret$req <- NULL - ret$res <- NULL - ret -} +#* @parser json +return_inputs #* @post /repeated #* @parser json #* @parser json -function(...){ - ret <- list(...) - ret$req <- NULL - ret$res <- NULL - ret -} +return_inputs + +#* @post /none +#* @parser none +return_inputs + +#* @post /all +#* @parser all +return_inputs diff --git a/tests/testthat/test-parse-block.R b/tests/testthat/test-parse-block.R index a97f20d1a..1c1de52c0 100644 --- a/tests/testthat/test-parse-block.R +++ b/tests/testthat/test-parse-block.R @@ -164,9 +164,9 @@ test_that("@parser parameters produce an error or not", { # due to covr changing some code, the return answer is very strange testthat::skip_on_covr() - expect_block_fn <- function(lines, fn) { - b <- plumber:::plumbBlock(length(lines), lines) - expect_equal_functions(b$parsers, fn) + expect_block_parser <- function(lines, fn) { + b <- plumbBlock(length(lines), lines) + expect_equal(b$parsers, fn) } expect_block_error <- function(lines, ...) { expect_error({ @@ -174,13 +174,25 @@ test_that("@parser parameters produce an error or not", { }, ...) } - expect_block_fn("#' @parser octet", select_parsers("octet")) - expect_block_fn("#' @parser octet list()", select_parsers("octet")) - expect_block_fn("#' @parser octet list( )", select_parsers("octet")) - expect_block_fn("#' @parser octet list ( ) ", select_parsers("octet")) + expected <- list(octet = list()) + expect_block_parser("#' @parser octet", expected) - expect_block_error("#' @parser octet list(key = \"val\")", "unused argument") + expect_block_parser("#' @parser octet list()", expected) + expect_block_parser("#' @parser octet list( )", expected) + expect_block_parser("#' @parser octet list ( ) ", expected) + + expect_error({ + evaluateBlock( + srcref = 3, # which evaluates to line 2 + file = c("#' @get /test", "#' @parser octet list(key = \"val\")"), + expr = substitute(identity), + envir = new.env(), + addEndpoint = function(a, b, ...) { str(list(a , b, ...)); browser()}, + addFilter = as.null, + pr = plumber$new() + ) + }, "unused argument (key = \"val\")", fixed = TRUE) }) # TODO: more testing around filter, assets, endpoint, etc. diff --git a/tests/testthat/test-parse-body.R b/tests/testthat/test-parse-body.R index 56720ad55..1742ebb0f 100644 --- a/tests/testthat/test-parse-body.R +++ b/tests/testthat/test-parse-body.R @@ -1,21 +1,21 @@ context("POST body") test_that("JSON is consumed on POST", { - expect_equal(parse_body('{"a":"1"}', content_type = NULL, parsers = select_parsers("json")), list(a = "1")) + expect_equal(parse_body('{"a":"1"}', content_type = NULL, parsers = make_parser("json")), list(a = "1")) }) test_that("ending in `==` does not produce a unexpected key", { # See https://github.com/rstudio/plumber/issues/463 - expect_equal(parse_body("randomcharshere==", content_type = NULL, parsers = select_parsers("query")), list()) + expect_equal(parse_body("randomcharshere==", content_type = NULL, parsers = make_parser("query")), list()) }) test_that("Query strings on post are handled correctly", { - expect_equivalent(parse_body("a=", parsers = select_parsers("query")), list()) # It's technically a named list() - expect_equal(parse_body("a=1&b=&c&d=1", content_type = NULL, select_parsers("query")), list(a="1", d="1")) + expect_equivalent(parse_body("a=", parsers = make_parser("query")), list()) # It's technically a named list() + expect_equal(parse_body("a=1&b=&c&d=1", content_type = NULL, make_parser("query")), list(a="1", d="1")) }) test_that("Able to handle UTF-8", { - expect_equal(parse_body('{"text":"élise"}', content_type = "application/json; charset=UTF-8", parsers = select_parsers("json"))$text, "élise") + expect_equal(parse_body('{"text":"élise"}', content_type = "application/json; charset=UTF-8", parsers = make_parser("json"))$text, "élise") }) #charset moved to part parsing @@ -31,21 +31,21 @@ test_that("filter passes on content-type", { print(content_type) body }, - expect_output(postbody_parser(req, select_parsers("text")), "text/html; charset=testset"), + expect_output(postbody_parser(req, make_parser("text")), "text/html; charset=testset"), .env = "plumber" ) }) # parsers test_that("Test text parser", { - expect_equal(parse_body("Ceci est un texte.", "text/html", select_parsers("text")), "Ceci est un texte.") + expect_equal(parse_body("Ceci est un texte.", "text/html", make_parser("text")), "Ceci est un texte.") }) test_that("Test yaml parser", { skip_if_not_installed("yaml") r_object <- list(a=1,b=list(c=2,d=list(e=3,f=4:6))) - expect_equal(parse_body(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml", select_parsers("yaml")), r_object) + expect_equal(parse_body(charToRaw(yaml::as.yaml(r_object)), "application/x-yaml", make_parser("yaml")), r_object) }) test_that("Test csv parser", { @@ -57,7 +57,13 @@ test_that("Test csv parser", { r_object <- cars write.csv(r_object, tmp, row.names = FALSE) val <- readBin(tmp, "raw", 1000) - expect_equal(parse_body(val, "application/csv", select_parsers("csv")), r_object) + + parsed <- parse_body(val, "application/csv", make_parser("csv")) + # convert from readr tibble to data.frame + parsed <- as.data.frame(parsed, stringsAsFactors = FALSE) + attr(parsed, "spec") <- NULL + + expect_equal(parsed, r_object) }) test_that("Test tsv parser", { @@ -69,7 +75,13 @@ test_that("Test tsv parser", { r_object <- cars write.table(r_object, tmp, sep = "\t", row.names = FALSE) val <- readBin(tmp, "raw", 1000) - expect_equal(parse_body(val, "application/tab-separated-values", select_parsers("tsv")), r_object) + + parsed <- parse_body(val, "application/tab-separated-values", make_parser("tsv")) + # convert from readr tibble to data.frame + parsed <- as.data.frame(parsed, stringsAsFactors = FALSE) + attr(parsed, "spec") <- NULL + + expect_equal(parsed, r_object) }) test_that("Test multipart parser", { @@ -79,7 +91,7 @@ test_that("Test multipart parser", { body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) parsed_body <- parse_body(body, "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ", - select_parsers(c("multi", "json", "rds", "octet"))) + make_parser(c("multi", "json", "rds", "octet"))) expect_equal(names(parsed_body), c("json", "img1", "img2", "rds")) expect_equal(parsed_body[["rds"]], women) @@ -93,6 +105,6 @@ test_that("Test multipart respect content-type", { body <- readBin(bin_file, what = "raw", n = file.info(bin_file)$size) parsed_body <- parse_body(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272", - select_parsers(c("multi", "tsv"))) + make_parser(c("multi", "tsv"))) expect_s3_class(parsed_body$file, "data.frame") }) diff --git a/tests/testthat/test-parser.R b/tests/testthat/test-parser.R index fbcd5d5a8..c5bbcf8de 100644 --- a/tests/testthat/test-parser.R +++ b/tests/testthat/test-parser.R @@ -1,19 +1,57 @@ context("Parsers tag") +test_that("parsers can be combined", { + + expect_parsers <- function(names, target_names, sort_items = TRUE) { + aliases <- names(make_parser(names)$alias) + if (sort_items) { + aliases <- sort(aliases) + target_names <- sort(target_names) + } + expect_equal(aliases, target_names) + } + + expect_parsers("json", "json") + + expect_parsers(c("query", "json"), c("query", "json"), sort_items = FALSE) + + expect_parsers("all", setdiff(registered_parsers(), c("all", "none"))) + expect_parsers(TRUE, setdiff(registered_parsers(), c("all", "none"))) + expect_parsers(list(all = list()), setdiff(registered_parsers(), c("all", "none"))) + + # make sure parameters are not overwritten even when including all + parsers_plain <- make_parser(list(all = list(), json = list(simplifyVector = FALSE))) + expect_equal( + parsers_plain$alias$json(jsonlite::toJSON(1:3) %>% charToRaw()), + list(1,2,3) + ) + + parsers_guess <- make_parser(list(all = list(), json = list(simplifyVector = TRUE))) + expect_equal( + parsers_guess$alias$json(jsonlite::toJSON(1:3) %>% charToRaw()), + c(1,2,3) + ) + + # check that parsers return already combined parsers + expect_parsers(parsers_plain, setdiff(registered_parsers(), c("all", "none"))) +}) + test_that("parsers work", { r <- plumber$new(test_path("files/parsers.R")) res <- PlumberResponse$new() + + expect_identical(r$route(make_req("POST", "/default", body='{"a":1}'), res), structure(list(1L), names = "a")) expect_identical(r$route(make_req("POST", "/none", body='{"a":1}'), res), structure(list(), names = character())) expect_identical(r$route(make_req("POST", "/all", body='{"a":1}'), res), structure(list(1L), names = "a")) - expect_identical(r$route(make_req("POST", "/default", body='{"a":1}'), res), structure(list(1L), names = "a")) bin_file <- test_path("files/multipart-ctype.bin") bin_body <- readBin(bin_file, "raw", file.info(bin_file)$size) expect_identical(r$route(make_req("POST", "/none", body=rawToChar(bin_body)), res), structure(list(), names = character())) expect_message(r$route(make_req("POST", "/json", body=rawToChar(bin_body)), res), "No suitable parser found") - expect_equal(r$routes$none$parsers, list()) - expect_equal(r$routes$all$parsers, parser_all()) + + expect_equal(r$routes$none$parsers, make_parser("none")) + expect_equal(r$routes$all$parsers, make_parser("all")) expect_equal(r$routes$default$parsers, NULL) - expect_equal(r$routes$json$parsers, select_parsers("json")) - expect_equal(r$routes$mixed$parsers, select_parsers(c("json", "query"))) - expect_equal(r$routes$repeated$parsers, select_parsers("json")) + expect_equal(r$routes$json$parsers, make_parser("json")) + expect_equal(r$routes$mixed$parsers, make_parser(c("json", "query"))) + expect_equal(r$routes$repeated$parsers, make_parser("json")) }) From 194fd74fca44430aca304fb811434505f84e7e51 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 10:23:14 -0400 Subject: [PATCH 47/71] Fix checks and document --- NAMESPACE | 6 +++--- R/parse-body.R | 4 ++-- man/PlumberEndpoint.Rd | 2 +- man/parsers.Rd | 8 ++++---- man/register_parser.Rd | 9 +++++---- tests/testthat/test-async.R | 2 +- tests/testthat/test-querystring.R | 2 +- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index cb4b1417f..fd6c7ac21 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -4,7 +4,6 @@ export("%>%") export(PlumberEndpoint) export(PlumberStatic) export(addSerializer) -export(add_parser) export(as_attachment) export(do_configure_https) export(do_deploy_api) @@ -14,12 +13,11 @@ export(do_remove_api) export(do_remove_forward) export(forward) export(getCharacterSet) -export(get_parsers) export(include_file) export(include_html) export(include_md) export(include_rmd) -export(list_parsers) +export(make_parser) export(options_plumber) export(parser_all) export(parser_csv) @@ -52,6 +50,8 @@ export(pr_set_404) export(pr_set_error) export(pr_set_serializer) export(randomCookieKey) +export(register_parser) +export(registered_parsers) export(serializer_cat) export(serializer_content_type) export(serializer_csv) diff --git a/R/parse-body.R b/R/parse-body.R index 6695f694e..4b6b21ac0 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -218,7 +218,7 @@ make_parser <- function(aliases) { stop("aliases can not be `NA` values") } # turn aliases into a named list with empty values - aliases <- setNames( + aliases <- stats::setNames( replicate(length(aliases), {list()}), aliases ) @@ -290,7 +290,7 @@ make_parser <- function(aliases) { #' User should be aware that `rds` parsing should only be done from a #' trusted source. Do not accept `rds` files blindly. #' -#' See [list_parsers()] for a list of registered parsers. +#' See [registered_parsers()] for a list of registered parsers. #' #' @param ... parameters supplied to the appropriate internal function #' @describeIn parsers Query string parser diff --git a/man/PlumberEndpoint.Rd b/man/PlumberEndpoint.Rd index c93c0f3e9..13d25a152 100644 --- a/man/PlumberEndpoint.Rd +++ b/man/PlumberEndpoint.Rd @@ -143,7 +143,7 @@ Create a new \code{PlumberEndpoint} object \item{\code{serializer}}{endpoint serializer} -\item{\code{parsers}}{endpoint parsers} +\item{\code{parsers}}{endpoint parsers. If provided, the value will be sent processed by \code{\link[=make_parser]{make_parser()}}} \item{\code{lines}}{endpoint block} diff --git a/man/parsers.Rd b/man/parsers.Rd index 7bc5e1c45..1a3451f3a 100644 --- a/man/parsers.Rd +++ b/man/parsers.Rd @@ -61,7 +61,7 @@ Multiple parsers can be activated for the same endpoint using multiple \verb{@pa User should be aware that \code{rds} parsing should only be done from a trusted source. Do not accept \code{rds} files blindly. -See \code{\link[=list_parsers]{list_parsers()}} for a list of registered parsers. +See \code{\link[=registered_parsers]{registered_parsers()}} for a list of registered parsers. } \section{Functions}{ \itemize{ @@ -82,13 +82,13 @@ This parser should be used when reading from a file is required. \item \code{parser_rds}: RDS parser -\item \code{parser_octet}: Octet stream parser +\item \code{parser_octet}: Octet stream parser. Will add a filename attribute if the filename exists \item \code{parser_multi}: Multi part parser. This parser will then parse each individual body with its respective parser -\item \code{parser_all}: All parsers (For internal use only) +\item \code{parser_all}: Enable all parsers. Not recommended due to security concerns. -\item \code{parser_none}: No parser (For internal use only) +\item \code{parser_none}: No parser. Will not process the postBody. }} \examples{ diff --git a/man/register_parser.Rd b/man/register_parser.Rd index 8073fb504..701eeab54 100644 --- a/man/register_parser.Rd +++ b/man/register_parser.Rd @@ -3,19 +3,19 @@ \name{register_parser} \alias{register_parser} \alias{registered_parsers} -\alias{combine_parsers} +\alias{make_parser} \title{Manage parsers} \usage{ register_parser(alias, parser, fixed = NULL, regex = NULL, verbose = TRUE) registered_parsers() -combine_parsers(aliases) +make_parser(aliases) } \arguments{ \item{alias}{An alias to map parser from the \verb{@parser} plumber tag to the global parsers list.} -\item{parser}{The parser function to be added. This build the parser function.} +\item{parser}{The parser function to be added. This build the parser function. See Details for more information.} \item{fixed}{A character vector of fixed string to be matched against a request \code{content-type} to use \code{parser}.} @@ -29,6 +29,7 @@ displayed when alias in map are overwritten.} \item A character vector of \code{alias} names. \item A named \code{list()} whose keysare \code{alias} names and values are arguments to be applied with \code{\link[=do.call]{do.call()}} \item A \code{TRUE} value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes. +\item Already combined parsers. (Will be returned immediately.) }} } \description{ @@ -60,7 +61,7 @@ Parser function structure is something like below.\if{html}{\out{
\itemize{ \item \code{registered_parsers}: Return all registered parsers -\item \code{combine_parsers}: Select from global parsers and create a combined parsers list for programmatic use. +\item \code{make_parser}: Select from global parsers and create a combined parser list for programmatic use. }} \examples{ diff --git a/tests/testthat/test-async.R b/tests/testthat/test-async.R index ebe567e5a..db7591eb7 100644 --- a/tests/testthat/test-async.R +++ b/tests/testthat/test-async.R @@ -101,7 +101,7 @@ test_that("async hooks create async execution", { # make an exhaustive matrix of T/F values of which hooks are async hooks_are_async <- do.call( expand.grid, - lapply(setNames(hooks, hooks), function(...) c(FALSE, TRUE)) + lapply(stats::setNames(hooks, hooks), function(...) c(FALSE, TRUE)) ) # remove the all FALSE row diff --git a/tests/testthat/test-querystring.R b/tests/testthat/test-querystring.R index b302af6ce..29927c092 100644 --- a/tests/testthat/test-querystring.R +++ b/tests/testthat/test-querystring.R @@ -55,7 +55,7 @@ test_that("different lengths of query string return same shape", { parseQS( paste0("?", paste0(keys, "=", vals, collapse = "&")) ), - setNames( + stats::setNames( lapply(unique(keys), function(key) { unname(unlist(vals[keys == key])) }), From 98fd28a24f05a3727013987dcf61aee1d51b48f4 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 11:04:07 -0400 Subject: [PATCH 48/71] do not document hookable for now --- R/plumber.R | 3 ++- man/hookable.Rd | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/R/plumber.R b/R/plumber.R index 937d8665d..ac487f4dc 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -102,7 +102,8 @@ defaultPlumberFilters <- list( cookieParser = cookieFilter, sharedSecret = sharedSecretFilter) -#' hookable +#' @keywords internal +#' @title hookable hookable <- R6Class( "hookable", public=list( diff --git a/man/hookable.Rd b/man/hookable.Rd index 6c2802e83..13e8ef6b6 100644 --- a/man/hookable.Rd +++ b/man/hookable.Rd @@ -8,6 +8,7 @@ hookable hookable } +\keyword{internal} \section{Methods}{ \subsection{Public methods}{ \itemize{ From 4a4d97bc2dffae830a1609ed94e7c89596c49da3 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 11:04:19 -0400 Subject: [PATCH 49/71] Heavily update the pkgdown order --- pkgdown/_pkgdown.yml | 46 +++++++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index 352eaaaa6..dfffd269f 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -45,6 +45,8 @@ reference: - 'plumb' - 'pr' - 'pr_run' + - 'options_plumber' + - title: Router Methods contents: - 'pr_get' @@ -54,39 +56,53 @@ reference: - 'pr_head' - 'pr_handle' - 'pr_mount' + - title: Router Hooks contents: - 'pr_hook' - 'pr_hooks' - 'pr_cookie' - 'pr_filter' + - title: Router Defaults contents: - 'pr_set_serializer' - 'pr_set_404' - 'pr_set_error' -- title: All functions - # desc: description here + +- title: POST Body and Query String Parsers + contents: + - 'register_parser' + - 'parser_query' + - 'getCharacterSet' + +- title: Response + contents: + - 'as_attachment' + - 'addSerializer' + - 'serializer_json' + - 'include_file' + +- title: Cookies and Filters + contents: + - 'pr_cookie' + - 'randomCookieKey' + - 'sessionCookie' + - 'forward' + +- title: R6 Constructors contents: - 'PlumberEndpoint' - 'PlumberStatic' - 'PlumberStep' - - 'add_parser' - - 'addSerializer' - - 'as_attachment' + - 'hookable' + +- title: Digital Ocean + desc: TO BE REMOVED FROM PLUMBER!!! + contents: - 'do_configure_https' - 'do_deploy_api' - 'do_forward' - 'do_provision' - 'do_remove_api' - 'do_remove_forward' - - 'forward' - - 'getCharacterSet' - - 'hookable' - - 'include_file' - - 'options_plumber' - - 'parser_query' - - 'plumb' - - 'randomCookieKey' - - 'serializer_json' - - 'sessionCookie' From 364a8e10400a3a5a937277e1e50aa158fd9a7c9c Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 15:34:14 -0400 Subject: [PATCH 50/71] change name to sample_name --- tests/testthat/files/multipart-ctype.bin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/files/multipart-ctype.bin b/tests/testthat/files/multipart-ctype.bin index a76b50942..0b48369dd 100644 --- a/tests/testthat/files/multipart-ctype.bin +++ b/tests/testthat/files/multipart-ctype.bin @@ -1,5 +1,5 @@ -----------------------------90908882332870323642673870272 -Content-Disposition: form-data; name="file"; filename="sample.tsv" +Content-Disposition: form-data; name="sample_name"; filename="sample.tsv" Content-Type: text/tab-separated-values x y z From e86ea2e32996ec8b144acf0e1546b421373d4a06 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 15:35:38 -0400 Subject: [PATCH 51/71] If have default parsers already be sent through `make_parser()` --- R/plumber.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/R/plumber.R b/R/plumber.R index ac487f4dc..de91d9494 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -218,7 +218,7 @@ plumber <- R6Class( # Initialize private$serializer <- serializer_json() # Default parsers to maintain legacy features - private$default_parsers <- c("json", "query", "text", "octet", "multi") + private$default_parsers <- make_parser(c("json", "query", "text", "octet", "multi")) private$errorHandler <- defaultErrorHandler() private$notFoundHandler <- default404Handler private$maxSize <- getOption('plumber.maxRequestSize', 0) #0 Unlimited @@ -744,7 +744,7 @@ plumber <- R6Class( if (!is.null(h$parsers)) { h$parsers } else { - make_parser(private$default_parsers) + private$default_parsers } req$args <- c( h$getPathParams(path), @@ -894,9 +894,9 @@ plumber <- R6Class( private$serializer <- serializer }, #' @details Sets the default parsers of the router. - #' @param parsers a named list of parsers + #' @param parsers A value to be parsed by [make_parser()] setParsers = function(parsers) { - private$default_parsers <- parsers + private$default_parsers <- make_parser(parsers) }, #' @details Sets the handler that gets called if an #' incoming request can’t be served by any filter, endpoint, or sub-router. From 10c3d389b523f625882280a6bce0b15a9acf6b41 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 15:37:04 -0400 Subject: [PATCH 52/71] Remove `parser_all` from exising and being exported as it is not a _real_ parser (plus it didnt' work) --- R/parse-body.R | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 4b6b21ac0..0d4deca5a 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -201,9 +201,11 @@ registered_parsers <- function() { #' @describeIn register_parser Select from global parsers and create a combined parser list for programmatic use. #' @param aliases Can be one of: #' * A character vector of `alias` names. -#' * A named `list()` whose keysare `alias` names and values are arguments to be applied with [do.call()] +#' * A named `list()` whose keys are `alias` names and values are arguments to be applied with [do.call()] #' * A `TRUE` value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes. #' * Already combined parsers. (Will be returned immediately.) +#' +#' If `"all"` is found in any `alias` character value or list name, all remaining parsers will be added. When using a list, aliases already defined will maintain their existing argument values. All other parser aliases will use their default arguments. #' @export make_parser <- function(aliases) { if (inherits(aliases, "plumber_parsed_parsers")) { @@ -211,12 +213,15 @@ make_parser <- function(aliases) { } if (isTRUE(aliases)) { # use all available parsers except ("none") - aliases <- setdiff(registered_parsers(), c("all", "none")) + aliases <- "all" } if (is.character(aliases)) { if (any(is.na(aliases))) { stop("aliases can not be `NA` values") } + if ("all" %in% aliases) { + aliases <- setdiff(registered_parsers(), c("all", "none")) + } # turn aliases into a named list with empty values aliases <- stats::setNames( replicate(length(aliases), {list()}), @@ -248,7 +253,6 @@ make_parser <- function(aliases) { } } - # convert parser functions into initialized information parser_infos <- lapply( @@ -431,23 +435,6 @@ parser_multi <- function() { } } -#' @describeIn parsers Enable all parsers. Not recommended due to security concerns. -#' @export -parser_all <- function() { - function(value, content_type, filename, ...) { - # re-perform parse_raw(), but provide all parsers - parse_raw( - # mimic the shape of the output of `webutils::parse_multipart` + parsers - list( - content_type = content_type, - value = value, - filename = filename, - parsers = .globals$parsers - ) - ) - } -} - #' @describeIn parsers No parser. Will not process the postBody. #' @export parser_none <- function() { @@ -469,4 +456,9 @@ register_parsers_onLoad <- function() { register_parser("yaml", parser_yaml, fixed = c("application/yaml", "application/x-yaml", "text/yaml", "text/x-yaml")) register_parser("all", parser_all, regex = "*") register_parser("none", parser_none, regex = "*") + + parser_all <- function() { + stop("This function should never be called. It should be handled by `make_parser('all')`") + } + register_parser("all", parser_all, regex = "*") } From d503a2542cf16cdc51cf09040da97dff0c28bd47 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 15:38:11 -0400 Subject: [PATCH 53/71] Require readr within the inner parsing function, not in the outer parsing function Allows for `'all'` to include everything and not throw if one serializer can't meet it's dependency --- R/parse-body.R | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 0d4deca5a..a92fb74b2 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -340,10 +340,10 @@ parser_text <- function(parse_fn = identity) { #' @describeIn parsers YAML parser #' @export parser_yaml <- function(...) { - if (!requireNamespace("yaml", quietly = TRUE)) { - stop("yaml must be installed for the yaml parser to work") - } parser_text(function(val) { + if (!requireNamespace("yaml", quietly = TRUE)) { + stop("yaml must be installed for the yaml parser to work") + } yaml::yaml.load(val, ..., eval.expr = FALSE) }) } @@ -367,10 +367,10 @@ parser_read_file <- function(read_fn = readLines) { #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { - if (!requireNamespace("readr", quietly = TRUE)) { - stop("`readr` must be installed for `parser_csv` to work") - } parser_read_file(function(tmpfile) { + if (!requireNamespace("readr", quietly = TRUE)) { + stop("`readr` must be installed for `parser_csv` to work") + } readr::read_csv(tmpfile, ...) }) } @@ -379,10 +379,10 @@ parser_csv <- function(...) { #' @describeIn parsers TSV parser #' @export parser_tsv <- function(...) { - if (!requireNamespace("readr", quietly = TRUE)) { - stop("`readr` must be installed for `parser_tsv` to work") - } parser_read_file(function(tmpfile) { + if (!requireNamespace("readr", quietly = TRUE)) { + stop("`readr` must be installed for `parser_tsv` to work") + } readr::read_tsv(tmpfile, ...) }) } @@ -454,7 +454,6 @@ register_parsers_onLoad <- function() { register_parser("text", parser_text, fixed = "text/plain", regex = "^text/") register_parser("tsv", parser_tsv, fixed = c("application/tab-separated-values", "text/tab-separated-values")) register_parser("yaml", parser_yaml, fixed = c("application/yaml", "application/x-yaml", "text/yaml", "text/x-yaml")) - register_parser("all", parser_all, regex = "*") register_parser("none", parser_none, regex = "*") parser_all <- function() { From 4efe31b111f6122a64514a12a68956e800e050d2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 15:38:44 -0400 Subject: [PATCH 54/71] document and add more tests and fix tests --- NAMESPACE | 1 - man/parsers.Rd | 5 ---- tests/testthat/test-parse-block.R | 2 +- tests/testthat/test-parser.R | 40 +++++++++++++++++++++++-------- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index fd6c7ac21..eb1e6d993 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -19,7 +19,6 @@ export(include_md) export(include_rmd) export(make_parser) export(options_plumber) -export(parser_all) export(parser_csv) export(parser_json) export(parser_multi) diff --git a/man/parsers.Rd b/man/parsers.Rd index 1a3451f3a..d31a90053 100644 --- a/man/parsers.Rd +++ b/man/parsers.Rd @@ -11,7 +11,6 @@ \alias{parser_rds} \alias{parser_octet} \alias{parser_multi} -\alias{parser_all} \alias{parser_none} \title{Plumber Parsers} \usage{ @@ -35,8 +34,6 @@ parser_octet() parser_multi() -parser_all() - parser_none() } \arguments{ @@ -86,8 +83,6 @@ This parser should be used when reading from a file is required. \item \code{parser_multi}: Multi part parser. This parser will then parse each individual body with its respective parser -\item \code{parser_all}: Enable all parsers. Not recommended due to security concerns. - \item \code{parser_none}: No parser. Will not process the postBody. }} diff --git a/tests/testthat/test-parse-block.R b/tests/testthat/test-parse-block.R index 1c1de52c0..7d5dfbbbc 100644 --- a/tests/testthat/test-parse-block.R +++ b/tests/testthat/test-parse-block.R @@ -188,7 +188,7 @@ test_that("@parser parameters produce an error or not", { file = c("#' @get /test", "#' @parser octet list(key = \"val\")"), expr = substitute(identity), envir = new.env(), - addEndpoint = function(a, b, ...) { str(list(a , b, ...)); browser()}, + addEndpoint = function(a, b, ...) { stop("should not reach here")}, addFilter = as.null, pr = plumber$new() ) diff --git a/tests/testthat/test-parser.R b/tests/testthat/test-parser.R index c5bbcf8de..e7da4d7cf 100644 --- a/tests/testthat/test-parser.R +++ b/tests/testthat/test-parser.R @@ -16,24 +16,22 @@ test_that("parsers can be combined", { expect_parsers(c("query", "json"), c("query", "json"), sort_items = FALSE) expect_parsers("all", setdiff(registered_parsers(), c("all", "none"))) - expect_parsers(TRUE, setdiff(registered_parsers(), c("all", "none"))) expect_parsers(list(all = list()), setdiff(registered_parsers(), c("all", "none"))) + expect_parsers(TRUE, setdiff(registered_parsers(), c("all", "none"))) + + # make sure parameters are not overwritten even when including all parsers_plain <- make_parser(list(all = list(), json = list(simplifyVector = FALSE))) - expect_equal( - parsers_plain$alias$json(jsonlite::toJSON(1:3) %>% charToRaw()), - list(1,2,3) - ) + json_input <- parsers_plain$alias$json(charToRaw(jsonlite::toJSON(1:3))) + expect_equal(json_input, list(1,2,3)) parsers_guess <- make_parser(list(all = list(), json = list(simplifyVector = TRUE))) - expect_equal( - parsers_guess$alias$json(jsonlite::toJSON(1:3) %>% charToRaw()), - c(1,2,3) - ) + json_input <- parsers_guess$alias$json(charToRaw(jsonlite::toJSON(1:3))) + expect_equal(json_input, c(1,2,3)) # check that parsers return already combined parsers - expect_parsers(parsers_plain, setdiff(registered_parsers(), c("all", "none"))) + expect_equal(make_parser(parsers_plain), parsers_plain) }) test_that("parsers work", { @@ -48,6 +46,28 @@ test_that("parsers work", { expect_identical(r$route(make_req("POST", "/none", body=rawToChar(bin_body)), res), structure(list(), names = character())) expect_message(r$route(make_req("POST", "/json", body=rawToChar(bin_body)), res), "No suitable parser found") + bin_file <- test_path("files/multipart-form.bin") + bin_body <- readBin(bin_file, "raw", file.info(bin_file)$size) + + + req <- new.env() + req$REQUEST_METHOD <- "POST" + req$PATH_INFO <- "/all" + req$QUERY_STRING <- "" + req$HTTP_CONTENT_TYPE <- "multipart/form-data; boundary=----WebKitFormBoundaryMYdShB9nBc32BUhQ" + req$rook.input <- list(read_lines = function(){ stop("should not be executed") }, + read = function(){ bin_body }, + rewind = function(){ length(bin_body) }) + withr::with_options(list(plumber.postBody = FALSE), { + parsed_body <- r$route(req, PlumberResponse$new()) + }) + expect_equal(names(parsed_body), c("json", "img1", "img2", "rds")) + expect_equal(parsed_body[["rds"]], women) + expect_equal(attr(parsed_body[["img1"]], "filename"), "avatar2-small.png") + expect_equal(parsed_body[["json"]], list(a=2,b=4,c=list(w=3,t=5))) + + + # expect parsers match expect_equal(r$routes$none$parsers, make_parser("none")) expect_equal(r$routes$all$parsers, make_parser("all")) expect_equal(r$routes$default$parsers, NULL) From f8624edc99677f4088373dc27bdb4b2617989c8d Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 16:03:52 -0400 Subject: [PATCH 55/71] "remove" addSerializer(). Define register_serializer() and registered_serializers() --- DESCRIPTION | 1 + NAMESPACE | 2 ++ R/deprecated.R | 9 +++++++++ R/parse-body.R | 6 +++++- R/plumb-block.R | 4 ++-- R/serializer.R | 12 +++++++++--- man/addSerializer.Rd | 17 ++++------------- man/plumber.Rd | 2 +- man/register_parser.Rd | 9 +++++++-- man/register_serializer.Rd | 31 +++++++++++++++++++++++++++++++ pkgdown/_pkgdown.yml | 2 +- tests/testthat/test-plumber.R | 6 +++--- 12 files changed, 75 insertions(+), 26 deletions(-) create mode 100644 R/deprecated.R create mode 100644 man/register_serializer.Rd diff --git a/DESCRIPTION b/DESCRIPTION index ce784715c..3dd4bbce5 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -54,6 +54,7 @@ Collate: 'parse-query.R' 'plumber.R' 'default-handlers.R' + 'deprecated.R' 'digital-ocean.R' 'find-port.R' 'globals.R' diff --git a/NAMESPACE b/NAMESPACE index eb1e6d993..3924263f7 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -50,7 +50,9 @@ export(pr_set_error) export(pr_set_serializer) export(randomCookieKey) export(register_parser) +export(register_serializer) export(registered_parsers) +export(registered_serializers) export(serializer_cat) export(serializer_content_type) export(serializer_csv) diff --git a/R/deprecated.R b/R/deprecated.R new file mode 100644 index 000000000..4437b8a4c --- /dev/null +++ b/R/deprecated.R @@ -0,0 +1,9 @@ +#' Register a Serializer +#' +#' Use [register_serializer()] in favor of addSerializer +#' +#' @export +#' @keywords internal +addSerializer <- function(name, serializer, verbose = TRUE) { + register_serializer(name = name, serializer = serializer, verbose = verbose) +} diff --git a/R/parse-body.R b/R/parse-body.R index a92fb74b2..8f7f4dfad 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -195,7 +195,7 @@ register_parser <- function( #' @describeIn register_parser Return all registered parsers #' @export registered_parsers <- function() { - names(.globals$parsers) + sort(names(.globals$parsers)) } #' @describeIn register_parser Select from global parsers and create a combined parser list for programmatic use. @@ -206,6 +206,10 @@ registered_parsers <- function() { #' * Already combined parsers. (Will be returned immediately.) #' #' If `"all"` is found in any `alias` character value or list name, all remaining parsers will be added. When using a list, aliases already defined will maintain their existing argument values. All other parser aliases will use their default arguments. +#' @examples +#' make_parser("json") +#' make_parser(json = list()) +#' make_parser(json = list(simplifyVector = FALSE)) #' @export make_parser <- function(aliases) { if (inherits(aliases, "plumber_parsed_parsers")) { diff --git a/R/plumb-block.R b/R/plumb-block.R index 7d21bdd66..35c4905bb 100644 --- a/R/plumb-block.R +++ b/R/plumb-block.R @@ -102,7 +102,7 @@ plumbBlock <- function(lineNum, file){ stopOnLine(lineNum, line, "Multiple @serializers specified for one function.") } - if (!s %in% names(.globals$serializers)){ + if (!(s %in% registered_serializers())){ stopOnLine(lineNum, line, paste0("No such @serializer registered: ", s)) } @@ -130,7 +130,7 @@ plumbBlock <- function(lineNum, file){ stopOnLine(lineNum, line, "Multiple @serializers specified for one function (shorthand serializers like @json count, too).") } - if (!is.na(s) && !s %in% names(.globals$serializers)){ + if (!is.na(s) && !(s %in% registered_serializers())){ stopOnLine(lineNum, line, paste0("No such @serializer registered: ", s)) } shortSerAttr <- trimws(shortSerMat[1,3]) diff --git a/R/serializer.R b/R/serializer.R index 554ac9386..58656e9dc 100644 --- a/R/serializer.R +++ b/R/serializer.R @@ -1,5 +1,5 @@ -#' Add a Serializer +#' Register a Serializer #' #' A serializer is responsible for translating a generated R value into output #' that a remote user can understand. For instance, the \code{serializer_json} @@ -9,9 +9,9 @@ #' @param name The name of the serializer (character string) #' @param serializer The serializer to be added. #' @param verbose Logical value which determines if a message should be printed when overwriting serializers -#' +#' @describeIn register_serializer Register a serializer with a name #' @export -addSerializer <- function(name, serializer, verbose = TRUE) { +register_serializer <- function(name, serializer, verbose = TRUE) { if (!is.null(.globals$serializers[[name]])) { if (isTRUE(verbose)) { message("Overwriting serializer: ", name) @@ -19,6 +19,12 @@ addSerializer <- function(name, serializer, verbose = TRUE) { } .globals$serializers[[name]] <- serializer } +#' @describeIn register_serializer Return a list of all registered serializers +#' @export +registered_serializers <- function(name) { + sort(names(.globals$serializers)) +} + # internal function to use directly within this file only. (performance purposes) # Other files should use `serializer_identity()` to avoid confusion diff --git a/man/addSerializer.Rd b/man/addSerializer.Rd index bb3bb207b..d72ffc035 100644 --- a/man/addSerializer.Rd +++ b/man/addSerializer.Rd @@ -1,21 +1,12 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/serializer.R +% Please edit documentation in R/deprecated.R \name{addSerializer} \alias{addSerializer} -\title{Add a Serializer} +\title{Register a Serializer} \usage{ addSerializer(name, serializer, verbose = TRUE) } -\arguments{ -\item{name}{The name of the serializer (character string)} - -\item{serializer}{The serializer to be added.} - -\item{verbose}{Logical value which determines if a message should be printed when overwriting serializers} -} \description{ -A serializer is responsible for translating a generated R value into output -that a remote user can understand. For instance, the \code{serializer_json} -serializes R objects into JSON before returning them to the user. The list of -available serializers in plumber is global. +Use \code{\link[=register_serializer]{register_serializer()}} in favor of addSerializer } +\keyword{internal} diff --git a/man/plumber.Rd b/man/plumber.Rd index 68999570c..d8014c400 100644 --- a/man/plumber.Rd +++ b/man/plumber.Rd @@ -596,7 +596,7 @@ Sets the default serializer of the router. \subsection{Arguments}{ \if{html}{\out{
}} \describe{ -\item{\code{parsers}}{a named list of parsers} +\item{\code{parsers}}{A value to be parsed by \code{\link[=make_parser]{make_parser()}}} } \if{html}{\out{
}} } diff --git a/man/register_parser.Rd b/man/register_parser.Rd index 701eeab54..efc8205f4 100644 --- a/man/register_parser.Rd +++ b/man/register_parser.Rd @@ -27,10 +27,12 @@ displayed when alias in map are overwritten.} \item{aliases}{Can be one of: \itemize{ \item A character vector of \code{alias} names. -\item A named \code{list()} whose keysare \code{alias} names and values are arguments to be applied with \code{\link[=do.call]{do.call()}} +\item A named \code{list()} whose keys are \code{alias} names and values are arguments to be applied with \code{\link[=do.call]{do.call()}} \item A \code{TRUE} value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes. \item Already combined parsers. (Will be returned immediately.) -}} +} + +If \code{"all"} is found in any \code{alias} character value or list name, all remaining parsers will be added. When using a list, aliases already defined will maintain their existing argument values. All other parser aliases will use their default arguments.} } \description{ A parser is responsible for decoding the raw body content of a request into @@ -75,4 +77,7 @@ parser_dcf <- function() { } } register_parser("dcf", parser_dcf, fixed = "text/x-dcf") +make_parser("json") +make_parser(json = list()) +make_parser(json = list(simplifyVector = FALSE)) } diff --git a/man/register_serializer.Rd b/man/register_serializer.Rd new file mode 100644 index 000000000..5e4a08d84 --- /dev/null +++ b/man/register_serializer.Rd @@ -0,0 +1,31 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/serializer.R +\name{register_serializer} +\alias{register_serializer} +\alias{registered_serializers} +\title{Register a Serializer} +\usage{ +register_serializer(name, serializer, verbose = TRUE) + +registered_serializers(name) +} +\arguments{ +\item{name}{The name of the serializer (character string)} + +\item{serializer}{The serializer to be added.} + +\item{verbose}{Logical value which determines if a message should be printed when overwriting serializers} +} +\description{ +A serializer is responsible for translating a generated R value into output +that a remote user can understand. For instance, the \code{serializer_json} +serializes R objects into JSON before returning them to the user. The list of +available serializers in plumber is global. +} +\section{Functions}{ +\itemize{ +\item \code{register_serializer}: Register a serializer with a name + +\item \code{registered_serializers}: Return a list of all registered serializers +}} + diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index dfffd269f..8b6b97a77 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -79,7 +79,7 @@ reference: - title: Response contents: - 'as_attachment' - - 'addSerializer' + - 'register_serializer' - 'serializer_json' - 'include_file' diff --git a/tests/testthat/test-plumber.R b/tests/testthat/test-plumber.R index 991d49c47..ac0e20c18 100644 --- a/tests/testthat/test-plumber.R +++ b/tests/testthat/test-plumber.R @@ -87,17 +87,17 @@ test_that("plumb accepts a directory with a `plumber.R` file", { }) test_that("plumb() a dir leverages `entrypoint.R`", { - expect_null(plumber:::.globals$serializers$fake, "This just that your Plumber environment is dirty. Restart your R session.") + expect_null(.globals$serializers$fake, "This just that your Plumber environment is dirty. Restart your R session.") r <- plumb(dir = test_path("files/entrypoint/")) expect_equal(length(r$endpoints), 1) expect_equal(length(r$endpoints[[1]]), 1) # A global serializer was added by entrypoint.R before parsing - expect_true(!is.null(plumber:::.globals$serializers$fake)) + expect_true(!is.null(.globals$serializers$fake)) # Clean up after ourselves - gl <- plumber:::.globals + gl <- .globals gl$serializers["fake"] <- NULL }) From fb3bb70650f91964922a394d68e72cf4583fb4f2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 16:03:59 -0400 Subject: [PATCH 56/71] Fix test --- tests/testthat/test-parse-body.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-parse-body.R b/tests/testthat/test-parse-body.R index 1742ebb0f..0b7cb6621 100644 --- a/tests/testthat/test-parse-body.R +++ b/tests/testthat/test-parse-body.R @@ -106,5 +106,5 @@ test_that("Test multipart respect content-type", { parsed_body <- parse_body(body, "multipart/form-data; boundary=---------------------------90908882332870323642673870272", make_parser(c("multi", "tsv"))) - expect_s3_class(parsed_body$file, "data.frame") + expect_s3_class(parsed_body$sample_name, "data.frame") }) From c5376d779c852d2f2b9543f871474ea30688071f Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 16:21:35 -0400 Subject: [PATCH 57/71] Moar docs --- R/parse-body.R | 36 +++++++++++++++++++++++++++--------- man/register_parser.Rd | 36 +++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 8f7f4dfad..b59c6c9e8 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -118,24 +118,34 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' #' Parser function structure is something like below. #' ```r -#' parser <- () { -#' function(value, ...) { -#' # do something with raw value -#' } +#' parser <- (parser_arguments_here) { +#' # return a function to parse a raw value +#' function(value, ...) { +#' # do something with raw value +#' } #' } #' ``` #' #' @examples #' # `content-type` header is mostly used to look up charset and adjust encoding -#' parser_dcf <- function() { +#' parser_dcf <- function(...) { #' function(value, content_type = "text/x-dcf", ...) { #' charset <- getCharacterSet(content_type) #' value <- rawToChar(value) #' Encoding(value) <- charset -#' read.dcf(value) +#' read.dcf(value, ...) #' } #' } -#' register_parser("dcf", parser_dcf, fixed = "text/x-dcf") +#' +#' # Could also leverage existing parsers +#' parser_dcf <- function(...) { +#' parser_read_file(function(tmpfile) { +#' read.dcf(tmpfile, ...) +#' }) +#' } +#' +#' # Register the newly created parser +#' \dontrun{register_parser("dcf", parser_dcf, fixed = "text/x-dcf")} #' @export register_parser <- function( alias, @@ -207,9 +217,17 @@ registered_parsers <- function() { #' #' If `"all"` is found in any `alias` character value or list name, all remaining parsers will be added. When using a list, aliases already defined will maintain their existing argument values. All other parser aliases will use their default arguments. #' @examples +#' # provide a character string #' make_parser("json") -#' make_parser(json = list()) -#' make_parser(json = list(simplifyVector = FALSE)) +#' +#' # provide a named list with no arguments +#' make_parser(list(json = list())) +#' +#' # provide a named list with arguments; include `rds` +#' make_parser(list(json = list(simplifyVector = FALSE), rds = list())) +#' +#' # default plumber parsers +#' make_parser(c("json", "query", "text", "octet", "multi")) #' @export make_parser <- function(aliases) { if (inherits(aliases, "plumber_parsed_parsers")) { diff --git a/man/register_parser.Rd b/man/register_parser.Rd index efc8205f4..67ecb46be 100644 --- a/man/register_parser.Rd +++ b/man/register_parser.Rd @@ -52,10 +52,11 @@ Functions signature should include \code{value}, \code{...} and possibly \code{content_type}, \code{filename}. Other parameters may be provided if you want to use the headers from \code{\link[webutils:parse_multipart]{webutils::parse_multipart()}}. -Parser function structure is something like below.\if{html}{\out{
}}\preformatted{parser <- () \{ - function(value, ...) \{ - # do something with raw value - \} +Parser function structure is something like below.\if{html}{\out{
}}\preformatted{parser <- (parser_arguments_here) \{ + # return a function to parse a raw value + function(value, ...) \{ + # do something with raw value + \} \} }\if{html}{\out{
}} } @@ -68,16 +69,33 @@ Parser function structure is something like below.\if{html}{\out{
\examples{ # `content-type` header is mostly used to look up charset and adjust encoding -parser_dcf <- function() { +parser_dcf <- function(...) { function(value, content_type = "text/x-dcf", ...) { charset <- getCharacterSet(content_type) value <- rawToChar(value) Encoding(value) <- charset - read.dcf(value) + read.dcf(value, ...) } } -register_parser("dcf", parser_dcf, fixed = "text/x-dcf") + +# Could also leverage existing parsers +parser_dcf <- function(...) { + parser_read_file(function(tmpfile) { + read.dcf(tmpfile, ...) + }) +} + +# Register the newly created parser +\dontrun{register_parser("dcf", parser_dcf, fixed = "text/x-dcf")} +# provide a character string make_parser("json") -make_parser(json = list()) -make_parser(json = list(simplifyVector = FALSE)) + +# provide a named list with no arguments +make_parser(list(json = list())) + +# provide a named list with arguments; include `rds` +make_parser(list(json = list(simplifyVector = FALSE), rds = list())) + +# default plumber parsers +make_parser(c("json", "query", "text", "octet", "multi")) } From 2ab37b8a7ba09746d227feeeb4774953e5ed247f Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 24 Jul 2020 16:21:52 -0400 Subject: [PATCH 58/71] Use `local()` rather than `withr::with_options()` --- tests/testthat/test-parser.R | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-parser.R b/tests/testthat/test-parser.R index e7da4d7cf..e543f868f 100644 --- a/tests/testthat/test-parser.R +++ b/tests/testthat/test-parser.R @@ -58,9 +58,13 @@ test_that("parsers work", { req$rook.input <- list(read_lines = function(){ stop("should not be executed") }, read = function(){ bin_body }, rewind = function(){ length(bin_body) }) - withr::with_options(list(plumber.postBody = FALSE), { - parsed_body <- r$route(req, PlumberResponse$new()) - }) + + parsed_body <- + local({ + op <- options(plumber.postBody = FALSE) + on.exit({options(op)}, add = TRUE) + r$route(req, PlumberResponse$new()) + }) expect_equal(names(parsed_body), c("json", "img1", "img2", "rds")) expect_equal(parsed_body[["rds"]], women) expect_equal(attr(parsed_body[["img1"]], "filename"), "avatar2-small.png") From 733760d538051232b111751e978fa55a86476b8e Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 28 Jul 2020 10:22:41 -0400 Subject: [PATCH 59/71] update comments --- R/parse-body.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index b59c6c9e8..381c59878 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -234,7 +234,7 @@ make_parser <- function(aliases) { return(aliases) } if (isTRUE(aliases)) { - # use all available parsers except ("none") + # use all parsers aliases <- "all" } if (is.character(aliases)) { @@ -242,6 +242,7 @@ make_parser <- function(aliases) { stop("aliases can not be `NA` values") } if ("all" %in% aliases) { + # use all available parsers expect `all` and `none` aliases <- setdiff(registered_parsers(), c("all", "none")) } # turn aliases into a named list with empty values @@ -264,7 +265,7 @@ make_parser <- function(aliases) { } }) - # if "all" is found, add all remaining registered parsers (except 'none') to the `aliases` list + # if "all" is found, remove "all" and add all remaining registered parsers (except 'none') to the `aliases` list if ("all" %in% names(aliases)) { all_parser_names <- setdiff(registered_parsers(), c("all", "none")) # remove to avoid infinite recursion From 18655f7d5af4afb973be11b818231329483cf966 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Wed, 29 Jul 2020 14:26:31 -0500 Subject: [PATCH 60/71] Eval in proper env (related to #620) Co-authored-by: Bruno Tremblay --- R/plumb-block.R | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/R/plumb-block.R b/R/plumb-block.R index 35c4905bb..623cddce1 100644 --- a/R/plumb-block.R +++ b/R/plumb-block.R @@ -165,7 +165,11 @@ plumbBlock <- function(lineNum, file){ if (!is.na(parsersMat[1, 4]) && parsersMat[1,4] != ""){ # We have an arg to pass in to the parser - arg_list <- eval(parse(text=parsersMat[1,4])) + arg_list <- tryCatch({ + eval(parse(text=parsersMat[1,4]), envir) + }, error = function(e) { + stopOnLine(lineNum, line, e) + }) } else { arg_list <- list() } From d95325d16b44e0fac605a8536bcb01d65d60c4cc Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 09:55:27 -0400 Subject: [PATCH 61/71] Forgot to change the docs Co-authored-by: Carson Sievert --- R/parse-body.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index 381c59878..348d69a31 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -329,7 +329,7 @@ make_parser <- function(aliases) { #' #* @parser multi #' #* @parser rds #' pr <- plumber$new() -#' pr$handle("GET", "/upload", function(rds) {rds}, parsers = c(parser_multi(), parser_rds())) +#' pr$handle("GET", "/upload", function(rds) {rds}, parsers = c("multi", "rds")) #' } #' @export parser_query <- function() { From 71b6aa892b0ef931b2f0e1acee11f50c52b477eb Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 10:26:07 -0400 Subject: [PATCH 62/71] Move parser_read_file down in file --- R/parse-body.R | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 348d69a31..f49a65fd1 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -371,22 +371,6 @@ parser_yaml <- function(...) { }) } -#' @describeIn parsers Helper parser that writes the binary post body to a file and reads it back again using `read_fn`. -#' This parser should be used when reading from a file is required. -#' @param read_fn function used to read a the content of a file. Ex: [readRDS()] -#' @export -parser_read_file <- function(read_fn = readLines) { - stopifnot(is.function(read_fn)) - function(value, filename = "", ...) { - tmp <- tempfile("plumb", fileext = paste0("_", basename(filename))) - on.exit({ - file.remove(tmp) - }, add = TRUE) - writeBin(value, tmp) - read_fn(tmp) - } -} - #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { @@ -411,6 +395,23 @@ parser_tsv <- function(...) { } +#' @describeIn parsers Helper parser that writes the binary post body to a file and reads it back again using `read_fn`. +#' This parser should be used when reading from a file is required. +#' @param read_fn function used to read a the content of a file. Ex: [readRDS()] +#' @export +parser_read_file <- function(read_fn = readLines) { + stopifnot(is.function(read_fn)) + function(value, filename = "", ...) { + tmp <- tempfile("plumb", fileext = paste0("_", basename(filename))) + on.exit({ + file.remove(tmp) + }, add = TRUE) + writeBin(value, tmp) + read_fn(tmp) + } +} + + #' @describeIn parsers RDS parser #' @export parser_rds <- function(...) { From 1710168e4122dbaec2397e70230ff8c38942a394 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 10:26:30 -0400 Subject: [PATCH 63/71] Allow for readr to take original raw value directly --- R/parse-body.R | 12 ++++++------ man/parsers.Rd | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index f49a65fd1..3b9af6d9e 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -374,24 +374,24 @@ parser_yaml <- function(...) { #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { - parser_read_file(function(tmpfile) { + function(value, ...) { if (!requireNamespace("readr", quietly = TRUE)) { stop("`readr` must be installed for `parser_csv` to work") } - readr::read_csv(tmpfile, ...) - }) + readr::read_csv(value, ...) + } } #' @describeIn parsers TSV parser #' @export parser_tsv <- function(...) { - parser_read_file(function(tmpfile) { + function(value, ...) { if (!requireNamespace("readr", quietly = TRUE)) { stop("`readr` must be installed for `parser_tsv` to work") } - readr::read_tsv(tmpfile, ...) - }) + readr::read_tsv(value, ...) + } } diff --git a/man/parsers.Rd b/man/parsers.Rd index d31a90053..3b0980ef1 100644 --- a/man/parsers.Rd +++ b/man/parsers.Rd @@ -5,9 +5,9 @@ \alias{parser_json} \alias{parser_text} \alias{parser_yaml} -\alias{parser_read_file} \alias{parser_csv} \alias{parser_tsv} +\alias{parser_read_file} \alias{parser_rds} \alias{parser_octet} \alias{parser_multi} @@ -22,12 +22,12 @@ parser_text(parse_fn = identity) parser_yaml(...) -parser_read_file(read_fn = readLines) - parser_csv(...) parser_tsv(...) +parser_read_file(read_fn = readLines) + parser_rds(...) parser_octet() @@ -70,13 +70,13 @@ See \code{\link[=registered_parsers]{registered_parsers()}} for a list of regist \item \code{parser_yaml}: YAML parser -\item \code{parser_read_file}: Helper parser that writes the binary post body to a file and reads it back again using \code{read_fn}. -This parser should be used when reading from a file is required. - \item \code{parser_csv}: CSV parser \item \code{parser_tsv}: TSV parser +\item \code{parser_read_file}: Helper parser that writes the binary post body to a file and reads it back again using \code{read_fn}. +This parser should be used when reading from a file is required. + \item \code{parser_rds}: RDS parser \item \code{parser_octet}: Octet stream parser. Will add a filename attribute if the filename exists @@ -94,6 +94,6 @@ This parser should be used when reading from a file is required. #* @parser multi #* @parser rds pr <- plumber$new() -pr$handle("GET", "/upload", function(rds) {rds}, parsers = c(parser_multi(), parser_rds())) +pr$handle("GET", "/upload", function(rds) {rds}, parsers = c("multi", "rds")) } } From d4b6339c756f1c2c26d766362aa42d38b00f8183 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 10:27:08 -0400 Subject: [PATCH 64/71] rename test file to have it be executed last. (it's slow) --- tests/testthat/{test-include.R => test-zzz-include.R} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/testthat/{test-include.R => test-zzz-include.R} (100%) diff --git a/tests/testthat/test-include.R b/tests/testthat/test-zzz-include.R similarity index 100% rename from tests/testthat/test-include.R rename to tests/testthat/test-zzz-include.R From c6c3a094182e9e7849f93744801ccb66432ade89 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 10:44:52 -0400 Subject: [PATCH 65/71] scope readr parser args properly --- R/parse-body.R | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 3b9af6d9e..d104274ce 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -374,11 +374,14 @@ parser_yaml <- function(...) { #' @describeIn parsers CSV parser #' @export parser_csv <- function(...) { - function(value, ...) { + parse_fn <- function(raw_val) { if (!requireNamespace("readr", quietly = TRUE)) { stop("`readr` must be installed for `parser_csv` to work") } - readr::read_csv(value, ...) + readr::read_csv(raw_val, ...) + } + function(value, ...) { + parse_fn(value) } } @@ -386,11 +389,14 @@ parser_csv <- function(...) { #' @describeIn parsers TSV parser #' @export parser_tsv <- function(...) { - function(value, ...) { + parse_fn <- function(raw_val) { if (!requireNamespace("readr", quietly = TRUE)) { stop("`readr` must be installed for `parser_tsv` to work") } - readr::read_tsv(value, ...) + readr::read_tsv(raw_val, ...) + } + function(value, ...) { + parse_fn(value) } } From 538e75a1b34744a26b521d9e102e418b221b7b3b Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 10:48:20 -0400 Subject: [PATCH 66/71] safe guard against temp files possibly being deleted by user --- R/parse-body.R | 4 +++- R/serializer.R | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index d104274ce..1281d0b9c 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -410,7 +410,9 @@ parser_read_file <- function(read_fn = readLines) { function(value, filename = "", ...) { tmp <- tempfile("plumb", fileext = paste0("_", basename(filename))) on.exit({ - file.remove(tmp) + if (file.exists(tmp)) { + file.remove(tmp) + } }, add = TRUE) writeBin(value, tmp) read_fn(tmp) diff --git a/R/serializer.R b/R/serializer.R index 58656e9dc..60a7af6a9 100644 --- a/R/serializer.R +++ b/R/serializer.R @@ -278,7 +278,9 @@ serializer_htmlwidget <- function(...) { file <- tempfile(fileext = ".html") on.exit({ # Delete the temp file - file.remove(file) + if (file.exists(file)) { + file.remove(file) + } }) # Write the widget out to a file (doesn't currently support in-memory connections - pandoc) From 0761880a90be5fb350f8d212a222ba10cd25e016 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 10:53:12 -0400 Subject: [PATCH 67/71] add comment about how readRDS does not work with rawConnection --- R/parse-body.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/parse-body.R b/R/parse-body.R index 1281d0b9c..475ef0b53 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -424,6 +424,7 @@ parser_read_file <- function(read_fn = readLines) { #' @export parser_rds <- function(...) { parser_read_file(function(tmpfile) { + # `readRDS()` does not work with `rawConnection()` readRDS(tmpfile, ...) }) } From 16e01a9ef0dc2d059e33f1826469a7e0ccbad73c Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 10:54:18 -0400 Subject: [PATCH 68/71] Forgot to add `function` in the pseudo docs Co-authored-by: Carson Sievert --- R/parse-body.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/parse-body.R b/R/parse-body.R index 348d69a31..5ec48d0fd 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -118,7 +118,7 @@ parser_picker <- function(content_type, first_byte, filename = NULL, parsers = N #' #' Parser function structure is something like below. #' ```r -#' parser <- (parser_arguments_here) { +#' parser <- function(parser_arguments_here) { #' # return a function to parse a raw value #' function(value, ...) { #' # do something with raw value From 6959dd70b71a59caca86d668091b52f9e5342057 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 11:14:23 -0400 Subject: [PATCH 69/71] Do not export `make_parsers()`. Copy over appropriate docs --- NAMESPACE | 1 - R/parse-body.R | 1 - R/plumber-step.R | 43 +++++++++++++++++++++++++++++++----------- R/plumber.R | 26 ++++++++++++++++++++++++- man/PlumberEndpoint.Rd | 41 +++++++++++++++++++++++++++++----------- man/plumber.Rd | 26 ++++++++++++++++++++++++- man/register_parser.Rd | 2 +- 7 files changed, 113 insertions(+), 27 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index 3924263f7..d1a2338d7 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -17,7 +17,6 @@ export(include_file) export(include_html) export(include_md) export(include_rmd) -export(make_parser) export(options_plumber) export(parser_csv) export(parser_json) diff --git a/R/parse-body.R b/R/parse-body.R index bf1340cdb..4d1973a69 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -228,7 +228,6 @@ registered_parsers <- function() { #' #' # default plumber parsers #' make_parser(c("json", "query", "text", "octet", "multi")) -#' @export make_parser <- function(aliases) { if (inherits(aliases, "plumber_parsed_parsers")) { return(aliases) diff --git a/R/plumber-step.R b/R/plumber-step.R index b8ae91dcf..53c985851 100644 --- a/R/plumber-step.R +++ b/R/plumber-step.R @@ -173,17 +173,38 @@ PlumberEndpoint <- R6Class( !is.na(stri_match_first_regex(path, private$regex$regex)[1,1]) }, #' @description Create a new `PlumberEndpoint` object - #' @param verbs endpoint verb - #' @param path endpoint path - #' @param expr endpoint expr - #' @param envir endpoint environment - #' @param serializer endpoint serializer - #' @param parsers endpoint parsers. If provided, the value will be sent processed by [make_parser()] - #' @param lines endpoint block - #' @param params endpoint params - #' @param comments endpoint comments - #' @param responses endpoint responses - #' @param tags endpoint tags + #' @param verbs Endpoint verb Ex: `"GET"`, `"POST"` + #' @param path Endpoint path. Ex: `"/index.html"`, `"/foo/bar/baz"` + #' @param expr Endpoint expression or function. + #' @param envir Endpoint environment + #' @param serializer Endpoint serializer + #' @param parsers Endpoint parsers. + #' Can be one of: + #' * A `NULL` value + #' * A character vector of parser names + #' * A named `list()` whose keys are parser names names and values are arguments to be applied with [do.call()] + #' * A `TRUE` value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes + #' + #' If the parser name `"all"` is found in any character value or list name, all remaining parsers will be added. + #' When using a list, parser information already defined will maintain their existing argument values. All remaining parsers will use their default arguments. + #' + #' Example: + #' ``` + #' # provide a character string + #' parsers = "json" + #' + #' # provide a named list with no arguments + #' parsers = list(json = list()) + #' + #' # provide a named list with arguments; include `rds` + #' parsers = list(json = list(simplifyVector = FALSE), rds = list()) + #' + #' # default plumber parsers + #' parsers = c("json", "query", "text", "octet", "multi") + #' ``` + #' @param lines Endpoint block + #' @param params Endpoint params + #' @param comments,responses,tags Values to be used within the OpenAPI Spec #' @details Parameters values are obtained from parsing blocks of lines in a plumber file. #' They can also be provided manually for historical reasons. #' @return A new `PlumberEndpoint` object diff --git a/R/plumber.R b/R/plumber.R index de91d9494..e8ddc22da 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -894,7 +894,31 @@ plumber <- R6Class( private$serializer <- serializer }, #' @details Sets the default parsers of the router. - #' @param parsers A value to be parsed by [make_parser()] + #' @param parsers Set default endpoint parsers. Initialized to `c("json", "query", "text", "octet", "multi")` + #' + #' Can be one of: + #' * A `NULL` value + #' * A character vector of parser names + #' * A named `list()` whose keys are parser names names and values are arguments to be applied with [do.call()] + #' * A `TRUE` value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes + #' + #' If the parser name `"all"` is found in any character value or list name, all remaining parsers will be added. + #' When using a list, parser information already defined will maintain their existing argument values. All remaining parsers will use their default arguments. + #' + #' Example: + #' ``` + #' # provide a character string + #' parsers = "json" + #' + #' # provide a named list with no arguments + #' parsers = list(json = list()) + #' + #' # provide a named list with arguments; include `rds` + #' parsers = list(json = list(simplifyVector = FALSE), rds = list()) + #' + #' # default plumber parsers + #' parsers = c("json", "query", "text", "octet", "multi") + #' ``` setParsers = function(parsers) { private$default_parsers <- make_parser(parsers) }, diff --git a/man/PlumberEndpoint.Rd b/man/PlumberEndpoint.Rd index 13d25a152..bbfb37567 100644 --- a/man/PlumberEndpoint.Rd +++ b/man/PlumberEndpoint.Rd @@ -133,27 +133,46 @@ Create a new \code{PlumberEndpoint} object \subsection{Arguments}{ \if{html}{\out{
}} \describe{ -\item{\code{verbs}}{endpoint verb} +\item{\code{verbs}}{Endpoint verb Ex: \code{"GET"}, \code{"POST"}} -\item{\code{path}}{endpoint path} +\item{\code{path}}{Endpoint path. Ex: \code{"/index.html"}, \code{"/foo/bar/baz"}} -\item{\code{expr}}{endpoint expr} +\item{\code{expr}}{Endpoint expression or function.} -\item{\code{envir}}{endpoint environment} +\item{\code{envir}}{Endpoint environment} -\item{\code{serializer}}{endpoint serializer} +\item{\code{serializer}}{Endpoint serializer} -\item{\code{parsers}}{endpoint parsers. If provided, the value will be sent processed by \code{\link[=make_parser]{make_parser()}}} +\item{\code{parsers}}{Endpoint parsers. +Can be one of: +\itemize{ +\item A \code{NULL} value +\item A character vector of parser names +\item A named \code{list()} whose keys are parser names names and values are arguments to be applied with \code{\link[=do.call]{do.call()}} +\item A \code{TRUE} value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes +} -\item{\code{lines}}{endpoint block} +If the parser name \code{"all"} is found in any character value or list name, all remaining parsers will be added. +When using a list, parser information already defined will maintain their existing argument values. All remaining parsers will use their default arguments. -\item{\code{params}}{endpoint params} +Example:\preformatted{# provide a character string +parsers = "json" -\item{\code{comments}}{endpoint comments} +# provide a named list with no arguments +parsers = list(json = list()) -\item{\code{responses}}{endpoint responses} +# provide a named list with arguments; include `rds` +parsers = list(json = list(simplifyVector = FALSE), rds = list()) -\item{\code{tags}}{endpoint tags} +# default plumber parsers +parsers = c("json", "query", "text", "octet", "multi") +}} + +\item{\code{lines}}{Endpoint block} + +\item{\code{params}}{Endpoint params} + +\item{\code{comments, responses, tags}}{Values to be used within the OpenAPI Spec} } \if{html}{\out{
}} } diff --git a/man/plumber.Rd b/man/plumber.Rd index d8014c400..b0e3025a7 100644 --- a/man/plumber.Rd +++ b/man/plumber.Rd @@ -596,7 +596,31 @@ Sets the default serializer of the router. \subsection{Arguments}{ \if{html}{\out{
}} \describe{ -\item{\code{parsers}}{A value to be parsed by \code{\link[=make_parser]{make_parser()}}} +\item{\code{parsers}}{Set default endpoint parsers. Initialized to \code{c("json", "query", "text", "octet", "multi")} + +Can be one of: +\itemize{ +\item A \code{NULL} value +\item A character vector of parser names +\item A named \code{list()} whose keys are parser names names and values are arguments to be applied with \code{\link[=do.call]{do.call()}} +\item A \code{TRUE} value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes +} + +If the parser name \code{"all"} is found in any character value or list name, all remaining parsers will be added. +When using a list, parser information already defined will maintain their existing argument values. All remaining parsers will use their default arguments. + +Example:\preformatted{# provide a character string +parsers = "json" + +# provide a named list with no arguments +parsers = list(json = list()) + +# provide a named list with arguments; include `rds` +parsers = list(json = list(simplifyVector = FALSE), rds = list()) + +# default plumber parsers +parsers = c("json", "query", "text", "octet", "multi") +}} } \if{html}{\out{
}} } diff --git a/man/register_parser.Rd b/man/register_parser.Rd index 67ecb46be..0fc644be6 100644 --- a/man/register_parser.Rd +++ b/man/register_parser.Rd @@ -52,7 +52,7 @@ Functions signature should include \code{value}, \code{...} and possibly \code{content_type}, \code{filename}. Other parameters may be provided if you want to use the headers from \code{\link[webutils:parse_multipart]{webutils::parse_multipart()}}. -Parser function structure is something like below.\if{html}{\out{
}}\preformatted{parser <- (parser_arguments_here) \{ +Parser function structure is something like below.\if{html}{\out{
}}\preformatted{parser <- function(parser_arguments_here) \{ # return a function to parse a raw value function(value, ...) \{ # do something with raw value From 4d8591f36d7187bbcfed83caefd121bbafd4813a Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 11:22:15 -0400 Subject: [PATCH 70/71] Do not display docs for `make_parser()` --- R/parse-body.R | 40 ++++++++++++++++++++-------------------- man/register_parser.Rd | 26 -------------------------- 2 files changed, 20 insertions(+), 46 deletions(-) diff --git a/R/parse-body.R b/R/parse-body.R index 4d1973a69..925e7a433 100644 --- a/R/parse-body.R +++ b/R/parse-body.R @@ -208,26 +208,26 @@ registered_parsers <- function() { sort(names(.globals$parsers)) } -#' @describeIn register_parser Select from global parsers and create a combined parser list for programmatic use. -#' @param aliases Can be one of: -#' * A character vector of `alias` names. -#' * A named `list()` whose keys are `alias` names and values are arguments to be applied with [do.call()] -#' * A `TRUE` value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes. -#' * Already combined parsers. (Will be returned immediately.) -#' -#' If `"all"` is found in any `alias` character value or list name, all remaining parsers will be added. When using a list, aliases already defined will maintain their existing argument values. All other parser aliases will use their default arguments. -#' @examples -#' # provide a character string -#' make_parser("json") -#' -#' # provide a named list with no arguments -#' make_parser(list(json = list())) -#' -#' # provide a named list with arguments; include `rds` -#' make_parser(list(json = list(simplifyVector = FALSE), rds = list())) -#' -#' # default plumber parsers -#' make_parser(c("json", "query", "text", "octet", "multi")) +# ' @describeIn register_parser Select from global parsers and create a combined parser list for programmatic use. +# ' @param aliases Can be one of: +# ' * A character vector of `alias` names. +# ' * A named `list()` whose keys are `alias` names and values are arguments to be applied with [do.call()] +# ' * A `TRUE` value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes. +# ' * Already combined parsers. (Will be returned immediately.) +# ' +# ' If `"all"` is found in any `alias` character value or list name, all remaining parsers will be added. When using a list, aliases already defined will maintain their existing argument values. All other parser aliases will use their default arguments. +# ' @examples +# ' # provide a character string +# ' make_parser("json") +# ' +# ' # provide a named list with no arguments +# ' make_parser(list(json = list())) +# ' +# ' # provide a named list with arguments; include `rds` +# ' make_parser(list(json = list(simplifyVector = FALSE), rds = list())) +# ' +# ' # default plumber parsers +# ' make_parser(c("json", "query", "text", "octet", "multi")) make_parser <- function(aliases) { if (inherits(aliases, "plumber_parsed_parsers")) { return(aliases) diff --git a/man/register_parser.Rd b/man/register_parser.Rd index 0fc644be6..0abd01db7 100644 --- a/man/register_parser.Rd +++ b/man/register_parser.Rd @@ -3,14 +3,11 @@ \name{register_parser} \alias{register_parser} \alias{registered_parsers} -\alias{make_parser} \title{Manage parsers} \usage{ register_parser(alias, parser, fixed = NULL, regex = NULL, verbose = TRUE) registered_parsers() - -make_parser(aliases) } \arguments{ \item{alias}{An alias to map parser from the \verb{@parser} plumber tag to the global parsers list.} @@ -23,16 +20,6 @@ make_parser(aliases) \item{verbose}{Logical value which determines if a warning should be displayed when alias in map are overwritten.} - -\item{aliases}{Can be one of: -\itemize{ -\item A character vector of \code{alias} names. -\item A named \code{list()} whose keys are \code{alias} names and values are arguments to be applied with \code{\link[=do.call]{do.call()}} -\item A \code{TRUE} value, which will default to combining all parsers. This is great for seeing what is possible, but not great for security purposes. -\item Already combined parsers. (Will be returned immediately.) -} - -If \code{"all"} is found in any \code{alias} character value or list name, all remaining parsers will be added. When using a list, aliases already defined will maintain their existing argument values. All other parser aliases will use their default arguments.} } \description{ A parser is responsible for decoding the raw body content of a request into @@ -63,8 +50,6 @@ Parser function structure is something like below.\if{html}{\out{
\section{Functions}{ \itemize{ \item \code{registered_parsers}: Return all registered parsers - -\item \code{make_parser}: Select from global parsers and create a combined parser list for programmatic use. }} \examples{ @@ -87,15 +72,4 @@ parser_dcf <- function(...) { # Register the newly created parser \dontrun{register_parser("dcf", parser_dcf, fixed = "text/x-dcf")} -# provide a character string -make_parser("json") - -# provide a named list with no arguments -make_parser(list(json = list())) - -# provide a named list with arguments; include `rds` -make_parser(list(json = list(simplifyVector = FALSE), rds = list())) - -# default plumber parsers -make_parser(c("json", "query", "text", "octet", "multi")) } From 7be4fecc28786685343b5ca3a16df7b0b9c9b6eb Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 31 Jul 2020 11:27:06 -0400 Subject: [PATCH 71/71] update news entry --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 0d4e3ba4a..774a2d879 100644 --- a/NEWS.md +++ b/NEWS.md @@ -62,7 +62,7 @@ plumber 1.0.0 * Added yaml support, serializer and parser. (@meztez, #556) -* Added csv and tsv parsers (#584) +* Added parsers: `parser_csv()`, `parser_json()`, `parser_multi()`, `parser_octet()`, `parser_query()`, `parser_rds()`, `parser_text()`, `parser_tsv()`, `parser_yaml()`, `parser_none()`, and pseudo `"all"` (#584) * Added `serializer_csv()` (@pachamaltese, #520)