Implemented validation on request header keys on Req.new

lib/req.ex

@@ -30,7 +30,6 @@ defmodule Req do
       iex> Req.post!("https://httpbin.org/post", form: [comments: "hello!"]).body["form"]
       %{"comments" => "hello!"}
   """
-
   @type url() :: URI.t() | String.t()
 
   @doc """
@@ -55,7 +54,12 @@ defmodule Req do
 
         * string header names are left as is. Because header keys are case-insensitive
           in both HTTP/1.1 and HTTP/2, it is recommended for header keys to be in
-          lowercase, to avoid sending duplicate keys in a request.
+          lowercase, to avoid sending duplicate keys in a request. This can be controlled
+          by an additional option, `:invalid_header_keys`. This option will have the
+          following values
+            * `:warn` - the default if the option is omitted
+            * `:raise` - raises a `RuntimeError` is a header key is invalid
+            * `:ignore` - ignores checking the header keys for validity
 
         * `DateTime` header values are encoded as "HTTP date". Otherwise,
           the header value is encoded with `String.Chars.to_string/1`.
@@ -64,6 +68,15 @@ defmodule Req do
 
     * `:body` - the request body.
 
+  Request header options:
+
+    * `:invalid_header_keys`. This option will have the following values which are used to validate the
+      keys of the request headers;
+
+            * `:warn` - the default if the option is omitted
+            * `:raise` - raises a `RuntimeError` is a header key is invalid
+            * `:ignore` - ignores checking the header keys for validity
+
   Additional URL options:
 
     * `:base_url` - if set, the request URL is prepended with this base URL (via
@@ -250,7 +263,8 @@ defmodule Req do
           :connect_options,
           :receive_timeout,
           :pool_timeout,
-          :unix_socket
+          :unix_socket,
+          :invalid_header_keys
         ])
     }
     |> update(options)
@@ -316,7 +330,8 @@ defmodule Req do
 
     request_options =
       if request_options[:headers] do
-        update_in(request_options[:headers], &encode_headers/1)
+        invalid_key_action = Keyword.get(options, :invalid_header_keys, :warn)
+        update_in(request_options[:headers], &encode_headers(&1, invalid_key_action))
       else
         request_options
       end
@@ -1002,7 +1017,7 @@ defmodule Req do
     Application.put_env(:req, :default_options, options)
   end
 
-  defp encode_headers(headers) do
+  defp encode_headers(headers, invalid_key_action) do
     for {name, value} <- headers do
       name =
         case name do
@@ -1013,6 +1028,8 @@ defmodule Req do
             binary
         end
 
+      Req.Request.validate_header_key(name, invalid_key_action)
+
       value =
         case value do
           %DateTime{} = datetime ->

lib/req/request.ex

@@ -307,6 +307,8 @@ defmodule Req.Request do
       #=> "Req is a batteries-included HTTP client for Elixir."
   """
 
+  require Logger
+
   @type t() :: %Req.Request{
           method: atom(),
           url: URI.t(),
@@ -590,6 +592,15 @@ defmodule Req.Request do
   it is recommended for header keys to be in lowercase, to avoid sending
   duplicate keys in a request.
 
+  Request header options which may be set on `Req.new/1`:
+
+    * `:invalid_header_keys`. This option will have the following values which are used to validate the
+      keys of the request headers;
+
+      * `:warn` - the default if the option is omitted
+      * `:raise` - raises a `RuntimeError` is a header key is invalid
+      * `:ignore` - ignores checking the header keys for validity
+
   Additionally, requests with mixed-case headers served over HTTP/2 are not
   considered valid by common clients, resulting in dropped requests.
 
@@ -600,10 +611,50 @@ defmodule Req.Request do
       iex> req.headers
       [{"accept", "application/json"}]
 
+  Passing no value for the `:invalid_header_keys` options prints a warning
+  [warning] Request header "Accept" is not lowercase.
+
+      iex> req = Req.new()
+      iex> req = Req.Request.put_header(req, "Accept", "application/json")
+      iex> req.headers
+      [{"Accept", "application/json"}]
+
+  Passing :warn for the `:invalid_header_keys` options prints a warning
+  [warning] Request header "Accept" is not lowercase.
+
+      iex> req = Req.new()
+      iex> req = Req.Request.put_header(req, "Accept", "application/json")
+      iex> req.headers
+      [{"Accept", "application/json"}]
+
+  Passing :warn for the `:invalid_header_keys` options prints a warning
+  [warning] Request header "Accept" is not lowercase.
+
+      iex> req = Req.new(invalid_header_keys: :warn)
+      iex> req = Req.Request.put_header(req, "Accept", "application/json")
+      iex> req.headers
+      [{"Accept", "application/json"}]
+
+  Passing :ignore for the `:invalid_header_keys` options doesn't print a warning
+
+      iex> req = Req.new(invalid_header_keys: :warn)
+      iex> req = Req.Request.put_header(req, "Accept", "application/json")
+      iex> req.headers
+      [{"Accept", "application/json"}]
+
+  Passing :raise for the `:invalid_header_keys` raises a RuntimeError
+
+      iex> req = Req.new(invalid_header_keys: :raise)
+      iex> req = Req.Request.put_header(req, "Accept", "application/json")
+      iex> req.headers
+      ** (RuntimeError) The request header Accept is invalid due to not being lowercase.
   """
   @spec put_header(t(), binary(), binary()) :: t()
   def put_header(%Req.Request{} = request, key, value)
       when is_binary(key) and is_binary(value) do
+    invalid_key_action = Map.get(request.options, :invalid_header_keys, :warn)
+    validate_header_key(key, invalid_key_action)
+
     %{request | headers: List.keystore(request.headers, key, 0, {key, value})}
   end
 
@@ -791,6 +842,12 @@ defmodule Req.Request do
     validate_options(options, request.registered_options)
   end
 
+  def validate_options([{:invalid_header_keys, value} | _rest], _registered)
+      when value not in [:warn, :raise, :ignore] do
+    raise ArgumentError,
+          "Value #{inspect(value)} not valid for option #{inspect(:invalid_header_keys)}"
+  end
+
   def validate_options([{name, _value} | rest], registered) do
     if name in registered do
       validate_options(rest, registered)
@@ -820,6 +877,23 @@ defmodule Req.Request do
     if score < current, do: best, else: {option, score}
   end
 
+  @doc false
+  def validate_header_key(key, invalid_key_action) do
+    if Regex.match?(~r/[A-Z]./, key) do
+      case invalid_key_action do
+        :raise ->
+          raise ~s(The request header #{key} is invalid due to not being lowercase.)
+
+        :warn ->
+          message = ~s(Request header #{inspect(key)} is not lowercase.)
+          Logger.warning(message)
+
+        :ignore ->
+          :noop
+      end
+    end
+  end
+
   defimpl Inspect do
     import Inspect.Algebra
 

test/req_test.exs

@@ -1,6 +1,8 @@
 defmodule ReqTest do
   use ExUnit.Case, async: true
 
+  import ExUnit.CaptureLog
+
   doctest Req,
     only: [
       new: 1,
@@ -46,6 +48,115 @@ defmodule ReqTest do
     assert headers == [{"x-a", "1, 2"}]
   end
 
+  test "validation header raises if option was not supported",
+       c do
+    headers = [{:x_Invalid, "any value"}, {"NotValid", "whoops!"}]
+
+    assert_raise(ArgumentError, "Value :anything not valid for option :invalid_header_keys", fn ->
+      Req.new(invalid_header_keys: :anything, url: c.url, headers: headers)
+      |> Req.get!()
+    end)
+  end
+
+  test "header validation defaults to a warning if invalid header key is not sent", c do
+    pid = self()
+
+    Bypass.expect(c.bypass, "GET", "/", fn conn ->
+      headers =
+        Enum.filter(conn.req_headers, fn {name, _} -> String.contains?(name, ["foo", "bar"]) end)
+
+      send(pid, {:headers, headers})
+      Plug.Conn.send_resp(conn, 200, "ok")
+    end)
+
+    headers = [{:x_Foo, "some value"}, {"Bar", 911}]
+
+    {_result, log} =
+      with_log(fn ->
+        Req.new(url: c.url, headers: headers)
+        |> Req.get!()
+      end)
+
+    expected_headers = [{"x-foo", "some value"}, {"bar", "911"}]
+
+    assert_receive {:headers, headers}
+
+    header_set = MapSet.new(headers)
+
+    assert Enum.all?(expected_headers, &MapSet.member?(header_set, &1))
+
+    assert log =~ ~r/is not lowercase/
+  end
+
+  test "header validation warns if option set and invalid header key sent", c do
+    pid = self()
+
+    Bypass.expect(c.bypass, "GET", "/", fn conn ->
+      headers = Enum.filter(conn.req_headers, fn {name, _} -> String.contains?(name, "valid") end)
+
+      send(pid, {:headers, headers})
+      Plug.Conn.send_resp(conn, 200, "ok")
+    end)
+
+    headers = [{:x_Invalid, "any value"}, {"NotValid", "whoops!"}]
+
+    {_result, log} =
+      with_log(fn ->
+        Req.new(invalid_header_keys: :warn, url: c.url, headers: headers)
+        |> Req.get!()
+      end)
+
+    expected_headers = [{"x-invalid", "any value"}, {"notvalid", "whoops!"}]
+
+    assert_receive {:headers, headers}
+
+    header_set = MapSet.new(headers)
+
+    assert Enum.all?(expected_headers, &MapSet.member?(header_set, &1))
+
+    assert log =~ ~r/is not lowercase/
+  end
+
+  test "header validation doesn't warn if option :ignore set and invalid header key sent",
+       c do
+    pid = self()
+
+    Bypass.expect(c.bypass, "GET", "/", fn conn ->
+      headers = Enum.filter(conn.req_headers, fn {name, _} -> String.contains?(name, "valid") end)
+
+      send(pid, {:headers, headers})
+      Plug.Conn.send_resp(conn, 200, "ok")
+    end)
+
+    headers = [{:x_Invalid, "any value"}, {"NotValid", "whoops!"}]
+
+    {_result, log} =
+      with_log(fn ->
+        Req.new(invalid_header_keys: :ignore, url: c.url, headers: headers)
+        |> Req.get!()
+      end)
+
+    expected_headers = [{"x-invalid", "any value"}, {"notvalid", "whoops!"}]
+
+    assert_receive {:headers, headers}
+
+    header_set = MapSet.new(headers)
+
+    assert Enum.all?(expected_headers, &MapSet.member?(header_set, &1))
+
+    refute log =~ ~r/is not lowercase/
+  end
+
+  test "header validation raises if option :raise set and invalid header key sent",
+       c do
+    headers = [{:x_Invalid, "any value"}, {"NotValid", "whoops!"}]
+
+    assert_raise(RuntimeError, ~r/[x_Invalid|NotValid]/, fn ->
+      Req.new(invalid_header_keys: :raise, url: c.url, headers: headers)
+      |> Req.get!()
+    end)
+  end
+
   test "redact" do
     assert inspect(Req.new(auth: {:bearer, "foo"})) =~ ~s|auth: {:bearer, "[redacted]"}|