Add request step to escape request URLs

See #430.

Author: jswanner

lib/req/steps.ex

@@ -35,6 +35,7 @@ defmodule Req.Steps do
       :compress_body,
       :checksum,
       :aws_sigv4,
+      :escape_url,
 
       # response steps
       :raw,
@@ -85,7 +86,8 @@ defmodule Req.Steps do
       put_plug: &Req.Steps.put_plug/1,
       compress_body: &Req.Steps.compress_body/1,
       checksum: &Req.Steps.checksum/1,
-      put_aws_sigv4: &Req.Steps.put_aws_sigv4/1
+      put_aws_sigv4: &Req.Steps.put_aws_sigv4/1,
+      escape_url: &Req.Steps.escape_url/1
     )
     |> Req.Request.prepend_response_steps(
       retry: &Req.Steps.retry/1,
@@ -1321,6 +1323,86 @@ defmodule Req.Steps do
     end
   end
 
+  def escape_url(request) do
+    if Req.Request.get_option(request, :escape_url, true) do
+      request |> escape_host() |> escape_path() |> escape_query()
+    else
+      request
+    end
+  end
+
+  defp escape_host(request) when is_binary(request.url.host) do
+    with {:ok, host} <- decode_host_and_ensure_not_encoded_ip(request.url.host),
+         {:ok, host} <- encode_host(host, request.url.scheme) do
+      put_in(request.url.host, host)
+    else
+      :error -> raise ArgumentError, "invalid URL host: #{inspect(request.url.host)}"
+    end
+  end
+
+  defp escape_host(request), do: request
+
+  defp decode_host_and_ensure_not_encoded_ip(host) do
+    case :inet.parse_address(to_charlist(host)) do
+      {:ok, _} ->
+        {:ok, host}
+
+      {:error, _} ->
+        with decoded <- URI.decode(host),
+             {:error, _} <- :inet.parse_address(to_charlist(decoded)) do
+          {:ok, decoded}
+        else
+          # a non-IP host decoded to an IP address, this kind of scenario was a
+          # vulnerability in hackney: GHSA-pj7v
+          {:ok, _encoded_ip} -> :error
+        end
+    end
+  end
+
+  defp encode_host(host, "http+unix"), do: {:ok, host}
+
+  defp encode_host(host, _scheme) do
+    if host |> to_charlist |> List.ascii_printable?() do
+      {:ok, host}
+    else
+      {:ok, host |> :idna.encode() |> to_string()}
+    end
+  catch
+    :exit, {:bad_label, _} -> :error
+  end
+
+  defp escape_path(request) when is_binary(request.url.path) do
+    if escape_segment?(request.url.path) do
+      update_in(request.url.path, &URI.encode/1)
+    else
+      request
+    end
+  end
+
+  defp escape_path(request), do: request
+
+  defp escape_query(request) when is_binary(request.url.query) do
+    if escape_segment?(request.url.query) do
+      update_in(request.url.query, &URI.encode/1)
+    else
+      request
+    end
+  end
+
+  defp escape_query(request), do: request
+
+  defp escape_segment?(<<?%, octet::binary-size(2), rest::binary>>) do
+    case Base.decode16(octet) do
+      {:ok, _} -> escape_segment?(rest)
+      :error -> true
+    end
+  end
+
+  defp escape_segment?(<<?%, _rest::binary>>), do: true
+  defp escape_segment?(<<?\s, _rest::binary>>), do: true
+  defp escape_segment?(<<_, rest::binary>>), do: escape_segment?(rest)
+  defp escape_segment?(<<>>), do: false
+
   ## Response steps
 
   @doc """

mix.exs

@@ -93,6 +93,7 @@ defmodule Req.MixProject do
       {:finch, "~> 0.21", finch_opts()},
       {:mime, "~> 2.0.6 or ~> 2.1"},
       {:jason, "~> 1.0"},
+      {:idna, "~> 7.0"},
       {:nimble_csv, "~> 1.0", optional: true},
       {:plug, "~> 1.0", [optional: true] ++ plug_opts()},
       {:brotli, "~> 0.3.1", optional: true},

mix.lock

@@ -10,6 +10,7 @@
   "ex_doc": {:hex, :ex_doc, "0.40.2", "f50edec428c4b0a457a167de42414c461122a3585a99515a69d09fff19e5597e", [:mix], [{:earmark_parser, "~> 1.4.44", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "4fa426e2beb47854a162e2c488727fdec51cd4692e319b23810c2804cb1a40fe"},
   "finch": {:hex, :finch, "0.22.0", "5c48fa6f9706a78eb9036cacb67b8b996b4e66d111c543f4c29bb0f879a6806b", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.8", [hex: :mint, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.4 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:nimble_pool, "~> 1.1", [hex: :nimble_pool, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b94e83c47780fc6813f746a1f1a34ee65cda42da4c5ea26a68f0acc4498e23dc"},
   "hpax": {:hex, :hpax, "1.0.3", "ed67ef51ad4df91e75cc6a1494f851850c0bd98ebc0be6e81b026e765ee535aa", [:mix], [], "hexpm", "8eab6e1cfa8d5918c2ce4ba43588e894af35dbd8e91e6e55c817bca5847df34a"},
+  "idna": {:hex, :idna, "7.1.0", "1067a13043538129602d2f2ce6899d8713125c7d19734aa557ce2e3ea55bd4f1", [:rebar3], [], "hexpm", "6ae959a025bf36df61a8cab8508d9654891b5426a84c44d82deaffd6ddf8c71f"},
   "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"},
   "makeup": {:hex, :makeup, "1.2.1", "e90ac1c65589ef354378def3ba19d401e739ee7ee06fb47f94c687016e3713d1", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "d36484867b0bae0fea568d10131197a4c2e47056a6fbe84922bf6ba71c8d17ce"},
   "makeup_elixir": {:hex, :makeup_elixir, "1.0.1", "e928a4f984e795e41e3abd27bfc09f51db16ab8ba1aebdba2b3a575437efafc2", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "7284900d412a3e5cfd97fdaed4f5ed389b8f2b4cb49efc0eb3bd10e2febf9507"},

test/req/steps_test.exs

@@ -1026,6 +1026,101 @@ defmodule Req.StepsTest do
     end
   end
 
+  describe "escape_url" do
+    test "can be disabled" do
+      resp =
+        Req.get!(
+          escape_url: false,
+          plug: &Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)),
+          url: "http://lüthje.com/with space"
+        )
+
+      assert resp.body == "http://lüthje.com/with space"
+    end
+
+    test "escapes host" do
+      resp =
+        Req.get!(
+          plug: &Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)),
+          url: "http://lüthje.com/"
+        )
+
+      assert resp.body == "http://xn--lthje-kva.com/"
+    end
+
+    test "escapes percent encoded host" do
+      resp =
+        Req.get!(
+          plug: &Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)),
+          url:
+            "http://www.%E3%81%BB%E3%82%93%E3%81%A8%E3%81%86%E3%81%AB%E3%81%AA%E3%81%8C%E3%81%84%E3%82%8F%E3%81%91%E3%81%AE%E3%82%8F%E3%81%8B%E3%82%89%E3%81%AA%E3%81%84%E3%81%A9%E3%82%81%E3%81%84%E3%82%93%E3%82%81%E3%81%84%E3%81%AE%E3%82%89%E3%81%B9%E3%82%8B%E3%81%BE%E3%81%A0%E3%81%AA%E3%81%8C%E3%81%8F%E3%81%97%E3%81%AA%E3%81%84%E3%81%A8%E3%81%9F%E3%82%8A%E3%81%AA%E3%81%84.w3.mag.keio.ac.jp/"
+        )
+
+      assert resp.body ==
+               "http://www.xn--n8jaaaaai5bhf7as8fsfk3jnknefdde3fg11amb5gzdb4wi9bya3kc6lra.w3.mag.keio.ac.jp/"
+    end
+
+    test "escapes percent encoded unix socket" do
+      resp =
+        Req.get!(
+          plug:
+            &Plug.Conn.send_resp(
+              &1,
+              200,
+              [to_string(&1.scheme), "://", &1.host, &1.request_path, "?", &1.query_string]
+            ),
+          url: "http+unix://user@%2Fvar%2Frun%2Ftest.sock/path?key=value"
+        )
+
+      assert resp.body == "http+unix:///var/run/test.sock/path?key=value"
+    end
+
+    test "raises on non-IP host that decodes to an IP" do
+      assert_raise ArgumentError, ~r/invalid URL host: "%31%32%37%2E%30%2E%30%2E%31"/, fn ->
+        Req.get!(
+          plug: &Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)),
+          url: "http://%31%32%37%2E%30%2E%30%2E%31/"
+        )
+      end
+    end
+
+    test "escapes path" do
+      %{req: req, url: url} = serve(&Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)))
+      resp = Req.get!(req, url: "#{url}/with space")
+      assert resp.body == "#{url}/with%20space"
+    end
+
+    test "escapes unescaped path with %" do
+      %{req: req, url: url} = serve(&Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)))
+      resp = Req.get!(req, url: "#{url}/with%2percent")
+      assert resp.body == "#{url}/with%252percent"
+    end
+
+    test "keeps escaped path untouched" do
+      %{req: req, url: url} = serve(&Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)))
+      resp = Req.get!(req, url: "#{url}/with%20space")
+      assert resp.body == "#{url}/with%20space"
+    end
+
+    test "escapes query" do
+      %{req: req, url: url} = serve(&Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)))
+      resp = Req.get!(req, url: "#{url}/?with space")
+      assert resp.body == "#{url}/?with%20space"
+    end
+
+    test "escapes unescaped query with %" do
+      %{req: req, url: url} = serve(&Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)))
+      resp = Req.get!(req, url: "#{url}/?with%2percent")
+      assert resp.body == "#{url}/?with%252percent"
+    end
+
+    test "keeps escaped query untouched" do
+      %{req: req, url: url} = serve(&Plug.Conn.send_resp(&1, 200, Plug.Conn.request_url(&1)))
+      resp = Req.get!(req, url: "#{url}/?with%20space")
+      assert resp.body == "#{url}/?with%20space"
+    end
+  end
+
   ## Response steps
 
   describe "decompress_body" do
@@ -1733,6 +1828,32 @@ defmodule Req.StepsTest do
       end
     end
 
+    @tag :capture_log
+    test "escapes location" do
+      %{req: req, url: url} =
+        serve(fn
+          conn when conn.request_path == "/redirect" ->
+            location =
+              case conn.query_string do
+                "" -> "/oh hai?param=with spaces"
+                string -> "/oh hai?" <> string
+              end
+
+            redirect(conn, 302, location)
+
+          conn ->
+            Plug.Conn.send_resp(conn, 200, Plug.Conn.request_url(conn))
+        end)
+
+      response = Req.get!(req, url: "#{url}/redirect")
+      assert response.status == 200
+      assert response.body == "#{url}/oh%20hai?param=with%20spaces"
+
+      response = Req.get!(req, url: "#{url}/redirect?a=already%20encoded")
+      assert response.status == 200
+      assert response.body == "#{url}/oh%20hai?a=already%20encoded"
+    end
+
     test "without location" do
       %{req: req, url: url} =
         serve(fn conn ->