[protobuf] Issue 568 in protobuf: DynamicMessage.Builder#setField allows enum values to be set for enum fields, instead of EnumValueDescriptors only

Thu, 17 Oct 2013 14:35:02 -0700 

Status: New
Owner: liuj...@google.com
Labels: Type-Defect Priority-Medium
New issue 568 by t...@squareup.com: DynamicMessage.Builder#setField allows enum values to be set for enum fields, instead of EnumValueDescriptors only 
http://code.google.com/p/protobuf/issues/detail?id=568

What steps will reproduce the problem?

Consider this test:
@Test(expected = IllegalArgumentException.class) public void dynamicMessageSetFieldToValue() { 
    // This should throw IllegalArgumentException, but doesn't!
    DynamicMessage dynamicMessage = DynamicMessage.newBuilder(descriptor)
        .setField(fieldDescriptor, MyEnum.ONE).build();
// Throws ClassCastException when casting MyEnum.ONE to EnumValueDescriptor. 
    dynamicMessage.toString();
  }

With this protocol buffer enum and message:

enum MyEnum {
  ONE = 1;
}

message MyEnumMessage {
  optional MyEnum my_enum = 1;
}

What is the expected output? What do you see instead?
I would expect that DynamicMessage.Builder.setField throws if you try to set a field to an enum value, instead of the EnumValueDescriptor for the value.
What I see instead, is that setField happily accepts the enum value, but then toString on the message blows up with a ClassCastException because it tries to cast the enum value to an EnumValueDescriptor.
Additionally, apart from the TextFormat implementation, the following makes me believe EnumValueDescriptor is the type it should accept: - roundtripping the erroneous DynamicMessage through the serialized form coalesces the enum value to a EnumValueDescriptor, and - the reflection API (setField) on GeneratedMessage does throw IllegalArgumentException if you try to set an enum field to the enum value, instead of the EnumValueDescriptor. 

What version of the product are you using? On what operating system?
Mac OS X 10.8.5, protocol buffers 2.4.1. From inspecting source I'm pretty sure this issue still exists: FieldSet#verifyType still does only an "instanceof Internal.EnumLite" check, and ProtocolMessageEnum still extends Internal.EnumLite. 

---
Here are more tests that show the inconsistency. The fourth test is the one quoted above, and the only test that fails in the code snippet below. 

public class ProtocolMessageEnumTest {
static final Descriptors.Descriptor descriptor = MyEnumMessage.getDescriptor(); static final Descriptors.FieldDescriptor fieldDescriptor = descriptor.findFieldByName("my_enum"); 

  @Test public void generatedMessageSetMyEnum() {
MyEnumMessage message = MyEnumMessage.newBuilder().setMyEnum(MyEnum.ONE).build(); 

    assertThat(message.getMyEnum()).isEqualTo(MyEnum.ONE);
assertThat(message.getField(fieldDescriptor)).isEqualTo(MyEnum.ONE.getValueDescriptor()); 
  }
@Test(expected = IllegalArgumentException.class) public void generatedMessageSetFieldToValue() { // Throws IllegalArgumentException because EnumValueDescriptor is expected. 
    MyEnumMessage message = MyEnumMessage.newBuilder()
        .setField(fieldDescriptor, MyEnum.ONE).build();
  }

  @Test public void generatedMessageSetFieldToValueDescriptor() {
    MyEnumMessage message = MyEnumMessage.newBuilder()
        .setField(fieldDescriptor, MyEnum.ONE.getValueDescriptor()).build();

    assertThat(message.getMyEnum()).isEqualTo(MyEnum.ONE);
assertThat(message.getField(fieldDescriptor)).isEqualTo(MyEnum.ONE.getValueDescriptor()); 
  }
@Test(expected = IllegalArgumentException.class) public void dynamicMessageSetFieldToValue() { 
    // This should throw IllegalArgumentException, but doesn't!
    DynamicMessage dynamicMessage = DynamicMessage.newBuilder(descriptor)
        .setField(fieldDescriptor, MyEnum.ONE).build();
// Throws ClassCastException when casting MyEnum.ONE to EnumValueDescriptor. 
    dynamicMessage.toString();
  }

  @Test public void dynamicMessageSetFieldToValueDescriptor() {
    DynamicMessage dynamicMessage = DynamicMessage.newBuilder(descriptor)
        .setField(fieldDescriptor, MyEnum.ONE.getValueDescriptor()).build();

    dynamicMessage.toString();
  }
@Test public void generatedMessageToDynamicMessage() throws InvalidProtocolBufferException { MyEnumMessage message = MyEnumMessage.newBuilder().setMyEnum(MyEnum.ONE).build(); DynamicMessage dynamicMessage = DynamicMessage.parseFrom(descriptor, message.toByteString()); 

    assertThat(message.getMyEnum()).isEqualTo(MyEnum.ONE);
assertThat(message.getField(fieldDescriptor)).isEqualTo(MyEnum.ONE.getValueDescriptor()); assertThat(dynamicMessage.getField(fieldDescriptor)).isEqualTo(MyEnum.ONE.getValueDescriptor()); 
  }
@Test public void dynamicMessageSetFieldToValueToDynamicMessage() throws InvalidProtocolBufferException { 
    DynamicMessage dynamicMessage1 = DynamicMessage.newBuilder(descriptor)
        .setField(fieldDescriptor, MyEnum.ONE).build();
    DynamicMessage dynamicMessage2 =
DynamicMessage.parseFrom(descriptor, dynamicMessage1.toByteString());
assertThat(dynamicMessage1.getField(fieldDescriptor)).isEqualTo(MyEnum.ONE); assertThat(dynamicMessage2.getField(fieldDescriptor)).isEqualTo(MyEnum.ONE.getValueDescriptor()); 
  }
}


--
You received this message because this project is configured to send all issue notifications to this address. 
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

--
You received this message because you are subscribed to the Google Groups "Protocol 
Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To post to this group, send email to protobuf@googlegroups.com.
Visit this group at http://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to