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

Fix mangled Protobuf.EncodeError #386

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Fix mangled Protobuf.EncodeError #386

merged 1 commit into from
Jan 2, 2025

Conversation

v0idpwn
Copy link
Collaborator

@v0idpwn v0idpwn commented Dec 28, 2024

Because of the recursive implementation of encode_fields, unrelated
fields would get mixed up in encode errors.

For example, our struct %TestMsg.Foo has fields :a, :b, :c,
:d, e (and more...). If we had an error at :e, we would get the
error popping up for all fields coming before it:

iex(1)>  %TestMsg.Foo{a: 90, e: %TestMsg.Foo.Bar{a: "not_a_number"}} |> Protobuf.encode()
** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#a: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#b: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#c: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#d: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#e: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo.Bar#a: ** (Protobuf.EncodeError) "not_a_number" is invalid for type :int32
    (protobuf 0.13.0) lib/protobuf/encoder.ex:71: Protobuf.Encoder.encode_fields/5
    (protobuf 0.13.0) lib/protobuf/encoder.ex:33: Protobuf.Encoder.encode_with_message_props/2
    (protobuf 0.13.0) lib/protobuf/encoder.ex:18: Protobuf.Encoder.encode/1
    iex:1: (file)

After the patch, only the relevant fields are mentioned in the error:

iex(3)> %TestMsg.Foo{a: 90, e: %TestMsg.Foo.Bar{a: "not_a_number"}} |> Protobuf.encode()
** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#e: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo.Bar#a: ** (Protobuf.EncodeError) "not_a_number" is invalid for type :int32
    (protobuf 0.13.0) lib/protobuf/encoder.ex:75: Protobuf.Encoder.encode_field/4
    (protobuf 0.13.0) lib/protobuf/encoder.ex:63: Protobuf.Encoder.encode_fields/5
    (protobuf 0.13.0) lib/protobuf/encoder.ex:33: Protobuf.Encoder.encode_with_message_props/2
    (protobuf 0.13.0) lib/protobuf/encoder.ex:18: Protobuf.Encoder.encode/1
    iex:3: (file)

Because of the recursive implementation of `encode_fields`, unrelated
fields would get mixed up in encode errors.

For example, our struct `%TestMsg.Foo` has fields `:a`, `:b`, `:c`,
`:d`, `e` (and more...). If we had an error at `:e`, we would get the
error popping up for all fields coming before it:

```elixir
iex(1)>  %TestMsg.Foo{a: 90, e: %TestMsg.Foo.Bar{a: "not_a_number"}} |> Protobuf.encode()
** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#a: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#b: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#c: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#d: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#e: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo.Bar#a: ** (Protobuf.EncodeError) "not_a_number" is invalid for type :int32
    (protobuf 0.13.0) lib/protobuf/encoder.ex:71: Protobuf.Encoder.encode_fields/5
    (protobuf 0.13.0) lib/protobuf/encoder.ex:33: Protobuf.Encoder.encode_with_message_props/2
    (protobuf 0.13.0) lib/protobuf/encoder.ex:18: Protobuf.Encoder.encode/1
    iex:1: (file)
```

After the patch, only the relevant fields are mentioned in the error:
```elixir
iex(3)> %TestMsg.Foo{a: 90, e: %TestMsg.Foo.Bar{a: "not_a_number"}} |> Protobuf.encode()
** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo#e: ** (Protobuf.EncodeError) Got error when encoding TestMsg.Foo.Bar#a: ** (Protobuf.EncodeError) "not_a_number" is invalid for type :int32
    (protobuf 0.13.0) lib/protobuf/encoder.ex:75: Protobuf.Encoder.encode_field/4
    (protobuf 0.13.0) lib/protobuf/encoder.ex:63: Protobuf.Encoder.encode_fields/5
    (protobuf 0.13.0) lib/protobuf/encoder.ex:33: Protobuf.Encoder.encode_with_message_props/2
    (protobuf 0.13.0) lib/protobuf/encoder.ex:18: Protobuf.Encoder.encode/1
    iex:3: (file)
```
@v0idpwn v0idpwn force-pushed the fix/encoding-errors-bug branch from b4149d9 to 3827f89 Compare December 28, 2024 02:47
@v0idpwn v0idpwn changed the title Fix mangled Protobuf.EncodeError for nested structs Fix mangled Protobuf.EncodeError Dec 28, 2024
@v0idpwn v0idpwn merged commit 65faf65 into main Jan 2, 2025
3 checks passed
@v0idpwn v0idpwn deleted the fix/encoding-errors-bug branch January 2, 2025 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants