diff --git a/CHANGELOG.md b/CHANGELOG.md index 0344b190..afb842fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ ## HEAD + * Change `request.headers` and `response.headers` to be maps. + + This is a major breaking change. If you cannot easily update your app + or your dependencies, do: + + # config/config.exs + config :req, legacy_headers_as_lists: true + + This legacy fallback will be removed on Req 1.0. + + * Make `request.registered_options` internal representation private. + + * Make `request.options` internal representation private. + + Currently `request.options` field is a map but it may change in the future. + One possible future change is using keywords lists internally which would + allow, for example, `Req.new(params: [a: 1]) |> Req.update(params: [b: 2])` + to keep duplicate `:params` in `request.options` which would then allow to + decide the duplicate key semantics on a per-step basis. And so, for example, + [`put_params`] would _merge_ params but most steps would simply use the + first value. + + To have some room for manoeuvre in the future we should stop pattern + matching on `request.options`. Calling `request.options[key]`, + `put_in(request.options[key], value)`, and + `update_in(request.options[key], fun)` _is_ allowed. + New functions [`Req.Request.get_option/3`], + [`Req.Request.fetch_option!/2`], and [`Req.Request.delete_option/1`] have been + added for additional ways to manipulate the internal representation. + * Fix typespecs for some functions * [`decompress_body`]: Remove support for `deflate` compression @@ -33,26 +63,6 @@ * [`Req.Request`]: Fix displaying redacted basic authentication - * Make `request.registered_options` internal representation private. - - * Make `request.options` internal representation private. - - Currently `request.options` field is a map but it may change in the future. - One possible future change is using keywords lists internally which would - allow, for example, `Req.new(params: [a: 1]) |> Req.update(params: [b: 2])` - to keep duplicate `:params` in `request.options` which would then allow to - decide the duplicate key semantics on a per-step basis. And so, for example, - [`put_params`] would _merge_ params but most steps would simply use the - first value. - - To have some room for manoeuvre in the future we should stop pattern - matching on `request.options`. Calling `request.options[key]`, - `put_in(request.options[key], value)`, and - `update_in(request.options[key], fun)` _is_ allowed. - New functions [`Req.Request.get_option/3`], - [`Req.Request.fetch_option!/2`], and [`Req.Request.delete_option/1`] have been - added for additional ways to manipulate the internal representation. - ## v0.3.11 (2023-07-24) * Support `Req.get(options)`, `Req.post(options)`, etc diff --git a/lib/req.ex b/lib/req.ex index 496c2d22..eca31b2d 100644 --- a/lib/req.ex +++ b/lib/req.ex @@ -373,7 +373,13 @@ defmodule Req do {:headers, new_headers}, acc -> update_in(acc.headers, fn old_headers -> - Map.merge(old_headers, encode_headers(new_headers)) + if unquote(Req.MixProject.headers_as_lists?()) do + new_headers = encode_headers(new_headers) + new_header_names = Enum.map(new_headers, &elem(&1, 0)) + Enum.reject(old_headers, &(elem(&1, 0) in new_header_names)) ++ new_headers + else + Map.merge(old_headers, encode_headers(new_headers)) + end end) {name, value}, acc -> @@ -941,23 +947,31 @@ defmodule Req do Application.put_env(:req, :default_options, options) end - defp encode_headers(headers) do - Enum.reduce(headers, %{}, fn {name, value}, acc -> - Map.update( - acc, - encode_header_name(name), - encode_header_values(List.wrap(value)), - &(&1 ++ encode_header_values(List.wrap(value))) - ) - end) - end + if Req.MixProject.headers_as_lists?() do + defp encode_headers(headers) do + for {name, value} <- headers do + {encode_header_name(name), encode_header_value(value)} + end + end + else + defp encode_headers(headers) do + Enum.reduce(headers, %{}, fn {name, value}, acc -> + Map.update( + acc, + encode_header_name(name), + encode_header_values(List.wrap(value)), + &(&1 ++ encode_header_values(List.wrap(value))) + ) + end) + end - defp encode_header_values([value | rest]) do - [encode_header_value(value) | encode_header_values(rest)] - end + defp encode_header_values([value | rest]) do + [encode_header_value(value) | encode_header_values(rest)] + end - defp encode_header_values([]) do - [] + defp encode_header_values([]) do + [] + end end defp encode_header_name(name) when is_atom(name) do diff --git a/lib/req/request.ex b/lib/req/request.ex index a512e6fb..f88f0b55 100644 --- a/lib/req/request.ex +++ b/lib/req/request.ex @@ -338,7 +338,7 @@ defmodule Req.Request do defstruct method: :get, url: URI.parse(""), - headers: %{}, + headers: if(Req.MixProject.headers_as_lists?(), do: [], else: %{}), body: nil, options: %{}, halted: false, @@ -368,25 +368,37 @@ defmodule Req.Request do ## Examples iex> req = Req.Request.new(url: "https://api.github.com/repos/wojtekmach/req") - iex> {request, response} = Req.Request.run_request(req) - iex> request.url.host + iex> {req, resp} = Req.Request.run_request(req) + iex> req.url.host "api.github.com" - iex> response.status + iex> resp.status 200 """ - def new(options) do - options = - options - |> Keyword.validate!([:method, :url, :headers, :body, :adapter, :options]) - |> Keyword.update(:url, URI.new!(""), &URI.new!/1) - |> Keyword.update(:headers, %{}, fn headers -> - Map.new(headers, fn {key, value} -> - {key, List.wrap(value)} + if Req.MixProject.headers_as_lists?() do + def new(options) do + options = + options + |> Keyword.validate!([:method, :url, :headers, :body, :adapter, :options]) + |> Keyword.update(:url, URI.new!(""), &URI.new!/1) + |> Keyword.update(:options, %{}, &Map.new/1) + + struct!(__MODULE__, options) + end + else + def new(options) do + options = + options + |> Keyword.validate!([:method, :url, :headers, :body, :adapter, :options]) + |> Keyword.update(:url, URI.new!(""), &URI.new!/1) + |> Keyword.update(:headers, %{}, fn headers -> + Map.new(headers, fn {key, value} -> + {key, List.wrap(value)} + end) end) - end) - |> Keyword.update(:options, %{}, &Map.new/1) + |> Keyword.update(:options, %{}, &Map.new/1) - struct!(__MODULE__, options) + struct!(__MODULE__, options) + end end @doc """ @@ -664,8 +676,16 @@ defmodule Req.Request do """ @spec get_header(t(), binary()) :: [binary()] - def get_header(%Req.Request{} = request, key) when is_binary(key) do - Map.get(request.headers, key, []) + if Req.MixProject.headers_as_lists?() do + def get_header(%Req.Request{} = request, key) when is_binary(key) do + for {^key, value} <- request.headers do + value + end + end + else + def get_header(%Req.Request{} = request, key) when is_binary(key) do + Map.get(request.headers, key, []) + end end @doc """ @@ -684,16 +704,24 @@ defmodule Req.Request do ## Examples iex> req = Req.new() + iex> Req.Request.get_header(req, "accept") + [] iex> req = Req.Request.put_header(req, "accept", "application/json") - iex> req.headers - %{"accept" => ["application/json"]} + iex> Req.Request.get_header(req, "accept") + ["application/json"] """ @spec put_header(t(), binary(), binary()) :: t() - def put_header(%Req.Request{} = request, key, value) - when is_binary(key) and - (is_binary(value) or is_list(value)) do - put_in(request.headers[key], List.wrap(value)) + if Req.MixProject.headers_as_lists?() do + def put_header(%Req.Request{} = request, key, value) + when is_binary(key) and is_binary(value) do + %{request | headers: List.keystore(request.headers, key, 0, {key, value})} + end + else + def put_header(%Req.Request{} = request, key, value) + when is_binary(key) and (is_binary(value) or is_list(value)) do + put_in(request.headers[key], List.wrap(value)) + end end @doc """ @@ -705,8 +733,10 @@ defmodule Req.Request do iex> req = Req.new() iex> req = Req.Request.put_headers(req, [{"accept", "text/html"}, {"accept-encoding", "gzip"}]) - iex> req.headers - %{"accept" => ["text/html"], "accept-encoding" => ["gzip"]} + iex> Req.Request.get_header(req, "accept") + ["text/html"] + iex> Req.Request.get_header(req, "accept-encoding") + ["gzip"] """ @spec put_headers(t(), [{binary(), binary()}]) :: t() def put_headers(%Req.Request{} = request, headers) do @@ -726,13 +756,35 @@ defmodule Req.Request do ...> Req.new() ...> |> Req.Request.put_new_header("accept", "application/json") ...> |> Req.Request.put_new_header("accept", "application/html") - iex> req.headers - %{"accept" => ["application/json"]} + iex> Req.Request.get_header(req, "accept") + ["application/json"] """ @spec put_new_header(t(), binary(), binary()) :: t() - def put_new_header(%Req.Request{} = request, key, value) - when is_binary(key) and (is_binary(value) or is_list(value)) do - update_in(request.headers, &Map.put_new(&1, key, List.wrap(value))) + if Req.MixProject.headers_as_lists?() do + def put_new_header(%Req.Request{} = request, key, value) do + case get_header(request, key) do + [] -> + put_header(request, key, value) + + _ -> + request + end + end + else + def put_new_header(%Req.Request{} = request, key, value) + when is_binary(key) and (is_binary(value) or is_list(value)) do + update_in(request.headers, &Map.put_new(&1, key, List.wrap(value))) + end + end + + if Req.MixProject.headers_as_lists?() do + def delete_header(%Req.Request{} = request, key) when is_binary(key) do + %{request | headers: List.keydelete(request.headers, key, 0)} + end + else + def delete_header(%Req.Request{} = request, key) when is_binary(key) do + update_in(request.headers, &Map.delete(&1, key)) + end end @doc """ @@ -914,19 +966,23 @@ defmodule Req.Request do {headers, options} = if Req.Request.get_option(request, :redact_auth, true) do headers = - case request.headers do - headers when is_map(headers) -> - for {name, values} <- request.headers, into: %{} do - if name in ["authorization", "Authorization"] do - [_] = values - {name, ["[redacted]"]} - else - {name, values} - end + if Req.MixProject.headers_as_lists?() do + for {name, value} <- request.headers do + if name in ["authorization", "Authorization"] do + {name, "[redacted]"} + else + {name, value} end - - # TODO: headers are always a map on Req v1.0 - # + end + else + for {name, values} <- request.headers, into: %{} do + if name in ["authorization", "Authorization"] do + [_] = values + {name, ["[redacted]"]} + else + {name, values} + end + end end options = diff --git a/lib/req/response.ex b/lib/req/response.ex index fbd5b871..6e66e75c 100644 --- a/lib/req/response.ex +++ b/lib/req/response.ex @@ -23,7 +23,7 @@ defmodule Req.Response do } defstruct status: 200, - headers: %{}, + headers: if(Req.MixProject.headers_as_lists?(), do: [], else: %{}), body: "", private: %{} @@ -47,20 +47,27 @@ defmodule Req.Response do def new(options) when is_list(options), do: new(Map.new(options)) - def new(options) do - options = - Map.take(options, [:status, :headers, :body]) - |> Map.update(:headers, %{}, fn - map when is_map(map) -> - map - - list when is_list(list) -> - Enum.reduce(list, %{}, fn {name, value}, acc -> - Map.update(acc, name, [value], &(&1 ++ [value])) - end) - end) - - struct!(__MODULE__, options) + if Req.MixProject.headers_as_lists?() do + def new(options) do + options = Map.take(options, [:status, :headers, :body]) + struct!(__MODULE__, options) + end + else + def new(options) do + options = + Map.take(options, [:status, :headers, :body]) + |> Map.update(:headers, %{}, fn + map when is_map(map) -> + map + + list when is_list(list) -> + Enum.reduce(list, %{}, fn {name, value}, acc -> + Map.update(acc, name, [value], &(&1 ++ [value])) + end) + end) + + struct!(__MODULE__, options) + end end @doc """ @@ -133,8 +140,16 @@ defmodule Req.Response do ["application/json"] """ @spec get_header(t(), binary()) :: [binary()] - def get_header(%Req.Response{} = response, key) when is_binary(key) do - Map.get(response.headers, key, []) + if Req.MixProject.headers_as_lists?() do + def get_header(%Req.Response{} = response, key) when is_binary(key) do + for {^key, value} <- response.headers do + value + end + end + else + def get_header(%Req.Response{} = response, key) when is_binary(key) do + Map.get(response.headers, key, []) + end end @doc """ @@ -148,9 +163,15 @@ defmodule Req.Response do """ @spec put_header(t(), binary(), binary() | [binary()]) :: t() - def put_header(%Req.Response{} = response, key, value) - when is_binary(key) and - (is_binary(value) or is_list(value)) do - put_in(response.headers[key], List.wrap(value)) + if Req.MixProject.headers_as_lists?() do + def put_header(%Req.Response{} = response, key, value) + when is_binary(key) and is_binary(value) do + %{response | headers: List.keystore(response.headers, key, 0, {key, value})} + end + else + def put_header(%Req.Response{} = response, key, value) + when is_binary(key) and (is_binary(value) or is_list(value)) do + put_in(response.headers[key], List.wrap(value)) + end end end diff --git a/lib/req/steps.ex b/lib/req/steps.ex index 793dc9f0..964fdd2c 100644 --- a/lib/req/steps.ex +++ b/lib/req/steps.ex @@ -649,9 +649,13 @@ defmodule Req.Steps do finch_name = finch_name(request) request_headers = - for {name, values} <- request.headers, - value <- values do - {name, value} + if unquote(Req.MixProject.headers_as_lists?()) do + request.headers + else + for {name, values} <- request.headers, + value <- values do + {name, value} + end end finch_request = @@ -828,9 +832,13 @@ defmodule Req.Steps do end req_headers = - for {name, values} <- request.headers, - value <- values do - {name, value} + if unquote(Req.MixProject.headers_as_lists?()) do + request.headers + else + for {name, values} <- request.headers, + value <- values do + {name, value} + end end conn = @@ -918,7 +926,14 @@ defmodule Req.Steps do {request, response} else algorithms = compression_algorithms(Req.Response.get_header(response, "content-encoding")) - {request, update_in(response.body, &decompress_body(&1, algorithms))} + decompressed_body = decompress_body(response.body, algorithms) + decompressed_content_length = decompressed_body |> byte_size() |> to_string() + + response = + %Req.Response{response | body: decompressed_body} + |> Req.Response.put_header("content-length", decompressed_content_length) + + {request, response} end end @@ -1125,7 +1140,7 @@ defmodule Req.Steps do end defp format(request, response) do - with [content_type] <- response.headers["content-type"] do + with [content_type] <- Req.Response.get_header(response, "content-type") do # TODO: remove ` || ` when we require Elixir v1.13 path = request.url.path || "" @@ -1239,7 +1254,7 @@ defmodule Req.Steps do end defp build_redirect_request(request, response) do - [location] = response.headers["location"] + [location] = Req.Response.get_header(response, "location") log_level = Req.Request.get_option(request, :redirect_log_level, :debug) log_redirect(log_level, location) @@ -1285,7 +1300,7 @@ defmodule Req.Steps do end defp remove_credentials(request) do - request = update_in(request.headers, &Map.delete(&1, "authorization")) + request = Req.Request.delete_header(request, "authorization") request = Req.Request.delete_option(request, :auth) request end diff --git a/mix.exs b/mix.exs index 8ca92adf..6b9bb696 100644 --- a/mix.exs +++ b/mix.exs @@ -81,4 +81,8 @@ defmodule Req.MixProject do ] ] end + + def headers_as_lists? do + Application.get_env(:req, :legacy_headers_as_lists, false) + end end diff --git a/test/req/request_test.exs b/test/req/request_test.exs index 9b9529c2..fc1e9720 100644 --- a/test/req/request_test.exs +++ b/test/req/request_test.exs @@ -242,11 +242,19 @@ defmodule Req.RequestTest do authorization = "Basic " <> Base.encode64("foo:bar") - assert %{ - "user-agent" => ["req/" <> _], - "accept-encoding" => ["zstd, br, gzip"], - "authorization" => [^authorization] - } = request.headers + if Req.MixProject.headers_as_lists?() do + assert [ + {"user-agent", "req/0.3.11"}, + {"accept-encoding", "zstd, br, gzip"}, + {"authorization", ^authorization} + ] = request.headers + else + assert %{ + "user-agent" => ["req/" <> _], + "accept-encoding" => ["zstd, br, gzip"], + "authorization" => [^authorization] + } = request.headers + end end ## Helpers diff --git a/test/req/steps_test.exs b/test/req/steps_test.exs index 7a691da1..24c76d98 100644 --- a/test/req/steps_test.exs +++ b/test/req/steps_test.exs @@ -50,19 +50,19 @@ defmodule Req.StepsTest do test "string" do req = Req.new(auth: "foo") |> Req.Request.prepare() - assert req.headers["authorization"] == ["foo"] + assert Req.Request.get_header(req, "authorization") == ["foo"] end test "basic" do req = Req.new(auth: {"foo", "bar"}) |> Req.Request.prepare() - assert req.headers["authorization"] == ["Basic #{Base.encode64("foo:bar")}"] + assert Req.Request.get_header(req, "authorization") == ["Basic #{Base.encode64("foo:bar")}"] end test "bearer" do req = Req.new(auth: {:bearer, "abcd"}) |> Req.Request.prepare() - assert req.headers["authorization"] == ["Bearer abcd"] + assert Req.Request.get_header(req, "authorization") == ["Bearer abcd"] end @tag :tmp_dir @@ -238,10 +238,10 @@ defmodule Req.StepsTest do test "put_range" do req = Req.new(range: "bytes=0-10") |> Req.Request.prepare() - assert req.headers["range"] == ["bytes=0-10"] + assert Req.Request.get_header(req, "range") == ["bytes=0-10"] req = Req.new(range: 0..20) |> Req.Request.prepare() - assert req.headers["range"] == ["bytes=0-20"] + assert Req.Request.get_header(req, "range") == ["bytes=0-20"] end describe "compress_body" do @@ -251,7 +251,7 @@ defmodule Req.StepsTest do req = Req.new(method: :post, json: %{a: 1}, compress_body: true) |> Req.Request.prepare() assert :zlib.gunzip(req.body) |> Jason.decode!() == %{"a" => 1} - assert req.headers["content-encoding"] == ["gzip"] + assert Req.Request.get_header(req, "content-encoding") == ["gzip"] end test "stream", c do @@ -351,7 +351,7 @@ defmodule Req.StepsTest do end) resp = Req.get!(c.url) - assert resp.headers["content-encoding"] == ["unknown"] + assert Req.Response.get_header(resp, "content-encoding") == ["unknown"] assert resp.body == <<1, 2, 3>> end @@ -705,19 +705,19 @@ defmodule Req.StepsTest do adapter = fn request -> case request.url.host do "original" -> - assert request.headers["authorization"] + assert [_] = Req.Request.get_header(request, "authorization") response = Req.Response.new( status: 301, - headers: %{"location" => ["http://untrusted"]}, + headers: [{"location", "http://untrusted"}], body: "redirecting" ) {request, response} "untrusted" -> - assert request.headers["authorization"] + assert [_] = Req.Request.get_header(request, "authorization") response = Req.Response.new( @@ -885,7 +885,7 @@ defmodule Req.StepsTest do fn request -> case Map.get(request.url, component) do ^original_value -> - assert request.headers["authorization"] + assert [_] = Req.Request.get_header(request, "authorization") new_url = request.url @@ -895,14 +895,14 @@ defmodule Req.StepsTest do response = Req.Response.new( status: 301, - headers: %{"location" => [new_url]}, + headers: [{"location", new_url}], body: "redirecting" ) {request, response} ^updated_value -> - refute request.headers["authorization"] + assert [] = Req.Request.get_header(request, "authorization") response = Req.Response.new( diff --git a/test/req_test.exs b/test/req_test.exs index 21888965..82560364 100644 --- a/test/req_test.exs +++ b/test/req_test.exs @@ -35,7 +35,11 @@ defmodule ReqTest do assert headers == [{"x-a", "1"}, {"x-b", "Fri, 01 Jan 2021 09:00:00 GMT"}] req = Req.new(headers: [x_a: 1, x_a: 2]) - assert req.headers == %{"x-a" => ["1", "2"]} + + unless Req.MixProject.headers_as_lists?() do + assert req.headers == %{"x-a" => ["1", "2"]} + end + Req.get!(req, url: c.url) assert_receive {:headers, headers} assert headers == [{"x-a", "1, 2"}] @@ -51,7 +55,12 @@ defmodule ReqTest do assert inspect(Req.new(auth: {"foo", "bar"})) =~ ~s|auth: {"[redacted]", "[redacted]"}| - assert inspect(Req.new(headers: [authorization: "bearer foo"])) =~ - ~s|"authorization" => ["[redacted]"]| + if Req.MixProject.headers_as_lists?() do + assert inspect(Req.new(headers: [authorization: "bearer foo"])) =~ + ~s|{"authorization", "[redacted]"}| + else + assert inspect(Req.new(headers: [authorization: "bearer foo"])) =~ + ~s|"authorization" => ["[redacted]"]| + end end end