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

Panic when trying to get optional enum fields through reflection 2.24.1 #564

Closed
mardaker opened this issue Aug 5, 2021 · 1 comment
Closed

Comments

@mardaker
Copy link

mardaker commented Aug 5, 2021

Hi

Latest 2.24.1 panics with following stack when trying to retrieve optional enum field:

stack backtrace:
   0: std::panicking::begin_panic
             at /Users/arturm/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:519:12
   1: <protobuf::reflect::acc::v1::FieldAccessorImpl<M> as protobuf::reflect::acc::v1::FieldAccessorTrait>::get_enum_generic
             at /Users/arturm/.cargo/registry/src/github.com-1ecc6299db9ec823/protobuf-2.24.1/src/reflect/acc/v1.rs:252:18
   2: protobuf::reflect::field::FieldDescriptor::get_enum
             at /Users/arturm/.cargo/registry/src/github.com-1ecc6299db9ec823/protobuf-2.24.1/src/reflect/field.rs:123:32
   3: proto_bug::print_fields
             at ./src/main.rs:14:21
   4: proto_bug::main
             at ./src/main.rs:25:5

Attached minimal sample and proto files proto_bug.zip.

Possible fix:

diff --git a/protobuf/src/reflect/acc/v1.rs b/protobuf/src/reflect/acc/v1.rs
index 5cf53d54..17c2d027 100644
--- a/protobuf/src/reflect/acc/v1.rs
+++ b/protobuf/src/reflect/acc/v1.rs
@@ -249,6 +249,17 @@ impl<M: Message + 'static> FieldAccessorTrait for FieldAccessorImpl<M> {
                 get_set: SingularGetSet::Enum(ref get),
                 ..
             } => get.get_enum(message_down_cast(m)),
+            FieldAccessorFunctions::Optional(ref t) => {
+                match t
+                    .get_field(message_down_cast(m))
+                    .to_option()
+                    .expect("field unset")
+                    .as_ref()
+                {
+                    ReflectValueRef::Enum(m) => m,
+                    _ => panic!("not a enum"),
+                }
+            }
             _ => panic!(),
         }
     }
@mardaker mardaker changed the title Panic when trying to get optional enum fields trhough reflection 2.24.1 Panic when trying to get optional enum fields through reflection 2.24.1 Aug 5, 2021
mardaker added a commit to mardaker/rust-protobuf that referenced this issue Aug 5, 2021
stepancheg added a commit that referenced this issue Aug 8, 2021
stepancheg added a commit that referenced this issue Aug 8, 2021
(The fix is in v2 branch, this commit only merges the test, no fix
needed.)

Issue #564
@stepancheg
Copy link
Owner

Published 2.24.2. Thank you for the detailed repro and proposed fix!

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

No branches or pull requests

2 participants