From bcfb564a93b6488c8b7d50814827657d6e8c0de3 Mon Sep 17 00:00:00 2001 From: Luis Guilherme Almeida Date: Wed, 22 Feb 2023 14:06:35 +0100 Subject: [PATCH 1/4] Run verify_msg before encoding --- lib/exprotobuf/encoder.ex | 1 + test/protobuf/encoder_test.exs | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/exprotobuf/encoder.ex b/lib/exprotobuf/encoder.ex index 6724c78..fc186a0 100644 --- a/lib/exprotobuf/encoder.ex +++ b/lib/exprotobuf/encoder.ex @@ -28,6 +28,7 @@ defmodule Protobuf.Encoder do |> wrap_scalars(Utils.msg_defs(defs)) |> fix_undefined |> Utils.convert_to_record(msg.__struct__) + |> tap(:gpb.verify_msg(&1, fixed_defs)) |> :gpb.encode_msg(fixed_defs) end diff --git a/test/protobuf/encoder_test.exs b/test/protobuf/encoder_test.exs index 2244a95..ada9369 100644 --- a/test/protobuf/encoder_test.exs +++ b/test/protobuf/encoder_test.exs @@ -40,7 +40,7 @@ defmodule Protobuf.Encoder.Test do } """ end - + #defmodule ExtensionsProto do #use Protobuf, """ #message Msg { @@ -59,6 +59,12 @@ defmodule Protobuf.Encoder.Test do assert <<10, 3, 8, 150, 1>> == Protobuf.Serializable.serialize(EncoderProto.WithSubMsg.new(f1: msg)) end + test "Raises when integer is out of range" do + assert_raise ErlangError, fn -> + EncoderProto.Msg.new(f1: 2 ** 128) + end + end + test "fixing a nil value in repeated submsg" do msg = EncoderProto.WithRepeatedSubMsg.new(f1: [EncoderProto.Msg.new(f1: 1)]) assert <<10, 2, 8, 1>> == Protobuf.Serializable.serialize(msg) From 5a8012cb37cce3da5f155e1dc5086504ece29c7f Mon Sep 17 00:00:00 2001 From: Luis Almeida Date: Thu, 4 Jul 2024 16:00:51 +0100 Subject: [PATCH 2/4] update gpb --- lib/exprotobuf/encoder.ex | 27 ++++++++++++++------------- mix.lock | 16 ++++++++-------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/lib/exprotobuf/encoder.ex b/lib/exprotobuf/encoder.ex index fc186a0..03c1718 100644 --- a/lib/exprotobuf/encoder.ex +++ b/lib/exprotobuf/encoder.ex @@ -1,8 +1,10 @@ defmodule Protobuf.Encoder do - require Protobuf.Utils, as: Utils + @moduledoc false alias Protobuf.Field alias Protobuf.OneOfField + require Protobuf.Utils, as: Utils + def encode(%{} = msg, defs) do fixed_defs = for {{type, mod}, fields} <- defs, into: [] do @@ -26,9 +28,9 @@ defmodule Protobuf.Encoder do msg |> wrap_scalars(Utils.msg_defs(defs)) - |> fix_undefined + |> fix_undefined() |> Utils.convert_to_record(msg.__struct__) - |> tap(:gpb.verify_msg(&1, fixed_defs)) + |> tap(&:gpb.verify_msg(&1, fixed_defs)) |> :gpb.encode_msg(fixed_defs) end @@ -48,8 +50,7 @@ defmodule Protobuf.Encoder do defp fix_value(nil), do: :undefined - defp fix_value(values) when is_list(values), - do: Enum.map(values, &fix_value/1) + defp fix_value(values) when is_list(values), do: Enum.map(values, &fix_value/1) defp fix_value(%module{} = value) do value @@ -71,32 +72,32 @@ defmodule Protobuf.Encoder do |> Map.from_struct() |> Enum.reduce(msg, fn # nil is unwrapped - {_, nil}, acc = %_{} -> + {_, nil}, %_{} = acc -> acc # recursive wrap repeated - {k, v}, acc = %_{} when is_list(v) -> + {k, v}, %_{} = acc when is_list(v) -> Map.put(acc, k, Enum.map(v, &wrap_scalars(&1, defs))) # recursive wrap message - {k, {oneof, v = %_{}}}, acc = %_{} when is_atom(oneof) -> + {k, {oneof, %_{} = v}}, %_{} = acc when is_atom(oneof) -> Map.put(acc, k, {oneof, wrap_scalars(v, defs)}) - {k, v = %_{}}, acc = %_{} -> + {k, %_{} = v}, %_{} = acc -> Map.put(acc, k, wrap_scalars(v, defs)) # plain wrap scalar - {k, {oneof, v}}, acc = %_{} when is_atom(oneof) and Utils.is_scalar(v) -> + {k, {oneof, v}}, %_{} = acc when is_atom(oneof) and Utils.is_scalar(v) -> Map.put(acc, k, {oneof, do_wrap(v, [msg_module, k, oneof], defs)}) - {k, v}, acc = %_{} when Utils.is_scalar(v) -> + {k, v}, %_{} = acc when Utils.is_scalar(v) -> Map.put(acc, k, do_wrap(v, [msg_module, k], defs)) end) end defp wrap_scalars(v, %{}), do: v - defp do_wrap(v, keys = [_ | _], defs = %{}) do + defp do_wrap(v, [_ | _] = keys, %{} = defs) do case get_in(defs, keys) do %Field{type: scalar} when is_atom(scalar) -> v @@ -113,7 +114,7 @@ defmodule Protobuf.Encoder do end end - defp do_wrap_enum(v, module, defs = %{}) do + defp do_wrap_enum(v, module, %{} = defs) do case Enum.to_list(Map.get(defs, module)) do [value: %Field{type: {:enum, enum_module}}] -> if Utils.is_enum_wrapper(module, enum_module) do diff --git a/mix.lock b/mix.lock index 0ea1df8..321a802 100644 --- a/mix.lock +++ b/mix.lock @@ -1,10 +1,10 @@ %{ - "benchfella": {:hex, :benchfella, "0.3.5", "b2122c234117b3f91ed7b43b6e915e19e1ab216971154acd0a80ce0e9b8c05f5", [:mix], [], "hexpm"}, - "dialyxir": {:hex, :dialyxir, "0.5.1", "b331b091720fd93e878137add264bac4f644e1ddae07a70bf7062c7862c4b952", [:mix], [], "hexpm"}, - "earmark": {:hex, :earmark, "1.3.2", "b840562ea3d67795ffbb5bd88940b1bed0ed9fa32834915125ea7d02e35888a5", [:mix], [], "hexpm"}, - "ex_doc": {:hex, :ex_doc, "0.20.2", "1bd0dfb0304bade58beb77f20f21ee3558cc3c753743ae0ddbb0fd7ba2912331", [:mix], [{:earmark, "~> 1.3", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.10", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm"}, - "gpb": {:hex, :gpb, "4.5.1", "1e575446c9827d092208c433f6cfd9df41a0bcb511d1334cd02d811218362f27", [:make, :rebar], [], "hexpm"}, - "makeup": {:hex, :makeup, "0.8.0", "9cf32aea71c7fe0a4b2e9246c2c4978f9070257e5c9ce6d4a28ec450a839b55f", [:mix], [{:nimble_parsec, "~> 0.5.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm"}, - "makeup_elixir": {:hex, :makeup_elixir, "0.13.0", "be7a477997dcac2e48a9d695ec730b2d22418292675c75aa2d34ba0909dcdeda", [:mix], [{:makeup, "~> 0.8", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm"}, - "nimble_parsec": {:hex, :nimble_parsec, "0.5.0", "90e2eca3d0266e5c53f8fbe0079694740b9c91b6747f2b7e3c5d21966bba8300", [:mix], [], "hexpm"}, + "benchfella": {:hex, :benchfella, "0.3.5", "b2122c234117b3f91ed7b43b6e915e19e1ab216971154acd0a80ce0e9b8c05f5", [:mix], [], "hexpm", "23f27cbc482cbac03fc8926441eb60a5e111759c17642bac005c3225f5eb809d"}, + "dialyxir": {:hex, :dialyxir, "0.5.1", "b331b091720fd93e878137add264bac4f644e1ddae07a70bf7062c7862c4b952", [:mix], [], "hexpm", "6c32a70ed5d452c6650916555b1f96c79af5fc4bf286997f8b15f213de786f73"}, + "earmark": {:hex, :earmark, "1.3.2", "b840562ea3d67795ffbb5bd88940b1bed0ed9fa32834915125ea7d02e35888a5", [:mix], [], "hexpm", "e3be2bc3ae67781db529b80aa7e7c49904a988596e2dbff897425b48b3581161"}, + "ex_doc": {:hex, :ex_doc, "0.20.2", "1bd0dfb0304bade58beb77f20f21ee3558cc3c753743ae0ddbb0fd7ba2912331", [:mix], [{:earmark, "~> 1.3", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.10", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "8e24fc8ff9a50b9f557ff020d6c91a03cded7e59ac3e0eec8a27e771430c7d27"}, + "gpb": {:hex, :gpb, "4.21.1", "72e229c242d252d690addcfd04a6416c26c4d4d2c3521e05570a7a78b48d3bd1", [:make, :rebar3], [], "hexpm", "c05c9aea9e25bd341367a43b3d3eb68e951563911072259c5ec4cb6642f4ef22"}, + "makeup": {:hex, :makeup, "0.8.0", "9cf32aea71c7fe0a4b2e9246c2c4978f9070257e5c9ce6d4a28ec450a839b55f", [:mix], [{:nimble_parsec, "~> 0.5.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "5fbc8e549aa9afeea2847c0769e3970537ed302f93a23ac612602e805d9d1e7f"}, + "makeup_elixir": {:hex, :makeup_elixir, "0.13.0", "be7a477997dcac2e48a9d695ec730b2d22418292675c75aa2d34ba0909dcdeda", [:mix], [{:makeup, "~> 0.8", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "adf0218695e22caeda2820eaba703fa46c91820d53813a2223413da3ef4ba515"}, + "nimble_parsec": {:hex, :nimble_parsec, "0.5.0", "90e2eca3d0266e5c53f8fbe0079694740b9c91b6747f2b7e3c5d21966bba8300", [:mix], [], "hexpm", "5c040b8469c1ff1b10093d3186e2e10dbe483cd73d79ec017993fb3985b8a9b3"}, } From 9711baa6db2c802c58bed50cc76691a491020f4c Mon Sep 17 00:00:00 2001 From: Luis Almeida Date: Thu, 4 Jul 2024 17:43:32 +0100 Subject: [PATCH 3/4] fix depreacted syntax that gets compiled into dependencies --- lib/exprotobuf/decoder.ex | 34 ++++++++++++++++++---------------- lib/exprotobuf/encoder.ex | 4 ++-- lib/exprotobuf/utils.ex | 26 +++++++++----------------- 3 files changed, 29 insertions(+), 35 deletions(-) diff --git a/lib/exprotobuf/decoder.ex b/lib/exprotobuf/decoder.ex index 060d0f2..c6aeb94 100644 --- a/lib/exprotobuf/decoder.ex +++ b/lib/exprotobuf/decoder.ex @@ -1,13 +1,16 @@ defmodule Protobuf.Decoder do + @moduledoc false use Bitwise, only_operators: true - require Protobuf.Utils, as: Utils + alias Protobuf.Field alias Protobuf.OneOfField + require Protobuf.Utils, as: Utils + # Decode with record/module def decode(bytes, module) do defs = - for {{type, mod}, fields} <- module.defs, into: [] do + for {{type, mod}, fields} <- module.defs(), into: [] do case type do :msg -> {{:msg, mod}, @@ -30,7 +33,7 @@ defmodule Protobuf.Decoder do |> :gpb.decode_msg(module, defs) |> Utils.convert_from_record(module) |> convert_fields() - |> unwrap_scalars(Utils.msg_defs(module.defs)) + |> unwrap_scalars(Utils.msg_defs(module.defs())) end def varint(bytes) do @@ -42,13 +45,13 @@ defmodule Protobuf.Decoder do |> Map.from_struct() |> Map.keys() |> Enum.reduce(msg, fn field, msg -> - value = Map.get(msg, field) + value = Map.get(msg, field) - if value == :undefined do - Map.put(msg, field, get_default(module.syntax(), field, module)) - else - convert_field(value, msg, module.defs(:field, field)) - end + if value == :undefined do + Map.put(msg, field, get_default(module.syntax(), field, module)) + else + convert_field(value, msg, module.defs(:field, field)) + end end) end @@ -67,7 +70,7 @@ defmodule Protobuf.Decoder do "" ty -> - case :gpb.proto3_type_default(ty, module.defs) do + case :gpb.proto3_type_default(ty, module.defs()) do :undefined -> nil @@ -85,6 +88,7 @@ defmodule Protobuf.Decoder do for v <- value do convert_value(type, v) end + Map.put(msg, field, value) {_, :string} -> @@ -118,8 +122,7 @@ defmodule Protobuf.Decoder do end end - defp convert_value(:string, value), - do: :unicode.characters_to_binary(value) + defp convert_value(:string, value), do: :unicode.characters_to_binary(value) defp convert_value({:msg, _}, value) do value @@ -131,8 +134,7 @@ defmodule Protobuf.Decoder do {convert_value(key_type, key), convert_value(value_type, value)} end - defp convert_value(_, value), - do: value + defp convert_value(_, value), do: value defp unwrap_scalars(%msg_module{} = msg, %{} = defs) do msg @@ -164,7 +166,7 @@ defmodule Protobuf.Decoder do defp unwrap_scalars(v, %{}), do: v - defp do_unwrap(v = %_{}, keys = [_ | _], defs = %{}) do + defp do_unwrap(%_{} = v, [_ | _] = keys, %{} = defs) do %Field{type: {:msg, module}} = get_in(defs, keys) if Utils.is_standard_scalar_wrapper(module) do @@ -174,7 +176,7 @@ defmodule Protobuf.Decoder do end end - defp do_unwrap_enum(v = %_{}, module, defs = %{}) do + defp do_unwrap_enum(%_{} = v, module, %{} = defs) do case Enum.to_list(Map.get(defs, module)) do [value: %Field{type: {:enum, enum_module}}] -> if Utils.is_enum_wrapper(module, enum_module) do diff --git a/lib/exprotobuf/encoder.ex b/lib/exprotobuf/encoder.ex index 03c1718..ec4c6d0 100644 --- a/lib/exprotobuf/encoder.ex +++ b/lib/exprotobuf/encoder.ex @@ -107,7 +107,7 @@ defmodule Protobuf.Encoder do %Field{type: {:msg, module}} when is_atom(module) -> if Utils.is_standard_scalar_wrapper(module) do - Map.put(module.new, :value, v) + Map.put(module.new(), :value, v) else do_wrap_enum(v, module, defs) end @@ -118,7 +118,7 @@ defmodule Protobuf.Encoder do case Enum.to_list(Map.get(defs, module)) do [value: %Field{type: {:enum, enum_module}}] -> if Utils.is_enum_wrapper(module, enum_module) do - Map.put(module.new, :value, v) + Map.put(module.new(), :value, v) else v end diff --git a/lib/exprotobuf/utils.ex b/lib/exprotobuf/utils.ex index cd95462..3637e30 100644 --- a/lib/exprotobuf/utils.ex +++ b/lib/exprotobuf/utils.ex @@ -1,7 +1,7 @@ defmodule Protobuf.Utils do @moduledoc false - alias Protobuf.OneOfField alias Protobuf.Field + alias Protobuf.OneOfField @standard_scalar_wrappers %{ "Google.Protobuf.DoubleValue" => true, @@ -51,7 +51,7 @@ defmodule Protobuf.Utils do end def convert_to_record(map, module) do - module.record + module.record() |> Enum.reduce([record_name(module)], fn {key, default}, acc -> value = Map.get(map, key, default) [value_transform(module, value) | acc] @@ -61,24 +61,16 @@ defmodule Protobuf.Utils do end def msg_defs(defs) when is_list(defs) do - defs - |> Enum.reduce(%{}, fn - {{:msg, module}, meta}, acc = %{} -> - Map.put(acc, module, do_msg_defs(meta)) - - {{type, _}, _}, acc = %{} when type in [:enum, :extensions, :service, :group] -> - acc + Enum.reduce(defs, %{}, fn + {{:msg, module}, meta}, %{} = acc -> Map.put(acc, module, do_msg_defs(meta)) + {{type, _}, _}, %{} = acc when type in [:enum, :extensions, :service, :group] -> acc end) end defp do_msg_defs(defs) when is_list(defs) do - defs - |> Enum.reduce(%{}, fn - meta = %Field{name: name}, acc = %{} -> - Map.put(acc, name, meta) - - %OneOfField{name: name, fields: fields}, acc = %{} -> - Map.put(acc, name, do_msg_defs(fields)) + Enum.reduce(defs, %{}, fn + %Field{name: name} = meta, %{} = acc -> Map.put(acc, name, meta) + %OneOfField{name: name, fields: fields}, %{} = acc -> Map.put(acc, name, do_msg_defs(fields)) end) end @@ -97,7 +89,7 @@ defmodule Protobuf.Utils do def convert_from_record(rec, module) do map = struct(module) - module.record + module.record() |> Enum.with_index() |> Enum.reduce(map, fn {{key, _default}, idx}, acc -> # rec has the extra element when defines the record type From 3abb06fd5b2f336defd03e3e2d3c1be943fc03d6 Mon Sep 17 00:00:00 2001 From: Luis Almeida Date: Fri, 28 Feb 2025 11:37:21 +0000 Subject: [PATCH 4/4] replace Module.eval_quoted/4 with Code.eval_quoted/3 --- lib/exprotobuf/builder.ex | 21 ++++++++++++--------- lib/exprotobuf/decoder.ex | 2 +- lib/exprotobuf/define_message.ex | 6 +++--- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/exprotobuf/builder.ex b/lib/exprotobuf/builder.ex index 72c2c9c..3805e0f 100644 --- a/lib/exprotobuf/builder.ex +++ b/lib/exprotobuf/builder.ex @@ -1,18 +1,19 @@ defmodule Protobuf.Builder do @moduledoc false - alias Protobuf.Config - import Protobuf.DefineEnum, only: [def_enum: 3] import Protobuf.DefineMessage, only: [def_message: 3] + alias Protobuf.Config + def define(msgs, %Config{inject: inject} = config) do # When injecting, use_in is not available, so we don't need to use @before_compile if inject do quote location: :keep do - Module.register_attribute(__MODULE__, :use_in, accumulate: true) import unquote(__MODULE__), only: [use_in: 2] + Module.register_attribute(__MODULE__, :use_in, accumulate: true) + unless is_nil(unquote(config.from_file)) do case unquote(config.from_file) do file when is_binary(file) -> @@ -28,13 +29,14 @@ defmodule Protobuf.Builder do @config unquote(Macro.escape(Map.to_list(%{config | :schema => nil}))) @msgs unquote(Macro.escape(msgs)) contents = unquote(__MODULE__).generate(@msgs, @config) - Module.eval_quoted(__MODULE__, contents, [], __ENV__) + Code.eval_quoted(contents, [], __ENV__) end else quote do - Module.register_attribute(__MODULE__, :use_in, accumulate: true) import unquote(__MODULE__), only: [use_in: 2] + Module.register_attribute(__MODULE__, :use_in, accumulate: true) + unless is_nil(unquote(config.from_file)) do case unquote(config.from_file) do file when is_binary(file) -> @@ -57,7 +59,7 @@ defmodule Protobuf.Builder do defmacro __before_compile__(_env) do quote location: :keep do contents = unquote(__MODULE__).generate(@msgs, @config) - Module.eval_quoted(__MODULE__, contents, [], __ENV__) + Code.eval_quoted(contents, [], __ENV__) end end @@ -83,12 +85,13 @@ defmodule Protobuf.Builder do item_type in [:msg, :proto3_msg, :enum], into: [] do if only != [] do - is_child? = - Enum.any?(only, fn o -> + is_child? = + Enum.any?(only, fn o -> o != item_name and is_child_type?(item_name, o) end) - + last_mod = last_module(item_name) + if last_mod in only or is_child? do case item_type do :msg when is_child? -> diff --git a/lib/exprotobuf/decoder.ex b/lib/exprotobuf/decoder.ex index c6aeb94..ae9da07 100644 --- a/lib/exprotobuf/decoder.ex +++ b/lib/exprotobuf/decoder.ex @@ -1,6 +1,6 @@ defmodule Protobuf.Decoder do @moduledoc false - use Bitwise, only_operators: true + import Bitwise alias Protobuf.Field alias Protobuf.OneOfField diff --git a/lib/exprotobuf/define_message.ex b/lib/exprotobuf/define_message.ex index 964e7f1..bf40de7 100644 --- a/lib/exprotobuf/define_message.ex +++ b/lib/exprotobuf/define_message.ex @@ -2,10 +2,10 @@ defmodule Protobuf.DefineMessage do @moduledoc false alias Protobuf.Decoder + alias Protobuf.Delimited alias Protobuf.Encoder alias Protobuf.Field alias Protobuf.OneOfField - alias Protobuf.Delimited alias Protobuf.Utils def def_message(name, fields, inject: inject, doc: doc, syntax: syntax) when is_list(fields) do @@ -61,7 +61,7 @@ defmodule Protobuf.DefineMessage do unquote(constructors(name)) if use_in != nil do - Module.eval_quoted(__MODULE__, use_in, [], __ENV__) + Code.eval_quoted(use_in, [], __ENV__) end defimpl Protobuf.Serializable do @@ -76,7 +76,7 @@ defmodule Protobuf.DefineMessage do defp constructors(name) do quote location: :keep do - def new(), do: new([]) + def new, do: new([]) def new(values) do struct(unquote(name), values)