ropensci / vcr

Record and replay HTTP requests
https://docs.ropensci.org/vcr
Other
77 stars 12 forks source link

Fix URI mismatch with httr #240 #247

Closed KevCaz closed 2 years ago

KevCaz commented 2 years ago

Hi @sckott,

Hope you're doing well!

This is a proposition to fix #240. The reason of the mismatch is that the request matchers are not accounting for potential escaping of special character and httr actually applies such escaping on URI parameters:

httr:::compose_query
function (elements)
{
    if (length(elements) == 0) {
        return("")
    }
    if (!all(has_name(elements))) {
        stop("All components of query must be named", call. = FALSE)
    }
    stopifnot(is.list(elements))
    elements <- compact(elements)
    names <- curl::curl_escape(names(elements))
    encode <- function(x) {
        if (inherits(x, "AsIs")) 
            return(x)
        curl::curl_escape(x)
    }
    values <- vapply(elements, encode, character(1))
    paste0(names, "=", values, collapse = "&")
} 

So to account for this, I use `curl::curl_unescape() in the request matchers for URI and query, I also added unit tests for this,

Let me know what do you think!

sckott commented 2 years ago

👋🏽 @KevCaz - Thanks for this!

I'll take a look and get back at you.

FYI, I need to push a version to cran by end of next week to fix some major errors on cran for vcr reverse dependenciess, so this may or may not make it into that submission.

sckott commented 2 years ago

Looks great, merging. Thanks so much for taking the time do a PR!