diff --git a/NAMESPACE b/NAMESPACE index 1eac16818..649abe543 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -31,7 +31,5 @@ import(stringi) importFrom(grDevices,dev.off) importFrom(grDevices,jpeg) importFrom(grDevices,png) -importFrom(httpuv,decodeURI) -importFrom(httpuv,encodeURI) importFrom(httpuv,runServer) importFrom(stats,runif) diff --git a/NEWS.md b/NEWS.md index 2985dfac5..62969de41 100644 --- a/NEWS.md +++ b/NEWS.md @@ -54,6 +54,8 @@ plumber 0.5.0 ### Bug fixes +* Fix URL-decoding of query parameters and URL-encoding/decoding of cookies. Both now use `httpuv::decodeURIComponent` instead of `httpuv::decodeURI`. (@antoine-sachet, [#462](https://github.com/trestletech/plumber/pull/462)) + * Fix bugs that prevented `do_provision` from deploying to DigitalOcean and updated to the latest `analogsea`. ([#448](https://github.com/trestletech/plumber/pull/448/files)) * Fixed bug where functions defined earlier in the file could not be found when `plumb()`ing a file. ([#416](https://github.com/trestletech/plumber/pull/416)) diff --git a/R/cookie-parser.R b/R/cookie-parser.R index dfa4dbcf8..c30edceb4 100644 --- a/R/cookie-parser.R +++ b/R/cookie-parser.R @@ -4,7 +4,6 @@ cookieFilter <- function(req){ forward() } -#' @importFrom httpuv decodeURI #' @noRd parseCookies <- function(cookie) { if (is.null(cookie) || nchar(cookie) == 0) { @@ -23,7 +22,7 @@ parseCookies <- function(cookie) { } cookies <- vapply(cookieList, "[[", character(1), 2) - decodedCookies <- as.list(decodeURI(cookies)) + decodedCookies <- as.list(httpuv::decodeURIComponent(cookies)) cookieNames <- vapply(cookieList, "[[", character(1), 1) names(decodedCookies) <- cookieNames decodedCookies diff --git a/R/query-string.R b/R/query-string.R index fd7a8a0a5..d9be1c6f9 100644 --- a/R/query-string.R +++ b/R/query-string.R @@ -27,7 +27,7 @@ parseQS <- function(qs){ return(list()) } - keys <- decodeURI(vapply(kv, "[[", character(1), 1)) # returns utf8 strings + keys <- httpuv::decodeURIComponent(vapply(kv, "[[", character(1), 1)) # returns utf8 strings if (any(Encoding(keys) != "unknown")) { # https://github.com/trestletech/plumber/pull/314#discussion_r239992879 non_ascii <- setdiff(unique(Encoding(keys)), "unknown") @@ -38,7 +38,7 @@ parseQS <- function(qs){ } vals <- vapply(kv, "[[", character(1), 2) - vals <- decodeURI(vals) # returns utf8 strings + vals <- httpuv::decodeURIComponent(vals) # returns utf8 strings ret <- as.list(vals) names(ret) <- keys diff --git a/R/response.R b/R/response.R index b1f5a107b..d3ab286ba 100644 --- a/R/response.R +++ b/R/response.R @@ -58,10 +58,9 @@ removeCookieStr <- function(name, path, http = FALSE, secure = FALSE) { str } -#' @importFrom httpuv encodeURI #' @noRd cookieToStr <- function(name, value, path, expiration=FALSE, http=FALSE, secure=FALSE){ - val <- encodeURI(as.character(value)) + val <- httpuv::encodeURIComponent(as.character(value)) str <- paste0(name, "=", val, "; ") if (!missing(path)){ diff --git a/tests/testthat/test-cookies.R b/tests/testthat/test-cookies.R index fc126321b..fd807e02c 100644 --- a/tests/testthat/test-cookies.R +++ b/tests/testthat/test-cookies.R @@ -7,9 +7,9 @@ skip_if_no_cookie_support <- function() { test_that("cookies are parsed", { - cookies <- parseCookies("spaced=cookie%20here; another=2") + cookies <- parseCookies("spaced=cookie%2C%20here; another=2") expect_equal(names(cookies), c("spaced", "another")) - expect_equal(cookies$spaced, "cookie here") + expect_equal(cookies$spaced, "cookie, here") expect_equal(cookies$another, "2") cookies <- parseCookies("a=zxcv=asdf; missingVal=; b=qwer=ttyui") @@ -48,6 +48,7 @@ test_that("cookies can convert to string", { expect_equal(cookieToStr("abc", 123), "abc=123") expect_equal(cookieToStr("complex", "string with spaces"), "complex=string%20with%20spaces") + expect_equal(cookieToStr("complex2", "forbidden:,%/"), "complex2=forbidden%3A%2C%25%2F") expect_equal(cookieToStr("abc", 123, path="/somepath"), "abc=123; Path=/somepath") expect_equal(cookieToStr("abc", 123, http=TRUE, secure=TRUE), "abc=123; HttpOnly; Secure") diff --git a/tests/testthat/test-querystring.R b/tests/testthat/test-querystring.R index 573d896de..4524de080 100644 --- a/tests/testthat/test-querystring.R +++ b/tests/testthat/test-querystring.R @@ -9,6 +9,7 @@ test_that("query strings are properly parsed", { test_that("special characters in query strings are handled properly", { expect_equal(parseQS("?a=1+.#"), list(a="1+.#")) expect_equal(parseQS("?a=a%20b"), list(a="a b")) + expect_equal(parseQS('?a=%2C%2B%2F%3F%25%26'), list(a=",+/?%&")) }) test_that("null an empty strings return empty list", { diff --git a/tests/testthat/test-sessions.R b/tests/testthat/test-sessions.R index dc4bd55bc..928d6da23 100644 --- a/tests/testthat/test-sessions.R +++ b/tests/testthat/test-sessions.R @@ -45,8 +45,7 @@ test_that("cookies are set", { cook <- res$headers[["Set-Cookie"]] expect_match(cook, "^plcook") - cook <- sub("^plcook=", "", cook, perl = TRUE) - cook <- sub(";.*", "", cook) + cook <- parseCookies(cook)$plcook expect_equal(decodeCookie(cook, asCookieKey(key)), list(abc = 1234)) })