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 proto buffer pass by value, shallow copying or unsafe comparisons #1738

Closed
Martin2112 opened this issue Jun 27, 2019 · 1 comment
Closed
Assignees
Labels

Comments

@Martin2112
Copy link
Contributor

We need to update proto buffer usages where they are passed by value or compared by generic code like reflect.DeepEquals(). Passing them by value creates shallow copies that can share internal state. As the generated structs contain additional exported XXX_ fields managed by the proto implementation generic comparisons using all fields can produce incorrect results.

Typical problem cases:

  • Passing a proto as a value parameter to a func
  • Using a proto as a map key
  • Struct assignment of a proto in cases like *proto1= proto2
  • Using a proto as a value field in another struct (commonly test structs)
  • Using json.Marshal, reflect.DeepEquals or similar on protos

The scope of this should be limited as we don't use protos heavily outside gRPC interfaces and storage.

@Martin2112 Martin2112 self-assigned this Jun 27, 2019
Martin2112 added a commit to Martin2112/keytransparency that referenced this issue Jul 3, 2019
Martin2112 added a commit to google/keytransparency that referenced this issue Jul 3, 2019
* Fix proto copying in revisions and paginator tests.

Towards github.com/google/trillian#1738.

* Fix that nil / empty protos don't compare the same.
@Martin2112
Copy link
Contributor Author

Can't 100% guarantee that I've fixed all of them but I think I've got most of the ones in our repositories that can be found by go vet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant