diff --git a/lib/decode_error.ex b/lib/decode_error.ex new file mode 100644 index 0000000..effb3fd --- /dev/null +++ b/lib/decode_error.ex @@ -0,0 +1,4 @@ +defmodule Exgencode.DecodeError do + @moduledoc false + defexception [:message] +end diff --git a/lib/exgencode.ex b/lib/exgencode.ex index 86900dd..898e3d2 100644 --- a/lib/exgencode.ex +++ b/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, pdu_bit_size) do + binary = + Exgencode.EncodeDecode.skip_to_offset( + pdu, + field, + binary, + pdu_bit_size, + 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, pdu_bit_size) end - defp do_decode(pdu, rest_bin, [], _) do + defp do_decode(pdu, rest_bin, [], _, _) do {pdu, rest_bin} end end diff --git a/lib/exgencode/encode_decode.ex b/lib/exgencode/encode_decode.ex index 6b898d7..ab07d71 100644 --- a/lib/exgencode/encode_decode.ex +++ b/lib/exgencode/encode_decode.ex @@ -303,6 +303,45 @@ defmodule Exgencode.EncodeDecode do wrap_conditional_decode(props, basic_fun) end + def skip_to_offset(pdu, field, rest_binary, pdu_bit_size, offset_targets) do + case offset_targets do + %{^field => offset_field} -> + do_skip_to_offset(pdu, field, offset_field, rest_binary, pdu_bit_size) + + _ -> + rest_binary + end + end + + defp do_skip_to_offset(pdu, field, offset_field, rest_binary, pdu_bit_size) do + case Map.get(pdu, offset_field) do + offset when offset in [0, nil] -> + rest_binary + + offset -> + cursor = pdu_bit_size - bit_size(rest_binary) + target = offset * 8 + + cond do + target == cursor -> + rest_binary + + target < cursor -> + raise Exgencode.DecodeError, + "offset field #{inspect(offset_field)} of #{inspect(field)} cannot point backwards!" + + target > pdu_bit_size -> + raise Exgencode.DecodeError, + "offset_field #{inspect(offset_field)} cannot point outside binary!" + + true -> + gap_bits = target - cursor + <<_padding::size(gap_bits), rest::bitstring>> = rest_binary + rest + end + end + end + def wrap_custom_encode(field_name, encode_fun) do quote do fn pdu -> diff --git a/lib/exgencode/validator.ex b/lib/exgencode/validator.ex index 7220d4c..a0d36ce 100644 --- a/lib/exgencode/validator.ex +++ b/lib/exgencode/validator.ex @@ -126,6 +126,12 @@ defmodule Exgencode.Validator do end def validate_pdu(pdu_name, fields) do + validate_pdu_size(pdu_name, fields) + validate_offset_ordering(pdu_name, fields) + validate_no_duplicate_offsets(pdu_name, fields) + end + + defp validate_pdu_size(pdu_name, fields) do total_size = fields |> Enum.reject(fn {_field_name, props} -> props[:type] == :variable end) @@ -143,6 +149,44 @@ defmodule Exgencode.Validator do ) end + defp validate_offset_ordering(pdu_name, fields) do + Enum.reduce(fields, [], fn {field_name, props}, seen_names -> + target = props[:offset_to] + + if not is_nil(target) and target in seen_names do + raise ArgumentError, + "#{inspect(Macro.to_string(pdu_name))} #{inspect(target)}: backward offsets are unsupported!" + end + + [field_name | seen_names] + end) + + :ok + end + + defp validate_no_duplicate_offsets(pdu_name, fields) do + Enum.reduce(fields, [], fn {field_name, props}, seen_targets -> + target = props[:offset_to] + + cond do + is_nil(target) -> + seen_targets + + target in seen_targets -> + raise_argument_error( + pdu_name, + field_name, + "multiple offset fields pointing to target #{inspect(target)} is unsupported!" + ) + + true -> + [target | seen_targets] + end + end) + + :ok + 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())} - " <> diff --git a/mix.exs b/mix.exs index d6686ab..ddab69a 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Exgencode.Mixfile do def project do [ app: :exgencode, - version: "2.5.2", + version: "2.5.3", elixir: "~> 1.7", start_permanent: Mix.env() == :prod, deps: deps(), diff --git a/test/exgencode_test.exs b/test/exgencode_test.exs index 634764f..1562a6d 100644 --- a/test/exgencode_test.exs +++ b/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, 9, 5, 0, 0, 0, 4, 0, 8, 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, 9, 5, 0, 0, 0, 4, 0, 8, 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, 8, 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/point 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/cannot point outside binary/, 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 pointing/, 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/backward offsets are unsupported/, fn -> + defmodule BadOrder do + import Exgencode + + defpdu BackwardsOffsetPdu, + target: [size: 8], + off: [size: 8, offset_to: :target] + end + end + end end diff --git a/test/helpers/test_pdu.ex b/test/helpers/test_pdu.ex index 6463827..c3f91fc 100644 --- a/test/helpers/test_pdu.ex +++ b/test/helpers/test_pdu.ex @@ -157,9 +157,9 @@ defmodule Exgencode.TestPdu do <> = 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], + anotherWhyNot: [size: 8, default: 8], oneMore: [default: 4, size: 8] defpdu OffsetMadnessPdu, @@ -168,4 +168,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