Status: New
Owner: ----
Labels: Type-Defect Priority-Medium
New issue 367 by [email protected]: MessageLite::ParseFromCodedStream:
Should check ConsumedEntireMessage()
http://code.google.com/p/protobuf/issues/detail?id=367
I'm unsure if this is a bug or not. I tried to ask on the mailing list, but
I think I'm stuck in the moderator queue.
In reading message_lite.cc I noticed that ::ParseFromCodedStream does *not*
call ConsumedEntireMessage(). Should it? It seems to me that it should,
especially since MergeFrom* has a big comment about "you must call
ConsumedEntireMessage()" and ParseFromCodedStream does *not* include this
warning. I can't think of any reason why a Parse* method should *not* make
this check, but I also don't really understand why this extra check is
needed.
As-is, I think this is somewhat dangerous. A quick search using Google
CodeSearch finds *many* instances where users call ParseFromCodedStream
without this extra check, which I think means they could get "bad" messages
without knowing it. Is my analysis true?
I've attached a diff of a quick and dirty unit test that fails with the
current SVN version of protobuf, as well as a proposed fix that passes make
check. If this seems sane, I'm happy to submit it as a code review. I
didn't do that because I'm not sure if this is the right thing to do.
If this fix is not correct, then at the very least ParseFromCodedStream
should be documented so users call it together with ConsumedEntireMessage()
Attachments:
parse.diff 2.8 KB
--
You received this message because you are subscribed to the Google Groups "Protocol
Buffers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/protobuf?hl=en.
