Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proto3 repeated elements do not work #99

Closed
SergejJurecko opened this issue Apr 30, 2018 · 7 comments
Closed

proto3 repeated elements do not work #99

SergejJurecko opened this issue Apr 30, 2018 · 7 comments

Comments

@SergejJurecko
Copy link

proto3 changes how repeated elements are serialized and quick-protobuf is incompatible. Is this a known issue?

@tafia
Copy link
Owner

tafia commented Apr 30, 2018

Can you be more specific?
It is supposed to work. Have you looked at the tests?

@SergejJurecko
Copy link
Author

I'm pretty sure this is the issue: https://developers.google.com/protocol-buffers/docs/encoding#packed

When using a different protocol buffers implementation server-side (erlang gpb) and specifying proto3 syntax, repeated types will not be packed by quick-protobuf and as a result will fail to deserialize on the server.

@SergejJurecko
Copy link
Author

I think the problem is that packing is used on non-primitive types.

syntax = "proto3";

message Embedded {
  int32 i = 1;
}

message WSMsg {
    repeated Embedded msgs = 1 [packed=true];
}

The above proto file will be considered invalid by protoc:
msg.proto:8:14: [packed = true] can only be specified for repeated primitive fields.

While it will compile fine with quick-protobuf/codegen
The resulting serialized data will be the same if packed=true or if packed modifier is not present. If packed is set to false, the binary data will be different and deserializable correctly with protoc --decode_raw

@SergejJurecko
Copy link
Author

You can try it with: https://github.com/SergejJurecko/qptest/

cargo run
make check

or

make protoc
make check

@tafia
Copy link
Owner

tafia commented May 2, 2018

That's interesting, thank you!

@tafia
Copy link
Owner

tafia commented Aug 15, 2018

Close for #68

@tafia tafia closed this as completed Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants