Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unnecessary warning when POST (JSON) body has multiple lines #270

Closed
jnolis opened this issue Jun 5, 2018 · 5 comments
Closed

Unnecessary warning when POST (JSON) body has multiple lines #270

jnolis opened this issue Jun 5, 2018 · 5 comments
Labels
difficulty: novice Anyone could help effort: low < 1 day of work help wanted Solution is well-specified enough that any community member could fix priority: low Will be fixed eventually type: bug Maintainers have validated that it is a real bug in the project code
Milestone

Comments

@jnolis
Copy link

jnolis commented Jun 5, 2018

If the POST JSON body has newlines in it, plumber creates a warning during the request:

Warning in if (stri_startswith_fixed(body, "{")) { :
  the condition has length > 1 and only the first element will be used

Here is a toy example that creates the warning with a multi-line JSON body:

functions.R:

#' @post /test
function(message){
  "it works!"
}

run.R

library(plumber)
r <- plumb("functions.R")
r$run(port=80)

I think this could be fixed by simply changing the body in this line to body[1]

@schloerke
Copy link
Collaborator

I believe the body should be set to a multiline string.

https://github.com/trestletech/plumber/blob/8987a48a590dcab9e918c9d3109308220e6bc1de/R/post-body.R#L4 should be

  body <- paste0(req$rook.input$read_lines(), collapse = "\n")

@schloerke schloerke added type: bug Maintainers have validated that it is a real bug in the project code difficulty: novice Anyone could help effort: low < 1 day of work priority: low Will be fixed eventually help wanted Solution is well-specified enough that any community member could fix labels Aug 15, 2018
@robertdj
Copy link
Contributor

robertdj commented Aug 18, 2018

Both suggestions fix this issue. I have just tried the collapse approach and it doesn't cause any tests to fail.

Edit: How to decide which of the suggestions is better? @schloerke : Is there a reason why the body should be a multiline string?

@schloerke
Copy link
Collaborator

schloerke commented Aug 20, 2018

I don't know why it was a multiline string. My belief is that is should be collapsed to a single string as soon as possible. Most body elements do not make sense as individual lines.

Mind making a PR with the collapse change? (and a news item.)

@robertdj
Copy link
Contributor

Sure :-)

@ksens
Copy link

ksens commented Jun 7, 2019

Just for reference, noting that this warning still comes up in the version of plumber currently available on CRAN (published 2018-06-05).

I was able to get rid of the issue by installing the latest version from Github:

# in R
devtools::install_github('trestletech/plumber')

@schloerke schloerke added this to the v0.5.0 - Next CRAN release milestone Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: novice Anyone could help effort: low < 1 day of work help wanted Solution is well-specified enough that any community member could fix priority: low Will be fixed eventually type: bug Maintainers have validated that it is a real bug in the project code
Projects
None yet
Development

No branches or pull requests

4 participants