Add :escape_url request step

[jswanner]

See #430.

[wojtekmach]

@jswanner thank you for the PR and apologies for the delay. I've synced this branch with main.

We need to handle the scenario when the user encoded the Location value (which they were supposed to do) and we shouldn't double-encode it. RFCs explicitly call this out. If you could take a look how exactly other clients solve this, that'd be appreciated.

[jswanner]

we shouldn't double-encode it

Ah, you're right, this PR definitely double encodes URLs

If you could take a look how exactly other clients solve this, that'd be appreciated.

Req, Tesla and :httpc behave basically the same way: neither escapes the URL of an initial request nor those caused by following a redirect, and the error each client returns when an initial request URL is not escaped is the same error that is returned when it's a redirect URL that is not escaped.

Hackney, on the other hand, will always escape the URL (if needed) whether it's the URL of an initial request or request caused by following redirects (using :hackney_url.pathencode/1).

{:ok, _} =
  Bandit.start_link(
    plug: fn
      conn, _ when conn.request_path == "/" ->
        conn
        |> Plug.Conn.put_resp_header("location", "/foo bar")
        |> Plug.Conn.send_resp(301, "redirecting")

      conn, _ ->
        Plug.Conn.send_resp(conn, 200, conn.request_path)
    end,
    port: 8000
  )

Req.get("http://localhost:8000/foo bar")
# {:error, %Req.HTTPError{protocol: :http1, reason: {:invalid_request_target, "/foo bar"}}}

Req.get("http://localhost:8000/")
# [debug] redirecting to /foo bar
# {:error, %Req.HTTPError{protocol: :http1, reason: {:invalid_request_target, "/foo bar"}}}

:httpc.request(~c"http://localhost:8000/foo bar")
# {:error, :invalid_uri}

:httpc.request(~c"http://localhost:8000/")
# {:error, :invalid_uri}

Tesla.get("http://localhost:8000/foo bar")
# {:error, :invalid_uri}

Tesla.client([Tesla.Middleware.FollowRedirects]) |> Tesla.get("http://localhost:8000/")
# {:error, :invalid_uri}

:hackney.get("http://localhost:8000/foo bar")
# {:ok, 200,
#  [
#    {"date", "Thu, 25 Jun 2026 21:14:02 GMT"},
#    {"content-length", "8"},
#    {"vary", "accept-encoding"},
#    {"cache-control", "max-age=0, private, must-revalidate"}
#  ], "/foo+bar"}

:hackney.get("http://localhost:8000/", [], "", follow_redirect: true)
# {:ok, 200,
#  [
#    {"date", "Thu, 25 Jun 2026 21:13:53 GMT"},
#    {"content-length", "8"},
#    {"vary", "accept-encoding"},
#    {"cache-control", "max-age=0, private, must-revalidate"}
#  ], "/foo+bar"}

[frerich]

Thanks for working on this!

Would it be sensible to also look at escaping the host name correctly? As it is, Hackney automatically Punycode-encodes it, but Req doesn't:

iex(5)> :hackney.get(~c"http://lüthje.com")
{:ok, 301,
 [
   {"Content-Type", "text/html; charset=UTF-8"},
   {"Content-Length", "0"},
   {"Connection", "keep-alive"},
   {"X-WS-Origin", "available"},
   {"X-WS-RateLimit-Limit", "1000"},
   {"X-WS-RateLimit-Remaining", "999"},
   {"server", "Apache"},
   {"location", "https://www.xn--lthje-kva.com/"},
   {"x-redirect-by", "WordPress"},
   {"date", "Mon, 29 Jun 2026 17:30:16 GMT"},
   {"vary", "Cookie"},
   {"x-pingback", "http://www.xn--lthje-kva.com/xmlrpc.php"}
 ], #Reference<0.1402623956.2774007809.204041>}
iex(6)> Reg.get("http://lüthje.com")
** (UndefinedFunctionError) function Reg.get/1 is undefined (module Reg is not available). Make sure the module name is correct and has been specified in full (or that an alias has been defined)
    Reg.get("http://lüthje.com")
    iex:7: (file)
iex(7)> Req.get("http://lüthje.com")
** (exit) :badarg
    (finch 0.23.0) lib/finch/http1/pool.ex:93: Finch.HTTP1.Pool.request/6
    (finch 0.23.0) lib/finch.ex:829: Finch.__stream__/6
    (finch 0.23.0) lib/finch.ex:898: anonymous fn/4 in Finch.request/3
    (telemetry 1.4.2) /Users/frerich/src/bd/medhub/deps/telemetry/src/telemetry.erl:359: :telemetry.span/3
    (req 0.6.2) lib/req/finch.ex:277: Req.Finch.run_finch_request/3
    (req 0.6.2) lib/req/finch.ex:71: Req.Finch.run/4
    (req 0.6.2) lib/req/request.ex:1118: Req.Request.run_request/1
    (req 0.6.2) lib/req/request.ex:1062: Req.Request.run/1

This encoding would also need to be applied to redirects, I guess.

[frerich]

I wonder: does this do the right thing for absolute redirects, in case the target domain contains characters to be escaped? Consider e.g. the redirect returned by http://www.praxis-luethje-aichhorn.de

I suspect it would percent-encode the host name, which would be wrong.

Instead, perhaps it should encode the host name to 'Punycode', e.g. using :idna.encode/1. Note however that :idna.encode/1 will raise in case the returned location cannot be encoded (e.g. https://www,foo.com).

[jswanner]

Thanks for the review, @frerich. My plan is to change this PR such that it proposes that Req behave like hackney with regard to escaping request URLs

[frerich]

That sounds great! Two thoughts on this:

• Would it be desirable if whatever URLs are given by the user (e.g. in a Req.get/1 call) are encoded the same way URLs returned by servers as part of redirect responses? That's the case right now, and I think the consistency here is fairly nice.
• Req (in fact: Finch) currently raises when passing a non-HTTP (or HTTPS) URL, or when the host portion of the given URL is empty. Should the same requirement be applied to URLs returned in redirect responses?

[frerich]

After thinking some more about this:

• I think it's safe to have Req always encode user-provided URLs to Punycode, such that Req.get("http://lüthje.com") works just like :hackney.get(~c"http://lüthje.com") does. This means that only the path should be subject to URI.encode/1.
• The URL provided by a server via a location response header however should be ASCII clean according to https://stackoverflow.com/a/7654605 -- however, perhaps there is no harm in unconditionally converting to Punycode to deal with misconfigured servers?

[jswanner]

@wojtekmach, I've changed intent of this PR. It now adds a request step to escape the request URL, if needed. It's largely based on how hackney "normalizes" URLs before a request is made.

If you are onboard with this idea then I will add documentation for the request step function. This PR also adds :idna as a dependency for punycode encoding, which I can understand may not be desirable.

Also, I wasn't sure about what to call the step, other options I debated: :encode_url & :normalize_url.

[frerich]

Oh, nice request steps are considered when following redirects? I see you added a test for that. 👍

In my local tests, a response step was needed for that, i.e. a response step which encodes the location URL, if any. I must have done something wrong.

[jswanner]

In my local tests, a response step was needed for that, i.e. a response step which encodes the location URL, if any. I must have done something wrong.

I'm pretty sure there's an unreleased change that fixes what you were running into

[frerich]

On the naming of this step: seeing how URI.encode/1 and :idna.encode/1 are used, maybe calling this step encode_url instead of escape_url would be more consistent.

[frerich]

:idna.encode/1 bails out for disallowed codepoints in the given host, e.g.

iex(1)> :idna.encode(~c"www.elixir-lang,org")
** (exit) {:bad_label, {:alabel, ~c"The label \"elixir-lang,org\"  is not a valid A-label: ulabel error={bad_label,\n                                                                   {context,\n                                                                    \"Codepoint 44 not allowed ('DISALLOWED') at position 11 in \\\"elixir-lang,org\\\"\"}}"}}

Should the code maybe guard against that and raise a "invalid URL host" error as above?

[frerich]

@jswanner For what it's worth, I've been working on a similar feature for a non-public project and ended up with a Req plugin much like this: https://gist.github.com/frerich/2d4131eef039faf6586185601a81c0b3

Maybe that can serve as a source of inspiration here.

[jswanner]

@frerich I've cleaned it up a bit, as it did need to be refactored. I want to make sure Wojtek is onboard with the general premise before going much further with it.