DynamicMessage.setField doesn't allow setting value on a repeated enum field!

[protobufel]

The DynamicMessage.setField has wrong check, preventing setting any ENUM type list on it!
This is a major bug; this has been introduced only in 2.6.0.

Here is the excerpt from the source:

public Builder setField(FieldDescriptor field, Object value) {
  verifyContainingType(field);
  ensureIsMutable();
  if (field.getType() == FieldDescriptor.Type.ENUM) {
    verifyEnumType(field, value);
  }

...
And verifyEnumType doesn't check for repeated value; here it is:

private void verifyEnumType(FieldDescriptor field, Object value) {
  if (value == null) {
    throw new NullPointerException();
  }
  if (!(value instanceof EnumValueDescriptor)) {
    throw new IllegalArgumentException(
      "DynamicMessage should use EnumValueDescriptor to set Enum Value.");
  }

...
If field is repeated, then its value is instanceof List, will never be EnumValueDescriptor!

[protobufel]

1. Is there any ETA for the fix? This bug renders entire DynamicMessage unusable - any merge, build, clone, parse, etc. methods won't work in the presence of any repeated enum field!

The bug is a MAJOR one, a showstopper!

Thanks in advance,
David

[xfxyjwf]

Sorry, we don't have an ETA for this. It should be fixed in the next minor release which hasn't been scheduled yet.

I believe most uses of DynamicMessage still work as we have unit-tests covering DynamicMessage:
https://github.com/google/protobuf/blob/master/java/src/test/java/com/google/protobuf/DynamicMessageTest.java

Presumably these tests are not failing despite of the bug. In most places we use addRepeatedField()/setRepeatedField() instead of setField() to manipulate repeated fields. Users could use these methods as a workaround as well.

[protobufel]

Nope, the merge uses setField, and the clone, new builder, and build, and
the rest use the merge!

This is really the showstopper! Update the tests and you'll see it!

Also, most of the users will care only about the Maven Central, so this
minor emergency release could be done just for Java there.
On Sep 10, 2014 1:12 PM, "xfxyjwf" [email protected] wrote:

Sorry, we don't have an ETA for this. It should be fixed in the next minor
release which hasn't been scheduled yet.

I believe most uses of DynamicMessage still work as we have unit-tests
covering DynamicMessage:

https://github.com/google/protobuf/blob/master/java/src/test/java/com/google/protobuf/DynamicMessageTest.java

Presumably these tests are not failing despite of the bug. In most places
we use addRepeatedField()/setRepeatedField() instead of setField() to
manipulate repeated fields. Users could use these methods as a workaround
as well.

—
Reply to this email directly or view it on GitHub
#17 (comment).