Navigation Menu

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

C# addressbook update #628

Conversation

jtattermusch
Copy link
Contributor

Use the version of addressbook.proto from #627.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Jul 20, 2015

LGTM

jtattermusch added a commit that referenced this pull request Jul 21, 2015
@jtattermusch jtattermusch merged commit c792db5 into protocolbuffers:csharp-experimental Jul 21, 2015
taoso pushed a commit to taoso/protobuf that referenced this pull request Aug 1, 2018
The proto specification officially says that proto2 and proto3 strings should be
validated, but pragmatically, compliance with the spec has been poor.
For example, the Go implementation did not validate either and added strict
validation recently to be compliant. However, this caused signficant breakage.

Cases of breakage should change the proto field type from string to the bytes type.
However, this is not always possible, when the field is part of the exposed API.
This tends to be the case for proto2, where some other notable language
implementations (like C++) do not validate proto2 for valid UTF-8.
However, since most language implementations do validate for UTF-8 in proto3,
we keep that behavior.

Making this change for Go is a little tricky since each field does not necessarily
know whether it is operating under the proto2 or proto3 syntax. Thus, we modify
the generator to emit a "proto3" struct field tag for all fields in proto3.
The implications of this change is that people will need to regenerate their
proto files to have UTF-8 validation.

We expand UTF-8 validation tests to ensure this works for the cross-product of
(proto2, proto3) and (scalar, vector, oneof, and maps) fields with strings.

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

Successfully merging this pull request may close these issues.

None yet

3 participants