Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run verify_msg before encoding #116

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions lib/exprotobuf/decoder.ex
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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

Expand All @@ -85,6 +88,7 @@ defmodule Protobuf.Decoder do
for v <- value do
convert_value(type, v)
end

Map.put(msg, field, value)

{_, :string} ->
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
30 changes: 16 additions & 14 deletions lib/exprotobuf/encoder.ex
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -26,8 +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))
|> :gpb.encode_msg(fixed_defs)
end

Expand All @@ -47,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
Expand All @@ -70,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
Expand All @@ -105,18 +107,18 @@ 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
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
Map.put(module.new, :value, v)
Map.put(module.new(), :value, v)
else
v
end
Expand Down
26 changes: 9 additions & 17 deletions lib/exprotobuf/utils.ex
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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]
Expand All @@ -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

Expand All @@ -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
Expand Down
16 changes: 8 additions & 8 deletions mix.lock
Original file line number Diff line number Diff line change
@@ -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"},
}
8 changes: 7 additions & 1 deletion test/protobuf/encoder_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ defmodule Protobuf.Encoder.Test do
}
"""
end

#defmodule ExtensionsProto do
#use Protobuf, """
#message Msg {
Expand All @@ -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)
Expand Down