Skip to content

Commit 924415c

Browse files
authored
Pass original URL to curl_modify_url() to preserve original encoding (#788)
1 parent e1751ee commit 924415c

File tree

3 files changed

+16
-6
lines changed

3 files changed

+16
-6
lines changed

NEWS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# httr2 (development version)
22

3+
* Refactor `url_modify()` to better retain exact formatting of URL components
4+
that are not modified. (#788)
5+
36
# httr2 1.2.1
47

58
* Colons in paths are no longer escaped.

R/url.R

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,6 @@ url_modify <- function(
9595
stop_input_type(url, "a string or parsed URL")
9696
}
9797
string_url <- is_string(url)
98-
if (string_url) {
99-
url <- url_parse(url)
100-
}
10198

10299
check_url_component(scheme)
103100
check_url_component(hostname)
@@ -130,11 +127,12 @@ url_modify <- function(
130127
fragment = fragment
131128
)
132129
new <- new[!map_lgl(new, leave_as_is)]
133-
url[names(new)] <- new
134130

135131
if (string_url) {
136-
url_build(url)
132+
new[map_lgl(new, is.null)] <- ""
133+
url_build(structure(c(url = url, new), class = "httr2_url"))
137134
} else {
135+
url[names(new)] <- new
138136
url
139137
}
140138
}
@@ -262,13 +260,16 @@ url_build <- function(url) {
262260
stop_input_type(url, "a parsed URL")
263261
}
264262

265-
if (length(url$query) == 0) {
263+
if (is.null(url$query)) {
264+
query <- NULL
265+
} else if (length(url$query) == 0 || identical(url$query, "")) {
266266
query <- ""
267267
} else {
268268
query <- I(url_query_build(url$query))
269269
}
270270

271271
url <- curl::curl_modify_url(
272+
url = url$url,
272273
scheme = url$scheme,
273274
host = url$hostname,
274275
user = url$username,

tests/testthat/test-url.R

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ test_that("path always starts with /", {
127127
expect_equal(url_modify("https://x.com/abc", path = NULL), "https://x.com/")
128128
})
129129

130+
test_that("only modifies specified components", {
131+
url <- "http://x.com/a%2Fb/"
132+
expect_equal(url_modify(url), url)
133+
expect_equal(url_modify(url, query = list(x=1)), "http://x.com/a%2Fb/?x=1")
134+
})
135+
130136
# relative url ------------------------------------------------------------
131137

132138
test_that("can set relative urls", {

0 commit comments

Comments
 (0)