From d446a73ad6dc68f9961e88dac6f90d11633c909e Mon Sep 17 00:00:00 2001 From: Joan Maspons Date: Mon, 29 Apr 2024 17:47:27 +0200 Subject: [PATCH 1/7] Fix `build_mock_url()` for requests with multipart body with string values --- NEWS.md | 2 ++ R/build-mock-url.R | 6 ++++-- tests/testthat/test-capture-requests.R | 20 ++++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index ae13e03..fd35d9f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # httptest2 1.1.0.9000 +* `build_mock_url()` now works with multipart body request containing strings (#40, @jmaspons) + # httptest2 1.1.0 * `request` is now removed when saving `httr2_response` objects. In earlier versions of `httr2`, requests were not included in responses, but in httr2 1.0.0, [they were added](https://github.com/r-lib/httr2/pull/359) in order to improve error messages. *If you recorded any responses with httr2 >= 1.0 and httptest2 prior to this version, you may have leaked auth secrets: this would happen if your requests included auth information (as in an `Authentication` header), and the response was saved in a .R file, not simplified to .json or other response-body-only formats. Please inspect your recorded responses and invalidate any tokens that were exposed.* diff --git a/R/build-mock-url.R b/R/build-mock-url.R index 74bd752..55b02a6 100644 --- a/R/build-mock-url.R +++ b/R/build-mock-url.R @@ -79,9 +79,11 @@ get_string_request_body <- function(req) { if (inherits(x, "form_file")) { # hash the file contents paste("File:", digest(x$path, serialize = FALSE, file = TRUE)) - } else { - # assume form_data + } else if (inherits(x, "form_data")){ rawToChar(x$value) + } else { + # assume string + x } }) b <- paste(c( diff --git a/tests/testthat/test-capture-requests.R b/tests/testthat/test-capture-requests.R index 94a38dd..2cfecb7 100644 --- a/tests/testthat/test-capture-requests.R +++ b/tests/testthat/test-capture-requests.R @@ -1,6 +1,8 @@ d <- tempfile() dl_file <- tempfile() webp_file <- tempfile() +file_path <- tempfile() +writeLines(letters[1:6], file_path) # The webfake URL will be something like 127.0.0.1:port, and port may vary # so the mock paths will be different every time it runs @@ -31,6 +33,14 @@ test_that("We can record a series of requests (a few ways)", { req_perform(path = webp_file) r8 <<- request(httpbin$url("/status/202")) %>% req_perform() r9 <<- request(httpbin$url("/status/200")) %>% req_perform() + r10 <<- request(httpbin$url("/post")) %>% + req_body_multipart( + file = curl::form_file(file_path), + string = "some text", + form_data = curl::form_data("form data") + ) %>% + req_method("POST") %>% + req_perform() stop_capturing() }) @@ -41,6 +51,7 @@ test_that("We can record a series of requests (a few ways)", { "httpbin.org/get.json", "httpbin.org/image/webp.R", # Not a simplifiable format, so .R "httpbin.org/image/webp.R-FILE", # The `write_disk` location + "httpbin.org/post-40b03d-POST.json", "httpbin.org/put-PUT.json", # Not a GET, but returns 200 "httpbin.org/response-headers-ac4928.json", "httpbin.org/status/200.txt", # empty 200 response "text/plain", so .txt @@ -94,6 +105,14 @@ test_that("We can then load the mocks it stores", { req_perform(path = mock_webp_file) m8 <- request(httpbin$url("/status/202")) %>% req_perform() m9 <- request(httpbin$url("/status/200")) %>% req_perform() + m10 <- request(httpbin$url("/post")) %>% + req_body_multipart( + file = curl::form_file(file_path), + string = "some text", + form_data = curl::form_data("form data") + ) %>% + req_method("POST") %>% + req_perform() }) }) expect_identical(resp_body_json(m1), resp_body_json(r1)) @@ -114,6 +133,7 @@ test_that("We can then load the mocks it stores", { expect_equal(resp_status(m9), 200) expect_equal(resp_content_type(m9), "text/plain") expect_false(resp_has_body(m9)) + expect_identical(resp_body_json(m10), resp_body_json(r10)) }) test_that("write_disk mocks can be reloaded even if the mock directory moves", { From 66e2195cb6db46b27aca57eb790cc51bfe889f11 Mon Sep 17 00:00:00 2001 From: Joan Maspons Date: Mon, 17 Jun 2024 18:48:53 +0200 Subject: [PATCH 2/7] Fix code style Co-authored-by: Neal Richardson --- R/build-mock-url.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/build-mock-url.R b/R/build-mock-url.R index 55b02a6..4f63edb 100644 --- a/R/build-mock-url.R +++ b/R/build-mock-url.R @@ -79,7 +79,7 @@ get_string_request_body <- function(req) { if (inherits(x, "form_file")) { # hash the file contents paste("File:", digest(x$path, serialize = FALSE, file = TRUE)) - } else if (inherits(x, "form_data")){ + } else if (inherits(x, "form_data")) { rawToChar(x$value) } else { # assume string From d68b18e57d434aa76f4188e416af5159baef39fc Mon Sep 17 00:00:00 2001 From: Joan Maspons Date: Mon, 17 Jun 2024 19:34:03 +0200 Subject: [PATCH 3/7] Multipart can also contain raw data --- NEWS.md | 4 ++-- R/build-mock-url.R | 2 +- tests/testthat/test-capture-requests.R | 8 +++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index fd35d9f..afa3de7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # httptest2 1.1.0.9000 -* `build_mock_url()` now works with multipart body request containing strings (#40, @jmaspons) +* `build_mock_url()` now works with multipart body request containing character strings or raw data (#40, @jmaspons) # httptest2 1.1.0 @@ -40,4 +40,4 @@ Changes to function signatures: * The `path` argument to `capture_requests()` and `start_capturing()` has been removed; instead set the mock path explicitly with `.mockPaths()` or use `with_mock_dir()`. * Internal function `save_response()` requires a `file` path argument because `httr2_response` objects do not contain their `request`, which is needed to construct the mock file path -* Internal function `build_mock_url()` no longer accepts a string URL as an input; it only accepts `request` objects \ No newline at end of file +* Internal function `build_mock_url()` no longer accepts a string URL as an input; it only accepts `request` objects diff --git a/R/build-mock-url.R b/R/build-mock-url.R index 4f63edb..9c9315f 100644 --- a/R/build-mock-url.R +++ b/R/build-mock-url.R @@ -82,7 +82,7 @@ get_string_request_body <- function(req) { } else if (inherits(x, "form_data")) { rawToChar(x$value) } else { - # assume string + # assume character string or raw x } }) diff --git a/tests/testthat/test-capture-requests.R b/tests/testthat/test-capture-requests.R index 2cfecb7..28dedf7 100644 --- a/tests/testthat/test-capture-requests.R +++ b/tests/testthat/test-capture-requests.R @@ -37,7 +37,8 @@ test_that("We can record a series of requests (a few ways)", { req_body_multipart( file = curl::form_file(file_path), string = "some text", - form_data = curl::form_data("form data") + form_data = curl::form_data("form data"), + raw_data = charToRaw("raw data") ) %>% req_method("POST") %>% req_perform() @@ -51,7 +52,7 @@ test_that("We can record a series of requests (a few ways)", { "httpbin.org/get.json", "httpbin.org/image/webp.R", # Not a simplifiable format, so .R "httpbin.org/image/webp.R-FILE", # The `write_disk` location - "httpbin.org/post-40b03d-POST.json", + "httpbin.org/post-4f024d-POST.json", "httpbin.org/put-PUT.json", # Not a GET, but returns 200 "httpbin.org/response-headers-ac4928.json", "httpbin.org/status/200.txt", # empty 200 response "text/plain", so .txt @@ -109,7 +110,8 @@ test_that("We can then load the mocks it stores", { req_body_multipart( file = curl::form_file(file_path), string = "some text", - form_data = curl::form_data("form data") + form_data = curl::form_data("form data"), + raw_data = charToRaw("raw data") ) %>% req_method("POST") %>% req_perform() From cd13553a4ecb0a75b0652cff384ebadc11fc5d75 Mon Sep 17 00:00:00 2001 From: Joan Maspons Date: Mon, 17 Jun 2024 19:39:22 +0200 Subject: [PATCH 4/7] Skip failing test on windows due to #42 --- tests/testthat/test-capture-requests.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-capture-requests.R b/tests/testthat/test-capture-requests.R index 28dedf7..fb3f38a 100644 --- a/tests/testthat/test-capture-requests.R +++ b/tests/testthat/test-capture-requests.R @@ -60,6 +60,7 @@ test_that("We can record a series of requests (a few ways)", { "httpbin.org/status/418.R" # Not 200 response, so .R ) # But since we don't use httpbin anymore, they're in the localhost-port dir + skip_on_os("windows") # TODO: remove after #42 is fixed expected_files <- sub("httpbin.org", httpbin_mock_url, expected_files) expect_identical(sort(dir(d, recursive = TRUE)), expected_files) From 604f2c33695b90c29dc485e8c27e30c409ab0552 Mon Sep 17 00:00:00 2001 From: Joan Maspons Date: Tue, 18 Jun 2024 21:41:21 +0200 Subject: [PATCH 5/7] Avoid newline differences among OSes --- tests/testthat/test-capture-requests.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-capture-requests.R b/tests/testthat/test-capture-requests.R index fb3f38a..009c308 100644 --- a/tests/testthat/test-capture-requests.R +++ b/tests/testthat/test-capture-requests.R @@ -2,7 +2,7 @@ d <- tempfile() dl_file <- tempfile() webp_file <- tempfile() file_path <- tempfile() -writeLines(letters[1:6], file_path) +cat(letters[1:6], file = file_path) # The webfake URL will be something like 127.0.0.1:port, and port may vary # so the mock paths will be different every time it runs @@ -52,7 +52,7 @@ test_that("We can record a series of requests (a few ways)", { "httpbin.org/get.json", "httpbin.org/image/webp.R", # Not a simplifiable format, so .R "httpbin.org/image/webp.R-FILE", # The `write_disk` location - "httpbin.org/post-4f024d-POST.json", + "httpbin.org/post-48db67-POST.json", "httpbin.org/put-PUT.json", # Not a GET, but returns 200 "httpbin.org/response-headers-ac4928.json", "httpbin.org/status/200.txt", # empty 200 response "text/plain", so .txt From 9f752a0f8e93e9bd1e7409b8c909d236ae7dae7b Mon Sep 17 00:00:00 2001 From: Joan Maspons Date: Tue, 18 Jun 2024 21:43:37 +0200 Subject: [PATCH 6/7] Check character and raw types in multipart requests --- tests/testthat/test-mock-api.R | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-mock-api.R b/tests/testthat/test-mock-api.R index beea45c..4a10434 100644 --- a/tests/testthat/test-mock-api.R +++ b/tests/testthat/test-mock-api.R @@ -175,13 +175,17 @@ with_mock_api({ r %>% req_body_multipart( a = curl::form_file(file_to_upload), - b = curl::form_data("strings") + b = curl::form_data("strings"), + c = "some text", + d = charToRaw("raw data") ) %>% req_perform(), "http://httpbin.not/post", "Multipart form: a = File: ae2b1fca515949e5d54fb22b8ed95575 - b = strings" + b = strings + c = some text + d = as.raw(c(0x72, 0x61, 0x77, 0x20, 0x64, 0x61, 0x74, 0x61))" ) }) From a99f21be47e4689c10ce4665ad37b935d1e6cf0a Mon Sep 17 00:00:00 2001 From: Joan Maspons Date: Tue, 18 Jun 2024 21:46:06 +0200 Subject: [PATCH 7/7] Test in windows --- tests/testthat/test-capture-requests.R | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testthat/test-capture-requests.R b/tests/testthat/test-capture-requests.R index 009c308..2e1a6f9 100644 --- a/tests/testthat/test-capture-requests.R +++ b/tests/testthat/test-capture-requests.R @@ -60,7 +60,6 @@ test_that("We can record a series of requests (a few ways)", { "httpbin.org/status/418.R" # Not 200 response, so .R ) # But since we don't use httpbin anymore, they're in the localhost-port dir - skip_on_os("windows") # TODO: remove after #42 is fixed expected_files <- sub("httpbin.org", httpbin_mock_url, expected_files) expect_identical(sort(dir(d, recursive = TRUE)), expected_files)