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

Regression in handling of 0 values for google.protobuf.Int64Value types (and maybe others) #7158

Closed
shidel-dev opened this issue Jan 30, 2020 · 6 comments

Comments

@shidel-dev
Copy link

What version of protobuf and what language are you using?
Version: Ruby 3.11.2

What did you do?
Simplified/Minimal Example.

message Wrapper {
  google.protobuf.Int64Value id = 1;
}
  Wrapper.decode(Wrapper.new( id: Google::Protobuf::Int64Value.new( value: 0) ).to_proto)

OUTOUT <Wrapper: id: nil>

What did you expect to see?
<Wrapper: id: <Google::Protobuf::Int64Value: value: 0>>

What did you see instead?
<Wrapper: id: nil>
Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs).

Anything else we should know about your project / environment?

This works on version 3.7.1 of the gem as expected. This is a very dangerous bug in my opinion, as it can break applications...

@shidel-dev
Copy link
Author

This is working as expected in 3.10.1 so it is just an issue in the two 3.11.X releases

@shidel-dev
Copy link
Author

Both versions encode the same, so it seems like an issue with decoding

@djquan
Copy link
Contributor

djquan commented Feb 4, 2020

I've seen this behavior too, and reproduced it in a test a while ago, #7037 . No response yet on that though

@shidel-dev
Copy link
Author

Thanks @djquan, I missed those other issues. This is probably a duplicate of #7029. This should probably have more attention than it is getting. This can cause serious bugs, as 0 values get dropped. It nearly caused a serious issue at my job.

@djquan
Copy link
Contributor

djquan commented Feb 4, 2020

Totally agree, it also almost caused an issue for us too

@haberman
Copy link
Member

This was fixed in #7195. Sorry about that!

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

3 participants