Improve HTTP1 invalid header value crash

[wojtekmach]

Ref: #393

Before this patch:

iex> {:ok, conn} = Mint.HTTP1.connect(:http, "httpbin.org", 80) ; Mint.HTTP.request(conn, "GET", "/json", [{"accept", nil}], nil)
** (ErlangError) Erlang error: {:bad_generator, nil}
    (mint 1.7.1) lib/mint/http1/request.ex:61: Mint.HTTP1.Request."-validate_header_value!/2-lc$^0/1-0-"/3
    (mint 1.7.1) lib/mint/http1/request.ex:61: Mint.HTTP1.Request.validate_header_value!/2
    (mint 1.7.1) lib/mint/http1/request.ex:26: anonymous fn/2 in Mint.HTTP1.Request.encode_headers/1
    (elixir 1.18.4) lib/enum.ex:2546: Enum."-reduce/3-lists^foldl/2-0-"/3
    (mint 1.7.1) lib/mint/http1/request.ex:9: Mint.HTTP1.Request.encode/4
    (mint 1.7.1) lib/mint/http1.ex:286: Mint.HTTP1.request/5
    iex:4: (file)

After:

iex> {:ok, conn} = Mint.HTTP1.connect(:http, "httpbin.org", 80) ; Mint.HTTP.request(conn, "GET", "/json", [{"accept", nil}], nil)
** (FunctionClauseError) no function clause matching in Mint.HTTP1.Request.validate_header_value!/2

    The following arguments were given to Mint.HTTP1.Request.validate_header_value!/2:

        # 1
        "accept"

        # 2
        nil

    Attempted function clauses (showing 1 out of 1):

        defp validate_header_value!(name, value) when is_binary(value)

    (mint 1.7.1) lib/mint/http1/request.ex:59: Mint.HTTP1.Request.validate_header_value!/2
    (mint 1.7.1) lib/mint/http1/request.ex:26: anonymous fn/2 in Mint.HTTP1.Request.encode_headers/1
    (elixir 1.18.4) lib/enum.ex:2546: Enum."-reduce/3-lists^foldl/2-0-"/3
    (mint 1.7.1) lib/mint/http1/request.ex:9: Mint.HTTP1.Request.encode/4
    (mint 1.7.1) lib/mint/http1.ex:286: Mint.HTTP1.request/5
    iex:4: (file)

The error message is still pretty bad for HTTP2 at the moment though:

iex> {:ok, conn} = Mint.HTTP2.connect(:https, "httpbin.org", 443) ; Mint.HTTP.request(conn, "GET", "/json", [{"accept", nil}], nil)
** (FunctionClauseError) no function clause matching in HPAX.encode_headers/3

    The following arguments were given to HPAX.encode_headers/3:

        # 1
        [{:store_name, "accept", nil}]

        # 2
        %HPAX.Table{
          max_table_size: 4096,
          huffman_encoding: :never,
          entries: [],
          size: 0,
          length: 0
        }

        # 3
        [
          [
            [[[[], <<130>>], [<<4>>, [<<5>>, "/json"]]], <<135>>],
            [<<1>>, ["\v", "httpbin.org"]]
          ],
          [<<15, 43>>, ["\n", "mint/1.7.1"]]
        ]

    Attempted function clauses (showing 2 out of 2):

        defp encode_headers([], table, acc)
        defp encode_headers([{action, name, value} | rest], table, acc) when (action === :store or action === :store_name or action === :no_store or action === :never_store) and is_binary(name) and is_binary(value)

    (hpax 1.0.0) lib/hpax.ex:281: HPAX.encode_headers/3
    (elixir 1.18.4) lib/map.ex:999: Map.get_and_update!/3
    (mint 1.7.1) lib/mint/http2.ex:1134: Mint.HTTP2.encode_headers/4
    (mint 1.7.1) lib/mint/http2.ex:520: Mint.HTTP2.request/5
    iex:4: (file)

[whatyouhide]

Great job @wojtekmach, any chance you want to take on the HTTP/2 side as well?

[wojtekmach]

@whatyouhide sure thing, wdyt about this?

diff --git a/lib/hpax.ex b/lib/hpax.ex
index 91c9401..6106ac7 100644
--- a/lib/hpax.ex
+++ b/lib/hpax.ex
@@ -298,7 +298,8 @@ defmodule HPAX do
   end

   defp encode_headers([{action, name, value} | rest], table, acc)
-       when action in @valid_header_actions and is_binary(name) and is_binary(value) do
+       when action in @valid_header_actions do
+    validate_header_value(name, value)
     huffman? = table.huffman_encoding == :always

     {encoded, table} =
@@ -330,6 +331,10 @@ defmodule HPAX do
     encode_headers(rest, table, [acc, encoded])
   end

+  defp validate_header_value(name, value) when is_binary(name) and is_binary(value) do
+    :ok
+  end
+

Then we have:

iex> {:ok, conn} = Mint.HTTP2.connect(:https, "httpbin.org", 443) ; Mint.HTTP.request(conn, "GET", "/json", [{"accept", nil}], nil)
** (FunctionClauseError) no function clause matching in HPAX.validate_header_value/2

    The following arguments were given to HPAX.validate_header_value/2:

        # 1
        "accept"

        # 2
        nil

    Attempted function clauses (showing 1 out of 1):

        defp validate_header_value(name, value) when is_binary(name) and is_binary(value)

    (hpax 1.0.0) lib/hpax.ex:334: HPAX.validate_header_value/2
    (hpax 1.0.0) lib/hpax.ex:302: HPAX.encode_headers/3
    (elixir 1.18.4) lib/map.ex:999: Map.get_and_update!/3
    (mint 1.7.1) lib/mint/http2.ex:1134: Mint.HTTP2.encode_headers/4
    (mint 1.7.1) lib/mint/http2.ex:520: Mint.HTTP2.request/5
    iex:1: (file)

I assume adding a function like that is not adding too much overhead! WDYT? I'm happy to send hpax PR for this.

[whatyouhide]

I'd probably go with a nice ArgumentError since we're adding code to handle this anyway but yes please do send a HPax PR ❤️

[wojtekmach]

Fixed in hpax!