Enhance decoding to include offset_to properties

lib/decode_error.ex

@@ -0,0 +1,8 @@
+defmodule Exgencode.DecodeError do
+  @moduledoc """
+  Raised during decoding when an `:offset_to` field points to an invalid
+  location - either backwards (before the current cursor, into already-parsed
+  bytes) or past the end of the binary.
+  """
+  defexception [:message]
+end

lib/exgencode.ex

@@ -346,6 +346,12 @@ defmodule Exgencode do
         {field_name, props[:decode]}
       end)
 
+    offset_targets =
+      for {field_name, props} <- field_list, props[:offset_to] != nil, into: %{} do
+        {props[:offset_to], field_name}
+      end
+      |> Macro.escape()
+
     struct_fields =
       for {field_name, props} <- field_list, props[:type] not in [:constant, :skip] do
         {field_name, props[:default]}
@@ -376,15 +382,24 @@ defmodule Exgencode do
         end
 
         def decode(pdu, binary, version) do
-          do_decode(pdu, binary, unquote(fields_for_decodes), version)
+          do_decode(pdu, binary, unquote(fields_for_decodes), version, bit_size(binary))
         end
 
-        defp do_decode(pdu, binary, [{field, decode_fun} | rest], version) do
+        defp do_decode(pdu, binary, [{field, decode_fun} | rest], version, init_bits) do
+          binary =
+            Exgencode.EncodeDecode.skip_to_offset(
+              pdu,
+              field,
+              binary,
+              init_bits,
+              unquote(offset_targets)
+            )
+
           {new_pdu, rest_binary} = decode_fun.(version).(pdu, binary)
-          do_decode(new_pdu, rest_binary, rest, version)
+          do_decode(new_pdu, rest_binary, rest, version, init_bits)
         end
 
-        defp do_decode(pdu, rest_bin, [], _) do
+        defp do_decode(pdu, rest_bin, [], _, _) do
           {pdu, rest_bin}
         end
       end

lib/exgencode/encode_decode.ex

@@ -303,6 +303,43 @@ defmodule Exgencode.EncodeDecode do
     wrap_conditional_decode(props, basic_fun)
   end
 
+  def skip_to_offset(pdu, field, binary, init_bits, offset_targets) do
+    case offset_targets do
+      %{^field => offset_field} ->
+        do_skip_to_offset(pdu, field, offset_field, binary, init_bits)
+
+      _ ->
+        binary
+    end
+  end
+
+  defp do_skip_to_offset(pdu, field, offset_field, binary, init_bits) do
+    case Map.get(pdu, offset_field) do
+      offset when offset in [nil, 0] ->
+        binary
+
+      offset ->
+        cursor_bits = init_bits - bit_size(binary)
+        target_bits = offset * 8
+
+        cond do
+          target_bits == cursor_bits ->
+            binary
+
+          target_bits < cursor_bits ->
+            raise Exgencode.DecodeError, "offset field #{inspect(offset_field)} of #{inspect(field)} cannot point backwards!"
+
+          target_bits - cursor_bits > bit_size(binary) ->
+            raise Exgencode.DecodeError, "offset_field #{inspect(offset_field)} cannot point outside binary!"
+
+          true ->
+            gap_bits = target_bits - cursor_bits
+            <<_padding::size(gap_bits), rest::bitstring>> = binary
+            rest
+        end
+    end
+  end
+
   def wrap_custom_encode(field_name, encode_fun) do
     quote do
       fn pdu ->

lib/exgencode/validator.ex

@@ -126,6 +126,8 @@ defmodule Exgencode.Validator do
   end
 
   def validate_pdu(pdu_name, fields) do
+    validate_offset_ordering(pdu_name, fields)
+
     total_size =
       fields
       |> Enum.reject(fn {_field_name, props} -> props[:type] == :variable end)
@@ -143,9 +145,26 @@ defmodule Exgencode.Validator do
         )
   end
 
-  defp raise_argument_error(pdu_name, field_name, msg) do
-    raise ArgumentError,
-          "Badly defined field #{inspect(field_name)} in #{inspect(pdu_name |> Macro.to_string())} - " <>
-            msg
+  defp validate_offset_ordering(pdu_name, fields) do
+    ordered_names = Enum.map(fields, fn {field_name, _props} -> field_name end)
+
+    fields
+    |> Enum.filter(fn {_field_name, props} -> props[:offset_to] != nil end)
+    |> Enum.reduce(MapSet.new(), fn {field_name, props}, seen_targets ->
+      target = props[:offset_to]
+
+      if MapSet.member?(seen_targets, target),
+        do: raise(ArgumentError, "#{inspect(pdu_name |> Macro.to_string())} multiple offset fields pointing to field #{inspect(target)} is unsupported!")
+
+      offset_index = Enum.find_index(ordered_names, &(&1 == field_name))
+      target_index = Enum.find_index(ordered_names, &(&1 == target))
+
+      if not is_nil(target_index) and target_index < offset_index,
+        do: raise(ArgumentError, "#{inspect(pdu_name |> Macro.to_string())} #{inspect(target)}: backward offsets are unsupported!")
+
+      MapSet.put(seen_targets, target)
+    end)
+
+    :ok
   end
 end

test/exgencode_test.exs

@@ -87,11 +87,15 @@ defmodule ExgencodeTest do
 
     nested_pdu = %TestPdu.NestedVersionedMsg{nested: %TestPdu.VersionedMsg{}}
     binary = <<2::size(16), 10::size(16)>>
-    assert {^nested_pdu, <<>>} = Exgencode.Pdu.decode(%TestPdu.NestedVersionedMsg{}, binary, "1.0.0")
+
+    assert {^nested_pdu, <<>>} =
+             Exgencode.Pdu.decode(%TestPdu.NestedVersionedMsg{}, binary, "1.0.0")
 
     nested_pdu = %TestPdu.NestedVersionedMsg{nested: %TestPdu.VersionedMsg{newerField: 111}}
     binary = <<2::size(16), 10::size(16), 111::size(8)>>
-    assert {^nested_pdu, <<>>} = Exgencode.Pdu.decode(%TestPdu.NestedVersionedMsg{}, binary, "2.0.0")
+
+    assert {^nested_pdu, <<>>} =
+             Exgencode.Pdu.decode(%TestPdu.NestedVersionedMsg{}, binary, "2.0.0")
   end
 
   test "versioned encode/decode symmetry" do
@@ -569,14 +573,14 @@ defmodule ExgencodeTest do
   test "custom size function" do
     pdu = %TestPdu.CustomSizeFunPdu{custom: {5, 1024}}
 
-    assert <<2, 8, 5, 0, 0, 0, 4, 0, 8, 4>> = Exgencode.Pdu.encode(pdu)
+    assert <<2, 8, 5, 0, 0, 0, 4, 0, 4>> = Exgencode.Pdu.encode(pdu)
 
     offsets = Exgencode.Pdu.set_offsets(pdu)
 
     assert {offsets, <<>>} ==
              Exgencode.Pdu.decode(
                %TestPdu.CustomSizeFunPdu{},
-               <<2, 8, 5, 0, 0, 0, 4, 0, 8, 4>>,
+               <<2, 8, 5, 0, 0, 0, 4, 0, 4>>,
                nil
              )
   end
@@ -585,4 +589,104 @@ defmodule ExgencodeTest do
     pdu = %TestPdu.OffsetMadnessPdu{}
     assert <<3, 4, 0, 11, 5, 0>> = Exgencode.Pdu.encode(pdu)
   end
+
+  test "decode reaches offset_to : skips gap" do
+    binary = <<7, 0, 3, "abc", 0, 0xDE, 0xAD, 0xBE, 0xEF>>
+
+    assert {%TestPdu.OffsetMsg{
+              offset_to_footer: 7,
+              name_length: 3,
+              name: "abc",
+              footer: 0xDEADBEEF
+            }, <<>>} == Exgencode.Pdu.decode(%TestPdu.OffsetMsg{}, binary)
+  end
+
+  test "decode with zero-length gap (offset == cursor) is unchanged" do
+    binary = <<6, 0, 3, "abc", 0xDE, 0xAD, 0xBE, 0xEF>>
+
+    assert {%TestPdu.OffsetMsg{
+              offset_to_footer: 6,
+              name_length: 3,
+              name: "abc",
+              footer: 0xDEADBEEF
+            }, <<>>} == Exgencode.Pdu.decode(%TestPdu.OffsetMsg{}, binary)
+  end
+
+  test "decode with offset 0 leaves the target field absent" do
+    binary = <<0, 0, 3, "abc">>
+
+    assert {%TestPdu.OffsetMsg{
+              offset_to_footer: 0,
+              name_length: 3,
+              name: "abc",
+              footer: nil
+            }, <<>>} == Exgencode.Pdu.decode(%TestPdu.OffsetMsg{}, binary)
+  end
+
+  test "multiple offsets with offset_to gap" do
+    binary = <<4, 7, 0, 2, "XY", 0, 0, 0xBE, 0xEF>>
+
+    assert {%TestPdu.MultiOffsetMsg{
+              offset_to_a: 4,
+              offset_to_b: 8,
+              len_a: 2,
+              field_a: "XY",
+              field_b: 0xBEEF
+            }, <<>>} == Exgencode.Pdu.decode(%TestPdu.MultiOffsetMsg{}, binary)
+  end
+
+  test "decode raises on a backwards offset" do
+    binary = <<4, 0, 3, "abc", 0xDE, 0xAD, 0xBE, 0xEF>>
+
+    assert_raise Exgencode.DecodeError, ~r/points backwards/, fn ->
+      Exgencode.Pdu.decode(%TestPdu.OffsetMsg{}, binary)
+    end
+  end
+
+  test "decode raises on an offset past the end of the binary" do
+    binary = <<20, 0, 3, "abc", 0, 0xDE, 0xAD, 0xBE, 0xEF>>
+
+    assert_raise Exgencode.DecodeError, ~r/points past end/, fn ->
+      Exgencode.Pdu.decode(%TestPdu.OffsetMsg{}, binary)
+    end
+  end
+
+  test "offsets in a nested subrecord stay relative to that subrecord" do
+    binary = <<9, 7, 0, 3, "abc", 0, 0xDE, 0xAD, 0xBE, 0xEF>>
+
+    assert {%TestPdu.NestedOffsetMsg{
+              header: 9,
+              sub: %TestPdu.OffsetMsg{
+                offset_to_footer: 7,
+                name_length: 3,
+                name: "abc",
+                footer: 0xDEADBEEF
+              }
+            }, <<>>} == Exgencode.Pdu.decode(%TestPdu.NestedOffsetMsg{}, binary)
+  end
+
+  test "defining two offset fields to the same target is rejected at compile time" do
+    assert_raise ArgumentError, ~r/Multiple offset fields point to/, fn ->
+      defmodule BadDup do
+        import Exgencode
+
+        defpdu DuplicateOffsetPdu,
+          off_a: [size: 8, offset_to: :target],
+          off_b: [size: 8, offset_to: :target],
+          target: [size: 8, conditional: :off_a]
+      end
+    end
+  end
+
+  test "defining an offset field after its target is rejected at compile time" do
+    assert_raise ArgumentError, ~r/must be defined before its target/, fn ->
+      defmodule BadOrder do
+        import Exgencode
+
+        defpdu BackwardsOffsetPdu,
+          target: [size: 8],
+          off: [size: 8, offset_to: :target]
+      end
+    end
+  end
 end

test/helpers/test_pdu.ex

@@ -157,9 +157,8 @@ defmodule Exgencode.TestPdu do
         <<vals::size*8, rest::binary>> = val
         {struct(pdu, %{custom: {size, vals}}), rest}
       end,
-      size: fn %CustomSizeFunPdu{custom: {size, _vals}} -> size * 8 end
+      size: fn %CustomSizeFunPdu{custom: {size, _vals}} -> (size + 1) * 8 end
     ],
-    anotherWhyNot: [offset_to: :oneMore, size: 8],
     oneMore: [default: 4, size: 8]
 
   defpdu OffsetMadnessPdu,
@@ -168,4 +167,21 @@ defmodule Exgencode.TestPdu do
     somethingIrrelevant: [default: 11, size: 16],
     anotherOffset: [offset_to: :somethingElse, size: 8],
     somethingElse: [default: 0, size: 8]
+
+  defpdu OffsetMsg,
+    offset_to_footer: [size: 8, offset_to: :footer, default: 0x00],
+    name_length: [size: 16, default: 0],
+    name: [type: :variable, size: :name_length, conditional: :name_length],
+    footer: [size: 32, conditional: :offset_to_footer]
+
+  defpdu MultiOffsetMsg,
+    offset_to_a: [size: 8, offset_to: :field_a, default: 0x00],
+    offset_to_b: [size: 8, offset_to: :field_b, default: 0x00],
+    len_a: [size: 16, default: 0],
+    field_a: [type: :variable, size: :len_a, conditional: :len_a],
+    field_b: [size: 16, conditional: :offset_to_b]
+
+  defpdu NestedOffsetMsg,
+    header: [size: 8, default: 0],
+    sub: [type: :subrecord, default: %OffsetMsg{}]
 end