Less-zealous redirect_uri normalization (#666)

Closes #646

Author: jonthegeek

NEWS.md

@@ -7,6 +7,7 @@
 * `req_headers_redacted()` supports dynamic dots (#647)
 * `resp_stream_sse()` now automatically retrieves the next event if the current event contains no data. The data is now returned as a single string (#650).
 * `aws_v4_signature()` now works if url contains query parameters (@jeffreyzuber, #645).
+* `req_oauth_auth_code()` no longer adds trailing "/" characters to well-formed `redirect_uri` values (@jonthegeek, #646).
 
 # httr2 1.1.0
 

R/oauth-flow-auth-code.R

@@ -209,7 +209,7 @@ normalize_redirect_uri <- function(redirect_uri,
                                    port = deprecated(),
                                    error_call = caller_env()) {
 
-  parsed <- url_parse(redirect_uri)
+  old <- parsed <- url_parse(redirect_uri)
 
   if (lifecycle::is_present(host_name)) {
     lifecycle::deprecate_warn(
@@ -250,7 +250,7 @@ normalize_redirect_uri <- function(redirect_uri,
   }
 
   list(
-    uri = url_build(parsed),
+    uri = if (identical(old, parsed)) redirect_uri else url_build(parsed),
     localhost = localhost,
     can_fetch_code = can_fetch_oauth_code(redirect_uri)
   )

tests/testthat/test-oauth-flow-auth-code.R

@@ -88,6 +88,15 @@ test_that("old args are deprecated", {
 
 })
 
+test_that("urls left as is if not changes needed", {
+  # Allow tests to run when is_hosted_session() is TRUE.
+  local_mocked_bindings(is_hosted_session = function() FALSE)
+
+  original_uri <- "http://localhost:8080"
+  normalized_uri <- normalize_redirect_uri(original_uri)
+  expect_equal(normalized_uri$uri, original_uri)
+})
+
 # ouath_flow_auth_code_parse ----------------------------------------------
 
 test_that("forwards oauth error", {
@@ -125,6 +134,7 @@ test_that("external auth code sources are detected correctly", {
 
 test_that("auth codes can be retrieved from an external source", {
   skip_on_cran()
+  local_mocked_bindings(sys_sleep = function(...) {})
 
   req <- local_app_request(function(req, res) {
     # Error on first, and then respond on second