search

temporalio / cli

Command-line interface for running Temporal Server and interacting with Workflows, Activities, Namespaces, and other parts of Temporal
https://docs.temporal.io/cli
MIT License
247 stars 34 forks source link

Allow proto strings to contain invalid UTF-8 data #489

Closed tdeebswihart closed 6 months ago

tdeebswihart commented 6 months ago

What was changed

All calls to build or test this code now contains -tags protolegacy

Why?

We discovered last week that the version of gogo/protobuf we use allowed for invalid UTF-8 data in proto strings. This violates the proto3 spec as strings must be valid UTF-8, and the google/protobuf library will correctly fail to serialize and deserialize strings that contain invalid data.

As we can't be sure that an arbitrary temporal installation has well-behaved data we've had to hack around things to support it in google/protobuf.

cretz commented 6 months ago

Should we just not support non-utf8 by default in our pre-1.0 CLI and document how they can build it if they must have it?

tdeebswihart commented 6 months ago

Should we just not support non-utf8 by default in our pre-1.0 CLI and document how they can build it if they must have it?

I'm certainly a fan of that approach. I aimed for compatibility first-and-foremost but will happily change that here

tdeebswihart commented 6 months ago

We've discussed this and don't wish to enable invalid UTF-8 data in our new cli. Users who desire that behavior will need to build it with -tags protolegacy themselves