Fix urls #366
Fix urls #366
Conversation
colearendt
commented
Feb 19, 2021
colearendt
commented
- add some otherwise failing tests on protocol mismatches
- address one case where "magic" ports (443 and 80, which tend to vanish) can cause issues in url parsing
| rsconnect_remote_path_from_url <- function(board, url) { | ||
| url <- gsub("^https?://", "", url) | ||
| server <- gsub("^https?://", "", board$server) | ||
| url <- gsub("^https?://", "", strip_default_ports(url)) |
There was a problem hiding this comment.
Could you use urtools to do everything here? I think that might be easier to understand and less error prone.
There was a problem hiding this comment.
Oh and maybe use httr::parse_url() to avoid adding another dependency?
|
|
||
|
|
||
| path <- rsconnect_remote_path_from_url(list(server = "https://foo.com:8443/rsc"), "http://foo.com:8443/rsc/foo/bar") | ||
| expect_equal(path, "/foo/bar") | ||
| }) |
There was a problem hiding this comment.
Suggesting a new test case based on #379 that seems to be fixed by some changes in this PR. (because of trailing / added after server now)
it is not about protocol and port, so maybe in a new test_that call ?
path <- rsconnect_remote_path_from_url(list(server = "https://foo.com"), "http://foo.com/bar/foo-com_baz")
expect_equal(path, "/bar/foo-com_baz")With current version (not this PR)
pins:::rsconnect_remote_path_from_url(list(server = "https://foo.com"), "http://foo.com/bar/foo-com_baz")
[1] "_baz"
|
Closing because I've completely rewritten all this code in #426, and I'm pretty sure this is no longer needed. |
|
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
