vbarach/protobuf-for-node

Fields of bytes should return Buffers.

Closed this issue · 3 comments

What steps will reproduce the problem?
1. Create a field in a message using "bytes".
2. Parse a serialized message containing bytes that are not valid UTF-8.
3. Get malformed data on the bytes field.

What is the expected output? What do you see instead?
I would expect the field to contain a Buffer, not a String.

What version of the product are you using? On what operating system?
Using the latest version of the node3 branch.

I've attached a patch against the latest version of protobuf_for_node.cc to 
provide this functionality, and to encode Buffers to bytes fields as well. 
(Although it still accepts strings, as well.) This patch may (read: probably 
will) break anything that currently uses bytes fields in protobuf-for-node, but 
it seems to be the "correct" solution.

Original issue reported on code.google.com by andy.sch...@gmail.com on 21 Dec 2010 at 9:44

Attachments:

Please double-check the code: I'm not used to editing V8 code, and there's a 
good chance I've done something wrong. (Especially with the const_cast in the 
parsing code.)

While you're looking at that section, the "String::AsciiValue ascii(value);" in 
setting string fields seems suspect (protobuf's string type handles UTF-8), but 
I'm not experienced enough with the V8 String object to say whether that call 
is problematic, and I haven't actually tested it yet.

Original comment by andy.sch...@gmail.com on 21 Dec 2010 at 9:48

Agree, had this in the back of my mind for a while. Will take a look at your 
patch.

Thanks
Matthias

Original comment by mer...@google.com on 23 Dec 2010 at 6:56

  • Changed state: Accepted
Thanks again. Fixed in 
http://code.google.com/p/protobuf-for-node/source/detail?r=58001008b42363b60b26b
c15b2466b5cd1f6f1b2:

Patch by andy.schmitz: Fields of bytes should return Buffers

This is not backward compatible in two ways:
* obviously, reading a Bytes field now returns a Buffer, not a string
* setting a bytes field to a string takes its UTF-8 value, not its ASCII value.
This is more consistent with the C++ api.

Original comment by mer...@google.com on 24 Dec 2010 at 12:07

  • Changed state: Fixed