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

WKT serialization is broken with golang/protobuf v1.4.0 and gogoproto_import = false #686

Open
xen0n opened this issue Apr 24, 2020 · 2 comments
Labels
apiv2 Issues blocking Go protobuf APIv2 compatibility

Comments

@xen0n
Copy link

xen0n commented Apr 24, 2020

tl;dr

The following combination is broken:

  • (gogoproto.gogoproto_import) = false in proto files
  • WKTs (google.protobuf.Timestamp and friends) are used
  • github.com/golang/protobuf version >= v1.4.0

WKT fields stop being recognized as such by gogo/protobuf/jsonpb in this case, breaking serialization.

Analysis

With option (gogoproto.gogoproto_import) = false set, WKTs are imported from github.com/golang/protobuf/ptypes packages.

For golang/protobuf >= v1.4.0 (more exactly golang/protobuf@b9f5089), the XXX_WellKnownType methods on WKT types are gone, but gogo/protobuf/jsonpb is not aware of the protobuf API v2.

protobuf/jsonpb/jsonpb.go

Lines 163 to 165 in 5628607

type isWkt interface {
XXX_WellKnownType() string
}

protobuf/jsonpb/jsonpb.go

Lines 206 to 207 in 5628607

// Handle well-known types.
if wkt, ok := v.(isWkt); ok {

Notably, one of the affected downstream packages is gogo/gateway, which is mandatory for grpc-gateway users depending on (gogoproto.stdtime) = true fields.

The protobuf API v2 compatibility, as implemented in grpc-gateway, cannot easily be ported to gogo/protobuf. Because golang/protobuf WKT types don't register to the gogo registry, the crucial proto.MessageName() call will return nothing. And the types cannot just be registered in app code either, because the gogo WKTs also have accompanying func init()'s doing the same thing, the names will collide if one does so.

If we don't allow gogo/protobuf/jsonpb to somehow lookup the message type in golang/protobuf registry, which would create a dependency on the latter package, the bug seems impossible to fix without bumping the fork (which I imagine is non-trivial work) or downgrading golang/protobuf (the easiest solution for now).

Any thoughts?

@FelixSeptem
Copy link

FelixSeptem commented Apr 30, 2020

same problem, any method to solve incompatible with v1.4.0 golang/protobuf ?

@xen0n
Copy link
Author

xen0n commented Apr 30, 2020

@FelixSeptem First check if you're affected (if you don't use (gogoproto.gogoproto_import) = false or gogo/gateway or gogo/protobuf/jsonpb then you're NOT affected). If you're as unfortunate as me, it seems the only short-term solution is to downgrade. Keeping 1.4.0 version of upstream protobuf seemingly needs updates to gogo/protobuf, which at this point seems unlikely, all maintainers disappeared last year without reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apiv2 Issues blocking Go protobuf APIv2 compatibility
Projects
None yet
Development

No branches or pull requests

3 participants