r-lib / httr

httr: a friendly http package for R
https://httr.r-lib.org
Other
985 stars 1.99k forks source link

parse_text checking encoding more than necessary #684

Closed bhogan-mitre closed 10 months ago

bhogan-mitre commented 3 years ago

parse_text is calling check_encoding (via guess_encoding) even in cases where no conversion is needed. Suppose httr::content is provided parameter encoding = "UTF-8", which is then passed through to parse_text. parse_text calls iconv with parameter to = "UTF-8", regardless of what the input encoding is.

Would you be open to skipping that check and conversion call in cases where the conversion from is equal to the conversion to (i.e. UTF-8)?

https://github.com/r-lib/httr/blob/master/R/content-parse.r#L28-L31

The reason I care is that I'm running into occasional errors where 'iconvlist' is not available on this system. It turns out that I don't actually need the conversion, and that I'm just trying to get at the raw content (via aws.s3::parse_aws_s3_response, https://github.com/cloudyr/aws.s3/blob/master/R/s3HTTP.R#L240).

Warning: Error in   iconvlist: 'iconvlist' is not available on this system
--
86:   stop
85:   iconvlist
82:   check_encoding
81:   guess_encoding
80:   parse_text
79:   httr::content
78:   parse_aws_s3_response
77:   s3HTTP

Aside: Yes, iconvlist should be consistently available, and I'm looking into why it is not. I think it has to do with some instability in a tmp directory managed by renv for a deployed Shiny app.

Browse[2]> icfile
[1] "/tmp/RtmppWmeyH/renv-system-library/utils/iconvlist"

Regardless, I think that is a separate issue from this question on httr::content and type conversion.

hadley commented 10 months ago

httr has been superseded in favour of httr2, so is no longer under active development. If this problem is still important to you in httr2, I'd suggest filing an issue offer there 😄. Thanks for using httr!