rstudio / rsconnect

Publish Shiny Applications, RMarkdown Documents, Jupyter Notebooks, Plumber APIs, and more
http://rstudio.github.io/rsconnect/
134 stars 82 forks source link

Switch from RCurl to curl #325

Closed jmcphers closed 5 years ago

jmcphers commented 5 years ago

rsconnect currently uses the RCurl package most of the time to perform HTTP operations.

https://cran.r-project.org/web/packages/RCurl/index.html

We should consider switching to curl, which is what most of the tidyverse uses.

https://cran.r-project.org/web/packages/curl/index.html

jeroen commented 5 years ago

This is quite important. The RCurl package is unmaintained and on Windows it uses a very very old version of openssl which only supports legacy versions of TLS (https).

For this reason Windows users are unable to connect with servers that use the latest HTTPS standards. Replacing RCurl with the curl (or httr) package will resolve your Windows HTTPS problems and probably fix many other random networking issues as well.

jmcphers commented 5 years ago

I've started working on this on a branch.

jjallaire commented 5 years ago

In case it wasn't obvious, you can add support for curl (and make it the default) while still keeping the old code around just in case any users rely on it. The package has multiple transports (curl binary, internal, RCurl) so you can just add another transport and make it the default: https://github.com/rstudio/rsconnect/blob/master/R/http.R

On Tue, Mar 12, 2019 at 5:50 PM Jonathan notifications@github.com wrote:

I've started working on this on a branch.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rstudio/rsconnect/issues/325#issuecomment-472195405, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGXx4G-_avj-6cDt7angHD6CMqvBm17ks5vWCE5gaJpZM4ZMKSM .

jmcphers commented 5 years ago

In recent versions of rsconnect, RCurl is no longer the default on Windows because of the issues @jeroen mentions above. We currently default to the curl command line utility on Windows instead (Windows 10 ships a competent one).

https://github.com/rstudio/rsconnect/commit/ae203f870f241e7b1e5de35230b635be754785ee#diff-031bbb2044688f5cb946c89f3a3dbd15

Consequently I was planning on dropping RCurl entirely -- we'd still have the rcurl transport, but it would mean "the curl R package" rather than "the RCurl package". Anyone opposed?

jjallaire commented 5 years ago

Fine by me On Tue, Mar 12, 2019 at 7:38 PM Jonathan notifications@github.com wrote:

In recent versions of rsconnect, RCurl is no longer the default on Windows because of the issues @jeroen https://github.com/jeroen mentions above. We currently default to the curl command line utility on Windows instead (Windows 10 ships a competent one).

ae203f8#diff-031bbb2044688f5cb946c89f3a3dbd15 https://github.com/rstudio/rsconnect/commit/ae203f870f241e7b1e5de35230b635be754785ee#diff-031bbb2044688f5cb946c89f3a3dbd15

Consequently I was planning on dropping RCurl entirely -- we'd still have the rcurl transport, but it would mean "the curl R package" rather than "the RCurl package". Anyone opposed?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/rstudio/rsconnect/issues/325#issuecomment-472222249, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGXx67HEMj_r1y2WVplXUnqNCJNG9apks5vWDpsgaJpZM4ZMKSM .

jeroen commented 5 years ago

Be careful that shelling out is often unreliable. For example if there is some networking error (which is not unusual), the curl command line returns non-zero and may print something to the terminal, but the exception does not get propagated back to R. This makes it difficult for the user to understand or debug what is the problem.