diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f59ea34..65022c8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Bug fixes + + * When decoding, a known field that arrives with a wire type other than the one its type uses is now kept in `__unknown_fields__` instead of raising `Protobuf.DecodeError` ("wrong wire_type for field ..."). This matches the reference implementations (C++, Java, Go, upb). A packed repeated field that arrives with a non-length-delimited wire type other than its element's is treated the same way instead of being decoded as packed data. + ## 0.17.1 * Harden depth limits when decoding. Each ordinary message, `Google.Protobuf.Any`, `Google.Protobuf.ListValue`, and `Google.Protobuf.Struct` adds one level, including the root. `Google.Protobuf.Value` uses the depth of its contents without adding a level of its own. This addresses [**CVE-2026-104635**](https://github.com/elixir-protobuf/protobuf/security/advisories/GHSA-m497-c2h9-rvw6). diff --git a/lib/protobuf/decoder.ex b/lib/protobuf/decoder.ex index 559f016b..87f456f3 100644 --- a/lib/protobuf/decoder.ex +++ b/lib/protobuf/decoder.ex @@ -256,7 +256,8 @@ defmodule Protobuf.Decoder do defp handle_value(<>, field_number, wire_type, value, message, props, nesting) do case props.field_props do - %{^field_number => %FieldProps{packed?: true, name_atom: name_atom} = prop} -> + %{^field_number => %FieldProps{packed?: true, name_atom: name_atom} = prop} + when wire_type == wire_delimited() -> new_message = update_in_message(message, name_atom, value, &value_for_packed/4, prop, nesting) @@ -281,10 +282,10 @@ defmodule Protobuf.Decoder do build_message(rest, new_message, props, nesting) - %{^field_number => %FieldProps{wire_type: expected, name: field}} -> - raise DecodeError, - message: "wrong wire_type for field #{field}: got #{wire_type}, expected #{expected}" - + # A known field whose wire type matches none of the clauses above is kept as an unknown + # field, like the reference implementations (C++, Java, Go, upb) do, instead of failing the + # whole message. The value has already been consumed according to its own wire type, so the + # rest of the message decodes normally and re-encoding preserves the original bytes. %{} -> %mod{__unknown_fields__: unknown_fields} = message diff --git a/test/protobuf/decoder_test.exs b/test/protobuf/decoder_test.exs index 073b6278..1c1670ba 100644 --- a/test/protobuf/decoder_test.exs +++ b/test/protobuf/decoder_test.exs @@ -26,9 +26,67 @@ defmodule Protobuf.DecoderTest do assert struct == %TestMsg.Foo{a: 42, c: "str", d: 123.5} end - test "raises for wrong wire type" do - assert_raise Protobuf.DecodeError, ~r{wrong wire_type for field a: got 1, expected 0}, fn -> - decode(<<9, 42, 0, 0, 0, 0, 0, 0, 0>>, TestMsg.Foo) + describe "known field with mismatched wire type" do + # Like the reference implementations (C++, Java, Go, upb), a known field whose wire type + # doesn't match the schema is kept as an unknown field instead of failing the whole message. + + test "int32 field encoded as fixed64 is kept as an unknown field" do + bin = <<9, 42, 0, 0, 0, 0, 0, 0, 0>> + struct = decode(bin, TestMsg.Foo) + + assert struct.a == 0 + assert struct.__unknown_fields__ == [{1, wire_64bits(), <<42, 0, 0, 0, 0, 0, 0, 0>>}] + assert Protobuf.Encoder.encode(struct) == bin + end + + test "string field encoded as varint is kept as an unknown field" do + # a = 42, then field 3 (string c) as varint 0 + bin = <<8, 42, 24, 0>> + struct = decode(bin, TestMsg.Foo) + + assert struct.a == 42 + assert struct.c == "" + assert struct.__unknown_fields__ == [{3, wire_varint(), 0}] + assert Protobuf.Encoder.encode(struct) == bin + end + + test "fields after the mismatched one are still decoded" do + # field 3 (string c) as varint 7, then a = 42 + struct = decode(<<24, 7, 8, 42>>, TestMsg.Foo) + + assert struct.a == 42 + assert struct.__unknown_fields__ == [{3, wire_varint(), 7}] + end + + test "embedded message field encoded as varint is kept as an unknown field" do + # field 6 (Foo.Bar e) as varint 1 + struct = decode(<<48, 1>>, TestMsg.Foo) + + assert struct.e == nil + assert struct.__unknown_fields__ == [{6, wire_varint(), 1}] + end + + test "packed repeated field encoded as fixed32 is kept as an unknown field" do + # field 10 (repeated int32 i, packed by default in proto3) as fixed32 + struct = decode(<<85, 1, 2, 3, 4>>, TestMsg.Foo) + + assert struct.i == [] + assert struct.__unknown_fields__ == [{10, wire_32bits(), <<1, 2, 3, 4>>}] + end + + test "map entry value encoded with a mismatched wire type falls back to the default" do + # field 13 (map l) entry: key = "k", value (int32) as fixed32 + struct = decode(<<106, 8, 10, 1, ?k, 21, 1, 0, 0, 0>>, TestMsg.Foo) + + assert struct.l == %{"k" => 0} + end + + test "packed repeated field still accepts the unpacked encoding" do + # field 10 (repeated int32 i) as two separate varints + struct = decode(<<80, 5, 80, 6>>, TestMsg.Foo) + + assert struct.i == [5, 6] + assert struct.__unknown_fields__ == [] end end