ebceu4 / protobuf-csharp-port

Automatically exported from code.google.com/p/protobuf-csharp-port
0 stars 0 forks source link

Inconsistent handling of nulls #1

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I am playing around with the C# protobuf implementation and I might have 
run into a problem.

When calling the RpcCallback with a null IMessage, the c# protobuf impl 
crashes in the AbstractBuilder with a null reference exception.

The funny thing is that when I run with a Java client, the java protobuf 
impl does not crash with a NullException. I attached a pic in the end 
pointing to the exception.

Looking at the 2 implementations, there a small difference on how the Java 
and C# protobuf were implemented:

JAVA

  public static <Type extends Message>

  RpcCallback<Message> generalizeCallback(
      final RpcCallback<Type> originalCallback,
      final Class<Type> originalClass,
      final Type defaultInstance) {
    return new RpcCallback<Message>() {
      public void run(Message parameter) {
        Type typedParameter;
        try {
        //This does not throw an exception if called with null Message.
        // So we don't fall into the catch stmt, avoiding the Null 
Exception at MergeFrom
          typedParameter = originalClass.cast(parameter);
        } catch (ClassCastException e) {
          typedParameter = copyAsType(defaultInstance, parameter);
        }
        originalCallback.run(typedParameter);
      }
    };
  }

C#
public static Action<IMessage> GeneralizeCallback<TMessage, TBuilder>
    (Action<TMessage> action, TMessage defaultInstance)
    where TMessage : class, IMessage<TMessage, TBuilder>
    where TBuilder : IBuilder<TMessage, TBuilder> {
   return message => {
      //This is null. So we fall into the if stmt, causing a Null Exception 
at MergeFrom
      TMessage castMessage = message as TMessage; 
      if (castMessage == null) { 
          castMessage = 
defaultInstance.CreateBuilderForType().MergeFrom(message).Build();
      }
      action(castMessage);
   };
}

Is there a way the cast done in the C# implementation could be done
differently such that both implementations would be consistent?

Original issue reported on code.google.com by jonathan.skeet on 12 Mar 2009 at 7:30

GoogleCodeExporter commented 9 years ago
I believe that you are asking the wrong question.  I don't think that the cast 
in the C# implementation should be done differently.  To mimic the Java 
behavior, you simply need to change the line:

if (castMessage == null) {

to

if (castMessage == null && message != null) {

The C# implementation will then match the behavior of the Java implementation 
exactly.

Original comment by bbro...@gmail.com on 25 Mar 2011 at 8:06

GoogleCodeExporter commented 9 years ago
I don't expect to do anything on this. We might revisit it when we look at the 
generated code for 3.0, but I suspect we won't look into it *specifically* - it 
may just drop out.

Original comment by jonathan.skeet on 15 Feb 2015 at 5:02