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

User defined cookies / headers are overwritten #405

Closed
colearendt opened this issue Feb 14, 2020 · 4 comments · Fixed by #779
Closed

User defined cookies / headers are overwritten #405

colearendt opened this issue Feb 14, 2020 · 4 comments · Fixed by #779
Assignees
Labels
bug an unexpected problem or unintended behavior libcurl 🕸️

Comments

@colearendt
Copy link
Member

For the purposes of my example, I have only tested with libcurl. However, hopefully we can remedy this issue with the other download handlers as well.

Straight invocation of libcurl:

hm <- curl::new_handle()
curl::handle_setopt(hm, .list = list(httpheader = "key:value"))
curl::curl_fetch_echo("http://localhost:32776/upload", handle = hm)$headers
#>                                                    accept 
#>                                                     "*/*" 
#>                                           accept-encoding 
#>                                           "deflate, gzip" 
#>                                                      host 
#>                                         "localhost:32776" 
#>                                                       key 
#>                                                   "value" 
#>                                                user-agent 
#> "R (3.4.3 x86_64-apple-darwin15.6.0 x86_64 darwin15.6.0)"
curl::handle_setopt(hm, .list = list(httpheader = "key2:value2"))

# NOTE: key:value missing
curl::curl_fetch_echo("http://localhost:32776/upload", handle = hm)$headers
#>                                                    accept 
#>                                                     "*/*" 
#>                                           accept-encoding 
#>                                           "deflate, gzip" 
#>                                                      host 
#>                                         "localhost:32776" 
#>                                                      key2 
#>                                                  "value2" 
#>                                                user-agent 
#> "R (3.4.3 x86_64-apple-darwin15.6.0 x86_64 darwin15.6.0)"
curl::handle_setopt(hm, .list = list(cookielist = c("key=value;key2=value2")))

# cannot see it here, but there are key=value; key2=value2 cookies defined on this request
curl::curl_fetch_echo("http://localhost:32776/upload", handle = hm)$headers
#>                                                    accept 
#>                                                     "*/*" 
#>                                           accept-encoding 
#>                                           "deflate, gzip" 
#>                                                      host 
#>                                         "localhost:32776" 
#>                                                      key2 
#>                                                  "value2" 
#>                                                user-agent 
#> "R (3.4.3 x86_64-apple-darwin15.6.0 x86_64 darwin15.6.0)"
curl::handle_setopt(hm, .list = list(httpheader = c("cookie:key=value")))

# cannot see here, but key2:value2 cookie is missing. Also, key2=value2 header missing
curl::curl_fetch_echo("http://localhost:32776/upload", handle = hm)$headers
#>                                                    accept 
#>                                                     "*/*" 
#>                                           accept-encoding 
#>                                           "deflate, gzip" 
#>                                                    cookie 
#>                                  "key=value; key2=value2" 
#>                                                      host 
#>                                         "localhost:32776" 
#>                                                user-agent 
#> "R (3.4.3 x86_64-apple-darwin15.6.0 x86_64 darwin15.6.0)"

Created on 2020-02-14 by the reprex package (v0.3.0)

As you can see, the httpheader setting is not additive, but overwrites. It even overwrites previously specified cookies. As a result, when we add the user's options here:

userOptions <- getOption("rsconnect.libcurl.options")
if (is.list(userOptions)) {
curl::handle_setopt(handle, .list = userOptions)
}

And then don't incorporate them later:

curl::handle_setheaders(handle, .list = headers)

Everything the user has specified gets overwritten.

Are there any other workarounds to specify arbitrary user-defined headers / cookies via .rsconnect_profile? This is something that we have a few customers stuck at when deploying to RStudio Connect (specifically when headers / cookies need to be specified to satisfy a proxy).

@colearendt
Copy link
Member Author

colearendt commented Feb 14, 2020

Spoke with @jeroen offline and libcurl (in particular) does not allow "reading" previously set headers. So to enable setting headers / cookies, we probably need to either:

  • expose a way to set headers / cookies outside of options("rsconnect.libcurl.options")
  • parse options("rsconnect.libcurl.options") to see which ones set headers

If we go with option 1 (which feels cleaner and may be more general across http handlers), then perhaps we should document that headers set via options("rsconnect.libcurl.options") will be overwritten.

At this point, I'm not sure whether this is a bug or feature request, but basically

users should be able to set arbitrary headers / cookies via .rsconnect_profile for use in enterprise environments

😄

@kevinushey
Copy link
Contributor

It seems like the right solution would involve us building up a list of options (overlaying user-specified options as appropriate) and then passing everything into curl::handle_setopt() in one go, rather than setting things piecemeal (as we then risk overwriting user-specified values)

Alternatively, the cheap way out would be to add options like rsconnect.libcurl.headers / rsconnect.libcurl.cookies.

@colearendt
Copy link
Member Author

colearendt commented Feb 14, 2020

Brilliant! That sounds like a win to me! 😄 Is it worth my investigating the other handlers as well? Ideally there'd be a way to set headers / cookies no matter which HTTP handler is chosen.

@adamconroy adamconroy self-assigned this Feb 24, 2020
@rstub
Copy link
Member

rstub commented Mar 9, 2020

It looks like (deprecated) RCurl has the same problem, since it overwrites anything that is present in options$httpheaders

options$httpheader <- extraHeaders

Plain curl does not offer any way to set custom headers. A general rsconnect.headers / rsconnect.cookies used by all backends might make sense.

@hadley hadley added the bug an unexpected problem or unintended behavior label Feb 21, 2023
hadley added a commit that referenced this issue Mar 20, 2023
Implements new `rsconnect.http.headers` and `rsconnect.http.cookies` options which allow you to set extra arbitrary additional headers/cookies on each request. Their use is documented in the new `vignette("custom-http")`.

Includes some minor refactoring of cookies code and tests.

Fixes #405.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior libcurl 🕸️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants