Clean up the API to expose Huffman encoding (#13)

Author: whatyouhide

lib/hpax.ex

@@ -31,22 +31,46 @@ defmodule HPAX do
 
   @valid_header_actions [:store, :store_name, :no_store, :never_store]
 
+  @doc """
+  Creates a new HPACK table.
+
+  Same as `new/2` with default options.
+  """
+  @spec new(non_neg_integer()) :: Table.t()
+  def new(max_table_size), do: new(max_table_size, [])
+
   @doc """
   Create a new HPACK table that can be used as encoding or decoding context.
 
   See the "Encoding and decoding contexts" section in the module documentation.
 
   `max_table_size` is the maximum table size (in bytes) for the newly created table.
 
+  ## Options
+
+  This function accepts the following `options`:
+
+    * `:huffman_encoding` - (since 0.3.0) `:always` or `:never`. If `:always`,
+      then HPAX will always encode headers using Huffman encoding. If `:never`,
+      HPAX will not use any Huffman encoding. Defaults to `:never`.
+
   ## Examples
 
       encoding_context = HPAX.new(4096)
 
   """
-  @spec new(non_neg_integer(), [Table.option()]) :: Table.t()
-  def new(max_table_size, options \\ [])
-      when is_integer(max_table_size) and max_table_size >= 0 do
-    Table.new(max_table_size, options)
+  @doc since: "0.3.0"
+  @spec new(non_neg_integer(), [keyword()]) :: Table.t()
+  def new(max_table_size, options)
+      when is_integer(max_table_size) and max_table_size >= 0 and is_list(options) do
+    options = Keyword.put_new(options, :huffman_encoding, :never)
+
+    Enum.each(options, fn
+      {:huffman_encoding, _huffman_encoding} -> :ok
+      {key, _value} -> raise ArgumentError, "unknown option: #{inspect(key)}"
+    end)
+
+    Table.new(max_table_size, Keyword.fetch!(options, :huffman_encoding))
   end
 
   @doc """
@@ -252,7 +276,7 @@ defmodule HPAX do
 
   defp encode_headers([{action, name, value} | rest], table, acc)
        when action in @valid_header_actions and is_binary(name) and is_binary(value) do
-    huffman? = table.huffman == :always
+    huffman? = table.huffman_encoding == :always
 
     {encoded, table} =
       case Table.lookup_by_header(table, name, value) do
@@ -287,27 +311,27 @@ defmodule HPAX do
     <<1::1, Types.encode_integer(index, 7)::bitstring>>
   end
 
-  defp encode_literal_header_with_indexing(index, value, huffman) when is_integer(index) do
-    [<<1::2, Types.encode_integer(index, 6)::bitstring>>, Types.encode_binary(value, huffman)]
+  defp encode_literal_header_with_indexing(index, value, huffman?) when is_integer(index) do
+    [<<1::2, Types.encode_integer(index, 6)::bitstring>>, Types.encode_binary(value, huffman?)]
   end
 
-  defp encode_literal_header_with_indexing(name, value, huffman) when is_binary(name) do
-    [<<1::2, 0::6>>, Types.encode_binary(name, huffman), Types.encode_binary(value, huffman)]
+  defp encode_literal_header_with_indexing(name, value, huffman?) when is_binary(name) do
+    [<<1::2, 0::6>>, Types.encode_binary(name, huffman?), Types.encode_binary(value, huffman?)]
   end
 
-  defp encode_literal_header_without_indexing(index, value, huffman) when is_integer(index) do
-    [<<0::4, Types.encode_integer(index, 4)::bitstring>>, Types.encode_binary(value, huffman)]
+  defp encode_literal_header_without_indexing(index, value, huffman?) when is_integer(index) do
+    [<<0::4, Types.encode_integer(index, 4)::bitstring>>, Types.encode_binary(value, huffman?)]
   end
 
-  defp encode_literal_header_without_indexing(name, value, huffman) when is_binary(name) do
-    [<<0::4, 0::4>>, Types.encode_binary(name, huffman), Types.encode_binary(value, huffman)]
+  defp encode_literal_header_without_indexing(name, value, huffman?) when is_binary(name) do
+    [<<0::4, 0::4>>, Types.encode_binary(name, huffman?), Types.encode_binary(value, huffman?)]
   end
 
-  defp encode_literal_header_never_indexed(index, value, huffman) when is_integer(index) do
-    [<<1::4, Types.encode_integer(index, 4)::bitstring>>, Types.encode_binary(value, huffman)]
+  defp encode_literal_header_never_indexed(index, value, huffman?) when is_integer(index) do
+    [<<1::4, Types.encode_integer(index, 4)::bitstring>>, Types.encode_binary(value, huffman?)]
   end
 
-  defp encode_literal_header_never_indexed(name, value, huffman) when is_binary(name) do
-    [<<1::4, 0::4>>, Types.encode_binary(name, huffman), Types.encode_binary(value, huffman)]
+  defp encode_literal_header_never_indexed(name, value, huffman?) when is_binary(name) do
+    [<<1::4, 0::4>>, Types.encode_binary(name, huffman?), Types.encode_binary(value, huffman?)]
   end
 end

lib/hpax/table.ex

@@ -1,25 +1,25 @@
 defmodule HPAX.Table do
   @moduledoc false
 
+  @enforce_keys [:max_table_size, :huffman_encoding]
   defstruct [
     :max_table_size,
-    :huffman,
+    :huffman_encoding,
     entries: [],
     size: 0,
     length: 0
   ]
 
+  @type huffman_encoding() :: :always | :never
+
   @type t() :: %__MODULE__{
           max_table_size: non_neg_integer(),
-          huffman: huffman_strategy(),
+          huffman_encoding: huffman_encoding(),
           entries: [{binary(), binary()}],
           size: non_neg_integer(),
           length: non_neg_integer()
         }
 
-  @type huffman_strategy() :: :always | :never
-  @type option :: {:huffman, huffman_strategy()}
-
   @static_table [
     {":authority", nil},
     {":method", "GET"},
@@ -93,10 +93,11 @@ defmodule HPAX.Table do
   The maximum size is not the maximum number of entries but rather the maximum size as defined in
   http://httpwg.org/specs/rfc7541.html#maximum.table.size.
   """
-  @spec new(non_neg_integer(), [option()]) :: t()
-  def new(max_table_size, options \\ []) do
-    huffman = Keyword.get(options, :huffman, :always)
-    %__MODULE__{max_table_size: max_table_size, huffman: huffman}
+  @spec new(non_neg_integer(), huffman_encoding()) :: t()
+  def new(max_table_size, huffman_encoding)
+      when is_integer(max_table_size) and max_table_size >= 0 and
+             huffman_encoding in [:always, :never] do
+    %__MODULE__{max_table_size: max_table_size, huffman_encoding: huffman_encoding}
   end
 
   @doc """

test/hpax/table_test.exs

@@ -5,11 +5,11 @@ defmodule HPAX.TableTest do
   alias HPAX.Table
 
   test "new/1" do
-    assert %Table{} = Table.new(100)
+    assert %Table{} = Table.new(100, :always)
   end
 
   test "adding headers and fetching them by value" do
-    table = Table.new(10_000)
+    table = Table.new(10_000, :always)
 
     # These are in the static table.
     assert {:full, _} = Table.lookup_by_header(table, ":status", "200")
@@ -30,7 +30,7 @@ defmodule HPAX.TableTest do
     dynamic_table_start = length(Table.__static_table__()) + 1
 
     # This fits two headers that have name and value of 4 bytes (4 + 4 + 32, twice).
-    table = Table.new(80)
+    table = Table.new(80, :always)
 
     table = Table.add(table, "aaaa", "AAAA")
     table = Table.add(table, "bbbb", "BBBB")
@@ -50,15 +50,15 @@ defmodule HPAX.TableTest do
 
   describe "looking headers up by index" do
     test "with an index out of bounds" do
-      assert Table.lookup_by_index(Table.new(100), 1000) == :error
+      assert Table.lookup_by_index(Table.new(100, :never), 1000) == :error
     end
 
     test "with an index in the static table" do
-      assert Table.lookup_by_index(Table.new(100), 1) == {:ok, {":authority", nil}}
+      assert Table.lookup_by_index(Table.new(100, :never), 1) == {:ok, {":authority", nil}}
     end
 
     test "with an index in the dynamic table" do
-      table = Table.new(100)
+      table = Table.new(100, :never)
       table = Table.add(table, "my-header", "my-value")
 
       assert Table.lookup_by_index(table, length(Table.__static_table__()) + 1) ==
@@ -68,7 +68,7 @@ defmodule HPAX.TableTest do
 
   property "adding a header and then looking it up always returns the index of that header" do
     check all {name, value} <- {string(0..127, min_length: 1), binary()} do
-      assert %Table{} = table = Table.new(10_000)
+      assert %Table{} = table = Table.new(10_000, :never)
       assert %Table{} = table = Table.add(table, name, value)
       assert {:full, 62} = Table.lookup_by_header(table, name, value)
     end

test/hpax_test.exs

@@ -6,6 +6,14 @@ defmodule HPAXTest do
     assert %HPAX.Table{} = HPAX.new(100)
   end
 
+  describe "new/2" do
+    test "raises for unknown options" do
+      assert_raise ArgumentError, "unknown option: :unknown", fn ->
+        HPAX.new(100, unknown: :option)
+      end
+    end
+  end
+
   # https://http2.github.io/http2-spec/compression.html#rfc.section.C.2.1
   test "decode/2 with an example from the spec" do
     table = HPAX.new(1000)
@@ -23,7 +31,7 @@ defmodule HPAXTest do
 
   # https://http2.github.io/http2-spec/compression.html#rfc.section.C.3.1
   test "encode/2 with a literal example from the spec" do
-    table = HPAX.new(1000, huffman: :never)
+    table = HPAX.new(1000, huffman_encoding: :never)
 
     headers = [
       {:store, ":method", "GET"},
@@ -43,7 +51,7 @@ defmodule HPAXTest do
 
   # https://http2.github.io/http2-spec/compression.html#rfc.section.C.4.1
   test "encode/2 with a Huffman example from the spec" do
-    table = HPAX.new(1000)
+    table = HPAX.new(1000, huffman_encoding: :always)
 
     headers = [
       {:store, ":method", "GET"},