Pagedown: Using a non standard character in chrome_print() input returns an unclear error message

[ColinFay]

Hey,

First of all, thanks for the amazing chrome_print(). I just LOVE this functionality!

I'm using chrome_print() to print shiny app elements which are bookmarked.

I noticed this strange error when using " in the url inputs (taking the rstudio gallery example below), which is probably to be found with other parameters.

pagedown::chrome_print(
  'https://gallery.shinyapps.io/113-bookmarking-url/?_inputs_&n="France"'
)
#> Error in pagedown::chrome_print("https://gallery.shinyapps.io/113-bookmarking-url/?_inputs_&n=\"France\""): Failed to generate output. Reason: Message must be a valid JSON

But it's solved if I use utils::URLencode on the URL

pagedown::chrome_print(
  utils::URLencode('https://gallery.shinyapps.io/113-bookmarking-url/?_inputs_&n="France"')
)

^{Created on 2019-03-18 by the reprex package (v0.2.1)}

IMHO, the Reason: Message must be a valid JSON is a little bit esoteric.

It could be avoided with adding a simple input <- utils::URLencode(input) here: https://github.com/rstudio/pagedown/blob/master/R/chrome.R#L52

Will be happy to make a PR if you feel like it.

[RLesur]

Glad you love the chrome_print() function!
Thanks for this bug report!
We'd love a PR! However, I think that the call to utils::URLencode(input) should be inserted here: https://github.com/rstudio/pagedown/blob/5f9e2969bfa71b77daa3eb01112aa643ad1b5696/R/chrome.R#L73

[cderv]

Let’s not that the issue comes also from the fact that we build Json message manually with sprintf using ” as demiliting string character.

Here , If ' and ” are inverted in the call it works.

pagedown::chrome_print(
  “https://gallery.shinyapps.io/113-bookmarking-url/?_inputs_&n='France'”
)

So yes encoding the url work, but we may encounter other issue. However I am not sure there is a lot of other cases where input from users can impact the message send to chrome :thinking:
In the case, building the json string using a toJSON call may help.

[yihui]

I agree with @cderv: we should definitely call toJSON() to guarantee the JSON string is valid when there are dynamic values in the string (sprintf() is not robust) just like

https://github.com/rstudio/pagedown/blob/fc26c2f77a212a62e334ba0f1c6f4cf36a14aaa3/R/chrome.R#L344

Please feel free to submit a PR to use the helper function to_json() to process other messages that contains dynamic R values (and I don't think URLencode() will be necessary after that). Thanks!

[RLesur]

Good catch @cderv! :100: