diff --git a/NEWS.md b/NEWS.md index 3e74a546..001b5ad4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # httr2 (development version) +* `req_body_json_modify()` can now be used on a request with an empty body. * `resp_timing()` exposes timing information about the request measured by libcurl (@arcresu, #725). * `req_url_query()` now re-calculates n lengths when using `.multi = "explode"` to avoid select/recycling issues (@Kevanness, #719). diff --git a/R/req-body.R b/R/req-body.R index cb0446c3..99681bd1 100644 --- a/R/req-body.R +++ b/R/req-body.R @@ -11,8 +11,9 @@ #' Adding a body to a request will automatically switch the method to POST. #' #' @inheritParams req_perform -#' @param type MIME content type. Will be ignored if you have manually set -#' a `Content-Type` header. +#' @param type MIME content type. The default, `""`, will not emit a +#' `Content-Type` header. Ignored if you have set a `Content-Type` header +#' with [req_headers()]. #' @returns A modified HTTP [request]. #' @examples #' req <- request(example_url()) |> @@ -53,36 +54,33 @@ NULL #' @export #' @rdname req_body #' @param body A literal string or raw vector to send as body. -req_body_raw <- function(req, body, type = NULL) { +req_body_raw <- function(req, body, type = "") { check_request(req) - if (!is.raw(body) && !is_string(body)) { + check_string(type) + + if (is.raw(body)) { + req_body(req, data = body, type = "raw", content_type = type) + } else if (is_string(body)) { + req_body(req, data = body, type = "string", content_type = type) + } else { cli::cli_abort("{.arg body} must be a raw vector or string.") } - - req_body( - req, - data = body, - type = "raw", - content_type = type %||% "" - ) } #' @export #' @rdname req_body #' @param path Path to file to upload. -req_body_file <- function(req, path, type = NULL) { +req_body_file <- function(req, path, type = "") { check_request(req) + check_string(path) if (!file.exists(path)) { - cli::cli_abort("{.arg path} ({.path {path}}) does not exist.") + cli::cli_abort("Can't find file {.path {path}}.") + } else if (dir.exists(path)) { + cli::cli_abort("{.arg path} must be a file, not a directory.") } + check_string(type) - # Need to override default content-type "application/x-www-form-urlencoded" - req_body( - req, - data = new_path(path), - type = "raw-file", - content_type = type %||% "" - ) + req_body(req, data = path, type = "file", content_type = type) } #' @export @@ -126,11 +124,11 @@ req_body_json <- function( #' @rdname req_body req_body_json_modify <- function(req, ...) { check_request(req) - if (req$body$type != "json") { - cli::cli_abort("Can only be used after {.fn req_body_json") + if (!req_body_type(req) %in% c("empty", "json")) { + cli::cli_abort("Can only be used after {.fn req_body_json}.") } - req$body$data <- utils::modifyList(req$body$data, list2(...)) + req$body$data <- utils::modifyList(req$body$data %||% list(), list2(...)) req } @@ -159,12 +157,7 @@ req_body_form <- function( dots <- multi_dots(..., .multi = .multi) data <- modify_list(.req$body$data, !!!dots) - req_body( - .req, - data = data, - type = "form", - content_type = "application/x-www-form-urlencoded" - ) + req_body(.req, data = data, type = "form") } #' @export @@ -174,12 +167,7 @@ req_body_multipart <- function(.req, ...) { data <- modify_list(.req$body$data, ...) # data must be character, raw, curl::form_file, or curl::form_data - req_body( - .req, - data = data, - type = "multipart", - content_type = NULL - ) + req_body(.req, data = data, type = "multipart") } # General structure ------------------------------------------------------- @@ -188,10 +176,12 @@ req_body <- function( req, data, type, - content_type, + content_type = NULL, params = list(), error_call = parent.frame() ) { + arg_match(type, c("raw", "string", "file", "json", "form", "multipart")) + if (!is.null(req$body) && req$body$type != type) { cli::cli_abort( c( @@ -211,94 +201,83 @@ req_body <- function( req } -req_body_info <- function(req) { - if (is.null(req$body)) { - "empty" - } else { - data <- req$body$data - if (is.raw(data)) { - glue("{length(data)} bytes of raw data") - } else if (is_string(data)) { - glue("a string") - } else if (is_path(data)) { - glue("path '{data}'") - } else if (is.list(data)) { - glue("{req$body$type} encoded data") - } else { - "invalid" - } - } +req_body_type <- function(req) { + req$body$type %||% "empty" } +req_body_info <- function(req) { + switch( + req_body_type(req), + empty = "empty", + raw = glue("a {length(req$body$data)} byte raw vector"), + string = "a string", + file = glue("a path '{req$body$data}'"), + json = "JSON data", + form = "form data", + multipart = "multipart data" + ) +} req_body_get <- function(req) { - if (is.null(req$body)) { - return("") - } switch( - req$body$type, + req_body_type(req), + empty = NULL, raw = req$body$data, - form = { - data <- unobfuscate(req$body$data) - url_query_build(data) - }, - json = exec(jsonlite::toJSON, req$body$data, !!!req$body$params), - cli::cli_abort("Unsupported request body type {.str {req$body$type}}.") + string = req$body$data, + file = readBin(req$body$data, "raw", n = file.size(req$body$data)), + json = unclass(exec(jsonlite::toJSON, req$body$data, !!!req$body$params)), + form = url_query_build(unobfuscate(req$body$data)), + multipart = { + # This is a bit clumsy because it requires a real request, which is + # currently a bit slow and requires httpuv. But better than nothing. + # Details at https://github.com/jeroen/curl/issues/388 + handle <- req_handle(req_body_apply(req)) + echo <- curl::curl_echo(handle, progress = FALSE) + rawToChar(echo$body) + } ) } req_body_apply <- function(req) { - if (is.null(req$body)) { - return(req) - } - - data <- req$body$data - type <- req$body$type - - if (type == "raw-file") { - size <- file.info(data)$size - # Only open connection if needed - delayedAssign("con", file(data, "rb")) - - req <- req_policies( - req, - done = function() close(con) - ) - req <- req_options( - req, - post = TRUE, - readfunction = function(nbytes, ...) readBin(con, "raw", nbytes), - seekfunction = function(offset, ...) seek(con, where = offset), - postfieldsize_large = size - ) - } else if (type == "raw") { - req <- req_body_apply_raw(req, data) - } else if (type == "json") { - req <- req_body_apply_raw(req, req_body_get(req)) - } else if (type == "multipart") { - data <- unobfuscate(data) - req$fields <- data - } else if (type == "form") { - req <- req_body_apply_raw(req, req_body_get(req)) - } else { - cli::cli_abort("Unsupported request body {.arg type}.", .internal = TRUE) - } + req <- switch( + req_body_type(req), + empty = req, + raw = req_body_apply_raw(req, req$body$data), + string = req_body_apply_string(req, req$body$data), + file = req_body_apply_connection(req, req$body$data), + json = req_body_apply_string(req, req_body_get(req)), + form = req_body_apply_string(req, req_body_get(req)), + multipart = req_body_apply_multipart(req, req$body$data), + ) - # Respect existing Content-Type if set - type_idx <- match("content-type", tolower(names(req$headers))) - if (!is.na(type_idx)) { - content_type <- req$headers[[type_idx]] - req$headers <- req$headers[-type_idx] - } else { - content_type <- req$body$content_type + # Set Content-Type if not already set + if (!is.null(req$body$content_type) && is.null(req$headers$`Content-Type`)) { + req <- req_headers(req, `Content-Type` = req$body$content_type) } - req <- req_headers(req, `Content-Type` = content_type) req } +req_body_apply_raw <- function(req, data) { + req_options(req, post = TRUE, postfieldsize = length(data), postfields = data) +} +req_body_apply_string <- function(req, data) { + req_body_apply_raw(req, charToRaw(enc2utf8(data))) +} +req_body_apply_connection <- function(req, data) { + size <- file.info(data)$size + # Only open connection if needed + delayedAssign("con", file(data, "rb")) -req_body_apply_raw <- function(req, body) { - if (is_string(body)) { - body <- charToRaw(enc2utf8(body)) - } - req_options(req, post = TRUE, postfieldsize = length(body), postfields = body) + req <- req_policies(req, done = function() close(con)) + req <- req_options( + req, + post = TRUE, + readfunction = function(nbytes, ...) readBin(con, "raw", nbytes), + seekfunction = function(offset, ...) seek(con, where = offset), + postfieldsize_large = size + ) + req +} +req_body_apply_multipart <- function(req, data) { + req$fields <- unobfuscate(req$body$data) + req } diff --git a/R/req-dry-run.R b/R/req-dry-run.R index ec7d8b53..b23e996f 100644 --- a/R/req-dry-run.R +++ b/R/req-dry-run.R @@ -82,6 +82,7 @@ req_dry_run <- function( invisible(list( method = resp$method, path = resp$path, + body = resp$body, headers = as.list(resp$headers) )) } diff --git a/man/req_body.Rd b/man/req_body.Rd index 67224a88..0a1ee500 100644 --- a/man/req_body.Rd +++ b/man/req_body.Rd @@ -9,9 +9,9 @@ \alias{req_body_multipart} \title{Send data in request body} \usage{ -req_body_raw(req, body, type = NULL) +req_body_raw(req, body, type = "") -req_body_file(req, path, type = NULL) +req_body_file(req, path, type = "") req_body_json( req, @@ -34,8 +34,9 @@ req_body_multipart(.req, ...) \item{body}{A literal string or raw vector to send as body.} -\item{type}{MIME content type. Will be ignored if you have manually set -a \code{Content-Type} header.} +\item{type}{MIME content type. The default, \code{""}, will not emit a +\code{Content-Type} header. Ignored if you have set a \code{Content-Type} header +with \code{\link[=req_headers]{req_headers()}}.} \item{path}{Path to file to upload.} diff --git a/tests/testthat/_snaps/req-body.md b/tests/testthat/_snaps/req-body.md index 6a2ccf80..6c85db0c 100644 --- a/tests/testthat/_snaps/req-body.md +++ b/tests/testthat/_snaps/req-body.md @@ -1,10 +1,37 @@ -# errors if file doesn't exist +# can't change body type + + Code + req %>% req_body_json(list(x = 1)) + Condition + Error in `req_body_json()`: + ! Can't change body type from raw to json. + i You must use only one type of `req_body_*()` per request. + +# can't send anything else Code - req_body_file(request_test(), "doesntexist", type = "text/plain") + req_body_raw(req, 1) + Condition + Error in `req_body_raw()`: + ! `body` must be a raw vector or string. + +# errors on invalid input + + Code + req_body_file(request_test(), 1) Condition Error in `req_body_file()`: - ! `path` ('doesntexist') does not exist. + ! `path` must be a single string, not the number 1. + Code + req_body_file(request_test(), "doesntexist") + Condition + Error in `req_body_file()`: + ! Can't find file 'doesntexist'. + Code + req_body_file(request_test(), ".") + Condition + Error in `req_body_file()`: + ! `path` must be a file, not a directory. # non-json type errors @@ -15,12 +42,26 @@ ! Unexpected content type "application/xml". * Expecting type "application/json" or suffix "json". -# can't change body type +# can't modify non-json data Code - req %>% req_body_json(list(x = 1)) + req_body_json_modify(req, a = 1) Condition - Error in `req_body_json()`: - ! Can't change body type from raw to json. - i You must use only one type of `req_body_*()` per request. + Error in `req_body_json_modify()`: + ! Can only be used after `req_body_json()`. + +# can send named elements as multipart + + Code + cat(req_body_get(req)) + Output + ---{id} + Content-Disposition: form-data; name="a" + + 1 + ---{id} + Content-Disposition: form-data; name="b" + + 2 + ---{id} diff --git a/tests/testthat/_snaps/req.md b/tests/testthat/_snaps/req.md index 50a81a20..bc72349d 100644 --- a/tests/testthat/_snaps/req.md +++ b/tests/testthat/_snaps/req.md @@ -18,7 +18,7 @@ Message POST https://example.com - Body: multipart encoded data + Body: multipart data # printing headers works with {} diff --git a/tests/testthat/test-req-auth-aws.R b/tests/testthat/test-req-auth-aws.R index 836f85cc..8d3cc4a6 100644 --- a/tests/testthat/test-req-auth-aws.R +++ b/tests/testthat/test-req-auth-aws.R @@ -41,7 +41,7 @@ test_that('aws_v4_signature calculates correct signature', { req <- request("https://example.execute-api.us-east-1.amazonaws.com/v0/") %>% req_method('POST') - body_sha256 <- openssl::sha256(req_body_get(req)) + body_sha256 <- openssl::sha256(req_body_get(req) %||% "") current_time <- as.POSIXct(1737483742, origin = "1970-01-01", tz = "EST") signature <- aws_v4_signature( diff --git a/tests/testthat/test-req-body.R b/tests/testthat/test-req-body.R index d1d754a0..6c056b56 100644 --- a/tests/testthat/test-req-body.R +++ b/tests/testthat/test-req-body.R @@ -1,14 +1,72 @@ +test_that("can't change body type", { + req <- request("http://example.com") %>% req_body_raw(raw(1)) + expect_snapshot(req %>% req_body_json(list(x = 1)), error = TRUE) +}) + +test_that("useful values for empty body", { + req <- request("http://example.com") + expect_equal(req_body_type(req), "empty") + expect_equal(req_body_info(req), "empty") + expect_equal(req_body_get(req), NULL) +}) + +# req_body_raw() --------------------------------------------------------------- + +test_that("can send string", { + req <- request_test("/post") %>% req_body_raw("test", type = "text/plain") + expect_equal(req_body_type(req), "string") + expect_equal(req_body_get(req), "test") + expect_equal(req_body_info(req), "a string") + + resp <- req_perform(req) + json <- resp_body_json(resp) + expect_equal(json$headers$`Content-Type`, "text/plain") + expect_equal(json$data, "test") +}) + +test_that("can send raw vector", { + data <- charToRaw("abcdef") + req <- request_test("/post") %>% req_body_raw(data) + expect_equal(req_body_type(req), "raw") + expect_equal(req_body_get(req), data) + expect_equal(req_body_info(req), "a 6 byte raw vector") + + resp <- req_perform(req) + json <- resp_body_json(resp) + expect_equal(json$headers$`Content-Type`, NULL) + expect_equal(json$headers$`Content-Length`, "6") +}) + +test_that("can't send anything else", { + req <- request_test() + expect_snapshot(req_body_raw(req, 1), error = TRUE) +}) + +test_that("can override body content type", { + req <- request_test("/post") %>% + req_body_raw('{"x":"y"}') %>% + req_headers("content-type" = "application/json") + resp <- req_perform(req) + headers <- resp_body_json(resp)$headers + expect_equal(headers$`content-type`, "application/json") + expect_equal(headers$`Content-Type`, NULL) +}) + +# req_body_file() -------------------------------------------------------------- + test_that("can send file", { - path <- withr::local_tempfile() # curl requests in 64kb chunks so this will hopefully illustrate # any subtle problems + path <- withr::local_tempfile() x <- strrep("x", 128 * 1024) writeChar(x, path, nchar(x)) - resp <- request_test("/post") %>% - req_body_file(path, type = "text/plain") %>% - req_perform() + req <- request_test("/post") %>% req_body_file(path, type = "text/plain") + expect_equal(req_body_type(req), "file") + expect_equal(rawToChar(req_body_get(req)), x) + expect_equal(req_body_info(req), glue::glue("a path '{path}'")) + resp <- req_perform(req) json <- resp_body_json(resp) expect_equal(json$headers$`Content-Type`, "text/plain") expect_equal(json$data, x) @@ -28,31 +86,26 @@ test_that("can send file with redirect", { expect_equal(resp_body_json(resp)$data, str) }) -test_that("errors if file doesn't exist", { - expect_snapshot( - req_body_file(request_test(), "doesntexist", type = "text/plain"), - error = TRUE - ) -}) - -test_that("can send string", { - resp <- request_test("/post") %>% - req_body_raw("test", type = "text/plain") %>% - req_perform() - - json <- resp_body_json(resp) - expect_equal(json$headers$`Content-Type`, "text/plain") - expect_equal(json$data, "test") +test_that("errors on invalid input", { + expect_snapshot(error = TRUE, { + req_body_file(request_test(), 1) + req_body_file(request_test(), "doesntexist") + req_body_file(request_test(), ".") + }) }) +# req_body_json() -------------------------------------------------------------- test_that("can send any type of object as json", { req <- request_test("/post") %>% req_body_json(mtcars) expect_equal(req$body$data, mtcars) data <- list(a = "1", b = "2") - resp <- request_test("/post") %>% - req_body_json(data) %>% - req_perform() + req <- request_test("/post") %>% req_body_json(data) + expect_equal(req_body_type(req), "json") + expect_equal(req_body_info(req), "JSON data") + expect_equal(req_body_get(req), '{"a":"1","b":"2"}') + + resp <- req_perform(req) json <- resp_body_json(resp) expect_equal(json$json, data) @@ -93,22 +146,31 @@ test_that("can modify json data", { expect_equal(req$body$data, list(a = list(b = list(c = 101, d = 2), e = 103))) }) -test_that("can send named elements as form/multipart", { +test_that("can modify empty body", { + req <- request_test() %>% + req_body_json_modify(a = 10, b = 20) + expect_equal(req$body$data, list(a = 10, b = 20)) +}) + +test_that("can't modify non-json data", { + req <- request_test() %>% req_body_raw("abc") + expect_snapshot(req |> req_body_json_modify(a = 1), error = TRUE) +}) + +# req_body_form() -------------------------------------------------------------- + +test_that("can send named elements as form", { data <- list(a = "1", b = "2") - resp <- request_test("/post") %>% - req_body_form(!!!data) %>% - req_perform() + req <- request_test("/post") %>% req_body_form(!!!data) + expect_equal(req_body_type(req), "form") + expect_equal(req_body_info(req), "form data") + expect_equal(req_body_get(req), "a=1&b=2") + + resp <- req_perform(req) json <- resp_body_json(resp) expect_equal(json$headers$`Content-Type`, "application/x-www-form-urlencoded") expect_equal(json$form, data) - - resp <- request_test("/post") %>% - req_body_multipart(!!!data) %>% - req_perform() - json <- resp_body_json(resp) - expect_match(json$headers$`Content-Type`, "multipart/form-data; boundary=-") - expect_equal(json$form, list(a = "1", b = "2")) }) test_that("can modify body data", { @@ -122,6 +184,25 @@ test_that("can modify body data", { expect_equal(req3$body$data, list(a = I("3"), a = I("4"))) }) +# req_body_multipart() --------------------------------------------------------- + +test_that("can send named elements as multipart", { + data <- list(a = "1", b = "2") + + req <- request_test("/post") %>% req_body_multipart(!!!data) + expect_equal(req_body_type(req), "multipart") + expect_equal(req_body_info(req), "multipart data") + expect_snapshot( + cat(req_body_get(req)), + transform = function(x) gsub("--------.*", "---{id}", x) + ) + + resp <- req_perform(req) + json <- resp_body_json(resp) + expect_match(json$headers$`Content-Type`, "multipart/form-data; boundary=-") + expect_equal(json$form, list(a = "1", b = "2")) +}) + test_that("can upload file with multipart", { skip_on_os("windows") # fails due to line ending difference @@ -141,24 +222,9 @@ test_that("can upload file with multipart", { ) }) -test_that("can override body content type", { - req <- request_test("/post") %>% - req_body_raw('{"x":"y"}') %>% - req_headers("content-type" = "application/json") - resp <- req_perform(req) - headers <- resp_body_json(resp)$headers - expect_equal(headers$`Content-Type`, "application/json") - expect_equal(headers$`content-type`, NULL) -}) - test_that("no issues with partial name matching", { req <- request_test("/get") %>% req_body_multipart(d = "some data") expect_named(req$body$data, "d") }) - -test_that("can't change body type", { - req <- request("http://example.com") %>% req_body_raw(raw(1)) - expect_snapshot(req %>% req_body_json(list(x = 1)), error = TRUE) -}) diff --git a/tests/testthat/test-req-dry-run.R b/tests/testthat/test-req-dry-run.R index 123cd9be..db859e8f 100644 --- a/tests/testthat/test-req-dry-run.R +++ b/tests/testthat/test-req-dry-run.R @@ -19,7 +19,7 @@ test_that("body is shown", { expect_snapshot(req_dry_run(req_json, pretty_json = FALSE)) # doesn't show binary data - req_binary <- req_body_raw(req, "CenĂ¡rio") + req_binary <- req_body_raw(req, charToRaw("CenĂ¡rio")) expect_snapshot(req_dry_run(req_binary)) })