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.
