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

Fix for wrappers with a zero value #7195

Merged
merged 4 commits into from
Feb 11, 2020

Conversation

haberman
Copy link
Member

The previous optimizations for wrapper types had a bug that prevented
wrappers from registering as "present" if the "value" field was not
present on the wire.

In practice the "value" field will not be serialized when it is zero,
according to proto3 semantics, but due to the optimization this
prevented it from creating a new object to represent the presence of the
field.

The fix is to ensure that if the wrapper message is present on the wire,
we always initialize its value to zero.

Fixes #7029

djquan and others added 4 commits December 19, 2019 07:21
The previous optimizations for wrapper types had a bug that prevented
wrappers from registering as "present" if the "value" field was not
present on the wire.

In practice the "value" field will not be serialized when it is zero,
according to proto3 semantics, but due to the optimization this
prevented it from creating a new object to represent the presence of the
field.

The fix is to ensure that if the wrapper message is present on the wire,
we always initialize its value to zero.
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@djquan
Copy link
Contributor

djquan commented Feb 11, 2020

@googlebot I consent

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@haberman haberman merged commit 1a74ba4 into protocolbuffers:master Feb 11, 2020
rafi-kamal pushed a commit that referenced this pull request Feb 12, 2020
* Add failing tests for issues with wrapped values where the value is the default

* Add test for wrapped values without a value set

* Bugfix for wrapper types with default values.

The previous optimizations for wrapper types had a bug that prevented
wrappers from registering as "present" if the "value" field was not
present on the wire.

In practice the "value" field will not be serialized when it is zero,
according to proto3 semantics, but due to the optimization this
prevented it from creating a new object to represent the presence of the
field.

The fix is to ensure that if the wrapper message is present on the wire,
we always initialize its value to zero.

Co-authored-by: Dan Quan <dan@quan.io>
rafi-kamal added a commit that referenced this pull request Feb 12, 2020
* Add failing tests for issues with wrapped values where the value is the default

* Add test for wrapped values without a value set

* Bugfix for wrapper types with default values.

The previous optimizations for wrapper types had a bug that prevented
wrappers from registering as "present" if the "value" field was not
present on the wire.

In practice the "value" field will not be serialized when it is zero,
according to proto3 semantics, but due to the optimization this
prevented it from creating a new object to represent the presence of the
field.

The fix is to ensure that if the wrapper message is present on the wire,
we always initialize its value to zero.

Co-authored-by: Joshua Haberman <jhaberman@gmail.com>
Co-authored-by: Dan Quan <dan@quan.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ruby: Issue decoding wrapped values after upgrading from 3.10.1 to 3.11.1
4 participants