Re: [protobuf] Possible C++ serialization issue with repeated fields

2014-06-18 Thread Ilia Mirkin
On Wed, Jun 18, 2014 at 12:26 PM, Justin jscad...@gmail.com wrote:
 My team and I noticed a potential bug in the serialization process, that
 seems unintended.

 If I defined a message structure as follows:

 message X {
required Y y = 1;
 }

 message Y {
repeated Things things = 1;
repeated Stuff stuff = 2;
 }

 message Things {
required string name = 1;
 }

 message Stuff {
required string name = 1;
 }


 When I create an object of type X, serialize and deserialize it, using the
 following code (for example):

 X x_original;
 std::string x_content;
 x_original.SerializeToString(x_content);

I believe this returns false, which indicates an error, since
x_original.y isn't set, and it's required.


 X x_new;
 x_new.ParseFromString(x_content);


 The ParseFromString call will throw an error :

[libprotobuf ERROR google/protobuf/message_lite.cc:123] Can't parse
 message of type testDomain.X because it is missing required fields: y


 Our intended use case for this is, in our example, letting another
 application know that there are no Things and no Stuff available, so I
 believe this should work as written.

Perhaps y should be optional then? As the proto is written, you must supply a y.


 if I, however, change the code to :

 X x_original;
 Y y;
 x_original.mutable_y()-CopyFrom(y);
 std::string x_content;
 x_original.SerializeToString(x_content);

 X x_new;
 x_new.ParseFromString(x_content);


 It works fine, as I believe would set_allocated_y() if I used a *y.

 Is this an intended 'feature' and should I be doing something different?

Yep, it's a feature -- if a field is required, it must be present in
order to serialize and deserialize without error.

  -ilia

-- 
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/d/optout.


Re: [protobuf] Possible C++ serialization issue with repeated fields

2014-06-18 Thread Justin


On Wednesday, June 18, 2014 12:36:27 PM UTC-4, Ilia Mirkin wrote:

 On Wed, Jun 18, 2014 at 12:26 PM, Justin jsca...@gmail.com javascript: 
 wrote: 
  My team and I noticed a potential bug in the serialization process, that 
  seems unintended. 
  
  If I defined a message structure as follows: 
  
  message X { 
 required Y y = 1; 
  } 
  
  message Y { 
 repeated Things things = 1; 
 repeated Stuff stuff = 2; 
  } 
  
  message Things { 
 required string name = 1; 
  } 
  
  message Stuff { 
 required string name = 1; 
  } 
  
  
  When I create an object of type X, serialize and deserialize it, using 
 the 
  following code (for example): 
  
  X x_original; 
  std::string x_content; 
  x_original.SerializeToString(x_content); 

 I believe this returns false, which indicates an error, since 
 x_original.y isn't set, and it's required. 


This actually doesn't return false, however a call to IsInitialized() does. 
In all of my testing, SerializeToString() returns true regardless of 
content. For example, if I change the code to this:

bool response = x_original.SerializeToString(x_content);
std::cout  Serialized:   std::boolalpha  response  std::endl;


I get,  Serialized: true
 


  
  X x_new; 
  x_new.ParseFromString(x_content); 
  
  
  The ParseFromString call will throw an error : 
  
 [libprotobuf ERROR google/protobuf/message_lite.cc:123] Can't parse 
  message of type testDomain.X because it is missing required fields: y 
  
  
  Our intended use case for this is, in our example, letting another 
  application know that there are no Things and no Stuff available, so I 
  believe this should work as written. 

 Perhaps y should be optional then? As the proto is written, you must 
 supply a y. 

 
I guess I assumed that it automatically creates the branches in the tree, 
but doesn't create the leaves, but if it only creates a branch for 
something that has a leaf, this functionality makes more sense to me.
 


  
  if I, however, change the code to : 
  
  X x_original; 
  Y y; 
  x_original.mutable_y()-CopyFrom(y); 
  std::string x_content; 
  x_original.SerializeToString(x_content); 
  
  X x_new; 
  x_new.ParseFromString(x_content); 
  
  
  It works fine, as I believe would set_allocated_y() if I used a *y. 
  
  Is this an intended 'feature' and should I be doing something different? 

 Yep, it's a feature -- if a field is required, it must be present in 
 order to serialize and deserialize without error. 


Thanks for the responses. We can work around this, but it's worth noting 
that it will serialize without error as written :).
 


   -ilia 


-- 
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/d/optout.


Re: [protobuf] Possible C++ serialization issue with repeated fields

2014-06-18 Thread 'Feng Xiao' via Protocol Buffers
On Wed, Jun 18, 2014 at 10:10 AM, Justin jscad...@gmail.com wrote:



 On Wednesday, June 18, 2014 12:36:27 PM UTC-4, Ilia Mirkin wrote:

 On Wed, Jun 18, 2014 at 12:26 PM, Justin jsca...@gmail.com wrote:
  My team and I noticed a potential bug in the serialization process,
 that
  seems unintended.
 
  If I defined a message structure as follows:
 
  message X {
 required Y y = 1;
  }
 
  message Y {
 repeated Things things = 1;
 repeated Stuff stuff = 2;
  }
 
  message Things {
 required string name = 1;
  }
 
  message Stuff {
 required string name = 1;
  }
 
 
  When I create an object of type X, serialize and deserialize it, using
 the
  following code (for example):
 
  X x_original;
  std::string x_content;
  x_original.SerializeToString(x_content);

 I believe this returns false, which indicates an error, since
 x_original.y isn't set, and it's required.


 This actually doesn't return false, however a call to IsInitialized()
 does. In all of my testing, SerializeToString() returns true regardless of
 content. For example, if I change the code to this:

 bool response = x_original.SerializeToString(x_content);
 std::cout  Serialized:   std::boolalpha  response  std::endl;


 I get,  Serialized: true

Yes, serialization will succeed even when there are missing required
fields. We do have a DCHECK(IsInitialized()) in the serialization code
though.





 
  X x_new;
  x_new.ParseFromString(x_content);
 
 
  The ParseFromString call will throw an error :
 
 [libprotobuf ERROR google/protobuf/message_lite.cc:123] Can't parse
  message of type testDomain.X because it is missing required fields: y
 
 
  Our intended use case for this is, in our example, letting another
  application know that there are no Things and no Stuff available, so I
  believe this should work as written.

 Perhaps y should be optional then? As the proto is written, you must
 supply a y.


 I guess I assumed that it automatically creates the branches in the tree,
 but doesn't create the leaves, but if it only creates a branch for
 something that has a leaf, this functionality makes more sense to me.



 
  if I, however, change the code to :
 
  X x_original;
  Y y;
  x_original.mutable_y()-CopyFrom(y);
  std::string x_content;
  x_original.SerializeToString(x_content);
 
  X x_new;
  x_new.ParseFromString(x_content);
 
 
  It works fine, as I believe would set_allocated_y() if I used a *y.
 
  Is this an intended 'feature' and should I be doing something
 different?

 Yep, it's a feature -- if a field is required, it must be present in
 order to serialize and deserialize without error.


 Thanks for the responses. We can work around this, but it's worth noting
 that it will serialize without error as written :).



   -ilia

  --
 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/d/optout.


-- 
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/d/optout.