search

mafintosh / protocol-buffers

Protocol Buffers for Node.js
MIT License
762 stars 75 forks source link

The library should throw if a repeated field with non-primitive type is provided with [packed=true] #49

Open DavidBruant opened 8 years ago

DavidBruant commented 8 years ago

Only repeated fields of primitive numeric types (types which use the varint, 32-bit, or 64-bit wire types) can be declared "packed".

source

We got bitten. Trying to do it ([packed=true] + non-primitive type). What ended up happening was the decoding call returning nonsensical objects.

The library should probably refuse such message descriptions with a corresponding warning.

mafintosh commented 8 years ago

Oh good point. You up for sending a PR that validates this? :)

DavidBruant commented 8 years ago

Sure. Should it rather be fixed at the protocol-buffers-schema level? Maybe as a validation function that comes after parsing, but before returning the schema? I see other validation errors being thrown there ('Duplicate option ' + opt.name, msg.name + ' does not declare ' + field.tag + ' as an extension number')?

mafintosh commented 8 years ago

Ah good point. Yes lets fix it there On Tue, 24 Nov 2015 at 15:48, David Bruant notifications@github.com wrote:

Sure. Should it rather be fixed at the protocol-buffers-schema level? Maybe as a validation function that comes after parsing, but before returning the schema? I see other validation errors being thrown there ('Duplicate option ' + opt.name, msg.name + ' does not declare ' + field.tag + ' as an extension number')?

— Reply to this email directly or view it on GitHub https://github.com/mafintosh/protocol-buffers/issues/49#issuecomment-159400677 .