Skip to content

Commit

Permalink
Body encoding (#259, #264)
Browse files Browse the repository at this point in the history
This should fix #153.  I added a test to validate that it does handle encoding issues, also I tested locally on my windows partition that it was broken and isn't now. 

#153: Cannot POST in UTF-8 under windows
#259: Body Encoding

* setting encoding to fix windows problems

* changing where setting encoding to ease testing

* adding test for encoding

* fixing encoding test (windows botched commit)

* fixing bad assumption that everything is UTF-8, now checking if the body is character and looking for charset

* adding a test for postBodyFilter

* natural collate order

* lints

Co-authored-by: Jeremy Farrell @farrjere
  • Loading branch information
schloerke authored Jun 4, 2018
1 parent a0f3e5d commit 8987a48
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 3 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Collate:
'digital-ocean.R'
'find-port.R'
'includes.R'
'new-rstudio-project.R'
'paths.R'
'plumber-static.R'
'plumber-step.R'
Expand All @@ -64,5 +65,4 @@ Collate:
'serializer.R'
'session-cookie.R'
'swagger.R'
'new-rstudio-project.R'
RoxygenNote: 6.0.1
19 changes: 19 additions & 0 deletions R/content-types.R
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,22 @@ getContentType <- function(ext, defaultType='application/octet-stream') {
}
return(ct)
}

getCharacterSet <- function(contentType){
default <- "UTF-8"
if (is.null(contentType)) {
return(default)
}
charsetStart <- attr(
gregexpr(".*charset=(.*)", contentType, perl = T)[[1]],
"capture.start"
)
charsetStart <- as.integer(charsetStart)
as.character(
ifelse(
charsetStart > -1,
substr(contentType, charsetStart, nchar(contentType)),
default
)
)
}
10 changes: 8 additions & 2 deletions R/post-body.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ postBodyFilter <- function(req){
handled <- req$.internal$postBodyHandled
if (is.null(handled) || handled != TRUE){
body <- req$rook.input$read_lines()
args <- parseBody(body)
charset <- getCharacterSet(req$HTTP_CONTENT_TYPE)
args <- parseBody(body, charset)
req$postBody <- body
req$args <- c(req$args, args)
req$.internal$postBodyHandled <- TRUE
Expand All @@ -12,12 +13,17 @@ postBodyFilter <- function(req){

#' @importFrom utils URLdecode
#' @noRd
parseBody <- function(body){
parseBody <- function(body, charset = "UTF-8"){
# The body in a curl call can also include querystring formatted data
# Is there data in the request?
if (is.null(body) || length(body) == 0 || body == "") {
return(list())
}

if (is.character(body)) {
Encoding(body) <- charset
}

# Is it JSON data?
if (stri_startswith_fixed(body, "{")) {
# Handle JSON with jsonlite
Expand Down
5 changes: 5 additions & 0 deletions R/response.R
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ PlumberResponse <- R6Class(
body <- ""
}

charset <- getCharacterSet(h$HTTP_CONTENT_TYPE)
if (is.character(body)) {
Encoding(body) <- charset
}

list(
status = self$status,
headers = h,
Expand Down
14 changes: 14 additions & 0 deletions tests/testthat/test-content-type.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,17 @@ test_that("contentType works in files", {
val <- r$serve(make_req("GET", "/"), res)
expect_equal(val$headers$`Content-Type`, "text/plain")
})

test_that("Parses charset properly", {
charset <- getCharacterSet("Content-Type: text/html; charset=latin1")
expect_equal(charset, "latin1")
charset <- getCharacterSet("Content-Type: text/html; charset=greek8")
expect_equal(charset, "greek8")
})

test_that("Defaults charset when not there", {
charset <- getCharacterSet("Content-Type: text/html")
expect_equal(charset, "UTF-8")
charset <- getCharacterSet(NULL)
expect_equal(charset, "UTF-8")
})
27 changes: 27 additions & 0 deletions tests/testthat/test-postbody.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,30 @@ 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"), list(a="1", d="1"))
})

test_that("Able to handle UTF-8", {
expect_equal(parseBody('{"text":"élise"}', "UTF-8")$text, "élise")
})

test_that("filter passes on charset", {
charset_passed <- ""
req <- list(
.internal = list(postBodyHandled = FALSE),
rook.input = list(
read_lines = function() {
called <- TRUE
return("this is a body")
}
),
HTTP_CONTENT_TYPE = "text/html; charset=testset",
args = c()
)
with_mock(
parseBody = function(body, charset = "UTF-8") {
print(charset)
body
},
expect_output(postBodyFilter(req), "testset"),
.env = "plumber"
)
})

0 comments on commit 8987a48

Please sign in to comment.