encoding/protojson: add protojson editions tests including fuzztests

Change-Id: I478bf6a945cb2c86c71fd20b64dedb4b2e585f1d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/566035
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Lasse Folger <lassefolger@google.com>

encoding/protojson/decode_test.go

@@ -19,6 +19,7 @@ import (
 	weakpb "google.golang.org/protobuf/internal/testprotos/test/weak1"
 	pb2 "google.golang.org/protobuf/internal/testprotos/textpb2"
 	pb3 "google.golang.org/protobuf/internal/testprotos/textpb3"
+	pbeditions "google.golang.org/protobuf/internal/testprotos/textpbeditions"
 	"google.golang.org/protobuf/types/known/anypb"
 	"google.golang.org/protobuf/types/known/durationpb"
 	"google.golang.org/protobuf/types/known/emptypb"
@@ -84,6 +85,52 @@ func TestUnmarshal(t *testing.T) {
 			OptBytes:    []byte{},
 			OptString:   proto.String(""),
 		},
+	}, {
+		inputMessage: &pbeditions.Scalars{},
+		inputText:    "{}",
+		wantMessage:  &pbeditions.Scalars{},
+	}, {
+		desc:         "unexpected value instead of EOF",
+		inputMessage: &pbeditions.Scalars{},
+		inputText:    "{} {}",
+		wantErr:      `(line 1:4): unexpected token {`,
+	}, {
+		desc:         "proto2 optional scalars set to zero values",
+		inputMessage: &pbeditions.Scalars{},
+		inputText: `{
+  "optBool": false,
+  "optInt32": 0,
+  "optInt64": 0,
+  "optUint32": 0,
+  "optUint64": 0,
+  "optSint32": 0,
+  "optSint64": 0,
+  "optFixed32": 0,
+  "optFixed64": 0,
+  "optSfixed32": 0,
+  "optSfixed64": 0,
+  "optFloat": 0,
+  "optDouble": 0,
+  "optBytes": "",
+  "optString": ""
+}`,
+		wantMessage: &pbeditions.Scalars{
+			OptBool:     proto.Bool(false),
+			OptInt32:    proto.Int32(0),
+			OptInt64:    proto.Int64(0),
+			OptUint32:   proto.Uint32(0),
+			OptUint64:   proto.Uint64(0),
+			OptSint32:   proto.Int32(0),
+			OptSint64:   proto.Int64(0),
+			OptFixed32:  proto.Uint32(0),
+			OptFixed64:  proto.Uint64(0),
+			OptSfixed32: proto.Int32(0),
+			OptSfixed64: proto.Int64(0),
+			OptFloat:    proto.Float32(0),
+			OptDouble:   proto.Float64(0),
+			OptBytes:    []byte{},
+			OptString:   proto.String(""),
+		},
 	}, {
 		desc:         "proto3 scalars set to zero values",
 		inputMessage: &pb3.Scalars{},
@@ -155,6 +202,83 @@ func TestUnmarshal(t *testing.T) {
   "optString": null
 }`,
 		wantMessage: &pb2.Scalars{},
+	}, {
+		desc:         "protoeditions implicit scalars set to null",
+		inputMessage: &pbeditions.ImplicitScalars{},
+		inputText: `{
+  "sBool": null,
+  "sInt32": null,
+  "sInt64": null,
+  "sUint32": null,
+  "sUint64": null,
+  "sSint32": null,
+  "sSint64": null,
+  "sFixed32": null,
+  "sFixed64": null,
+  "sSfixed32": null,
+  "sSfixed64": null,
+  "sFloat": null,
+  "sDouble": null,
+  "sBytes": null,
+  "sString": null
+}`,
+		wantMessage: &pbeditions.ImplicitScalars{},
+	}, {
+		desc:         "boolean",
+		inputMessage: &pbeditions.ImplicitScalars{},
+		inputText:    `{"sBool": true}`,
+		wantMessage: &pbeditions.ImplicitScalars{
+			SBool: true,
+		},
+	}, {
+		desc:         "not boolean",
+		inputMessage: &pbeditions.ImplicitScalars{},
+		inputText:    `{"sBool": "true"}`,
+		wantErr:      `invalid value for bool type: "true"`,
+	}, {
+		desc:         "float and double",
+		inputMessage: &pbeditions.ImplicitScalars{},
+		inputText: `{
+  "sFloat": 1.234,
+  "sDouble": 5.678
+}`,
+		wantMessage: &pbeditions.ImplicitScalars{
+			SFloat:  1.234,
+			SDouble: 5.678,
+		},
+	}, {
+		desc:         "float and double in string",
+		inputMessage: &pbeditions.ImplicitScalars{},
+		inputText: `{
+  "sFloat": "1.234",
+  "sDouble": "5.678"
+}`,
+		wantMessage: &pbeditions.ImplicitScalars{
+			SFloat:  1.234,
+			SDouble: 5.678,
+		},
+	}, {
+		desc:         "float and double in E notation",
+		inputMessage: &pbeditions.ImplicitScalars{},
+		inputText: `{
+  "sFloat": 12.34E-1,
+  "sDouble": 5.678e4
+}`,
+		wantMessage: &pbeditions.ImplicitScalars{
+			SFloat:  1.234,
+			SDouble: 56780,
+		},
+	}, {
+		desc:         "float and double in string E notation",
+		inputMessage: &pbeditions.ImplicitScalars{},
+		inputText: `{
+  "sFloat": "12.34E-1",
+  "sDouble": "5.678e4"
+}`,
+		wantMessage: &pbeditions.ImplicitScalars{
+			SFloat:  1.234,
+			SDouble: 56780,
+		},
 	}, {
 		desc:         "proto3 scalars set to null",
 		inputMessage: &pb3.Scalars{},
@@ -2259,6 +2383,82 @@ func TestUnmarshal(t *testing.T) {
   }
 }`,
 		wantErr: `(line 5:14): invalid UTF-8`,
+	}, {
+		desc:         "well known types as field values in editions proto",
+		inputMessage: &pbeditions.KnownTypes{},
+		inputText: `{
+  "optBool": false,
+  "optInt32": 42,
+  "optInt64": "42",
+  "optUint32": 42,
+  "optUint64": "42",
+  "optFloat": 1.23,
+  "optDouble": 3.1415,
+  "optString": "hello",
+  "optBytes": "aGVsbG8=",
+  "optDuration": "123s",
+  "optTimestamp": "2019-03-19T23:03:21Z",
+  "optStruct": {
+    "string": "hello"
+  },
+  "optList": [
+    null,
+    "",
+    {},
+    []
+  ],
+  "optValue": "world",
+  "optEmpty": {},
+  "optAny": {
+    "@type": "google.protobuf.Empty",
+    "value": {}
+  },
+  "optFieldmask": "fooBar,barFoo"
+}`,
+		wantMessage: &pbeditions.KnownTypes{
+			OptBool:      &wrapperspb.BoolValue{Value: false},
+			OptInt32:     &wrapperspb.Int32Value{Value: 42},
+			OptInt64:     &wrapperspb.Int64Value{Value: 42},
+			OptUint32:    &wrapperspb.UInt32Value{Value: 42},
+			OptUint64:    &wrapperspb.UInt64Value{Value: 42},
+			OptFloat:     &wrapperspb.FloatValue{Value: 1.23},
+			OptDouble:    &wrapperspb.DoubleValue{Value: 3.1415},
+			OptString:    &wrapperspb.StringValue{Value: "hello"},
+			OptBytes:     &wrapperspb.BytesValue{Value: []byte("hello")},
+			OptDuration:  &durationpb.Duration{Seconds: 123},
+			OptTimestamp: &timestamppb.Timestamp{Seconds: 1553036601},
+			OptStruct: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					"string": {Kind: &structpb.Value_StringValue{"hello"}},
+				},
+			},
+			OptList: &structpb.ListValue{
+				Values: []*structpb.Value{
+					{Kind: &structpb.Value_NullValue{}},
+					{Kind: &structpb.Value_StringValue{}},
+					{
+						Kind: &structpb.Value_StructValue{
+							&structpb.Struct{Fields: map[string]*structpb.Value{}},
+						},
+					},
+					{
+						Kind: &structpb.Value_ListValue{
+							&structpb.ListValue{Values: []*structpb.Value{}},
+						},
+					},
+				},
+			},
+			OptValue: &structpb.Value{
+				Kind: &structpb.Value_StringValue{"world"},
+			},
+			OptEmpty: &emptypb.Empty{},
+			OptAny: &anypb.Any{
+				TypeUrl: "google.protobuf.Empty",
+			},
+			OptFieldmask: &fieldmaskpb.FieldMask{
+				Paths: []string{"foo_bar", "bar_foo"},
+			},
+		},
 	}, {
 		desc:         "well known types as field values",
 		inputMessage: &pb2.KnownTypes{},

encoding/protojson/fuzz_test.go

@@ -0,0 +1,101 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Go native fuzzing was added in go1.18. Remove this once we stop supporting
+// go1.17.
+//go:build go1.18
+
+package protojson_test
+
+import (
+	"math"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"google.golang.org/protobuf/encoding/protojson"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
+	"google.golang.org/protobuf/testing/protocmp"
+
+	testfuzzpb "google.golang.org/protobuf/internal/testprotos/editionsfuzztest"
+)
+
+// roundTripAndCompareProto tests if a protojson.Marshal/Unmarshal roundtrip
+// preserves the contents of the message. Note: wireBytes are a protocol
+// buffer wire format message, not the JSON formatted proto. We do this because
+// a random stream of bytes (e.g. generated by the fuzz engine) is more likely
+// to be valid proto wire format than that it is valid json format.
+func roundTripAndCompareProto(t *testing.T, wireBytes []byte, messages ...proto.Message) {
+	for _, msg := range messages {
+		src := msg.ProtoReflect().Type().New().Interface()
+
+		if err := proto.Unmarshal(wireBytes, src); err != nil {
+			// Ignoring invalid wire format since we want to test the protojson
+			// implementation, not the wireformat implementation.
+			return
+		}
+
+		// Unknown fields are not marshaled by protojson so we strip them.
+		src.ProtoReflect().SetUnknown(nil)
+		var ranger func(protoreflect.FieldDescriptor, protoreflect.Value) bool
+		stripUnknown := func(m protoreflect.Message) {
+			m.SetUnknown(nil)
+			m.Range(ranger)
+		}
+		ranger = func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {
+			switch {
+			case fd.IsMap():
+				if fd.MapValue().Message() != nil {
+					v.Map().Range(func(_ protoreflect.MapKey, v protoreflect.Value) bool {
+						stripUnknown(v.Message())
+						return true
+					})
+				}
+			case fd.Message() != nil:
+				if fd.Cardinality() == protoreflect.Repeated {
+					l := v.List()
+					for i := 0; i < l.Len(); i++ {
+						stripUnknown(l.Get(i).Message())
+					}
+				} else {
+					stripUnknown(v.Message())
+				}
+			}
+			return true
+		}
+		stripUnknown(src.ProtoReflect())
+
+		jsonBytes, err := protojson.Marshal(src)
+		if err != nil {
+			t.Errorf("failed to marshal messsage to json: %v\nmessage: %v", err, src)
+		}
+		dst := msg.ProtoReflect().Type().New().Interface()
+
+		if err := protojson.Unmarshal(jsonBytes, dst); err != nil {
+			t.Errorf("failed to unmarshal messsage from json: %v\njson: %s", err, jsonBytes)
+		}
+
+		// The cmp package does not deal with NaN on its own and will report
+		// NaN != NaN.
+		optNaN64 := cmp.Comparer(func(x, y float32) bool {
+			return (math.IsNaN(float64(x)) && math.IsNaN(float64(y))) || x == y
+		})
+		optNaN32 := cmp.Comparer(func(x, y float64) bool {
+			return (math.IsNaN(x) && math.IsNaN(y)) || x == y
+		})
+		if diff := cmp.Diff(src, dst, protocmp.Transform(), optNaN64, optNaN32); diff != "" {
+			t.Error(diff)
+		}
+	}
+}
+
+func FuzzEncodeDecodeRoundTrip(f *testing.F) {
+	f.Add([]byte("Hello World!"))
+	f.Fuzz(func(t *testing.T, in []byte) {
+		// We cannot test proto2 because it does not have UTF-8 validation
+		// but the JSON spec requires valid UTF-8 and thus we might initialize
+		// proto2 messages with invalid UTF-8 and then fail marshalling it.
+		roundTripAndCompareProto(t, in, (*testfuzzpb.TestAllTypesProto3)(nil), (*testfuzzpb.TestAllTypesProto3Editions)(nil))
+	})
+}

internal/testprotos/textpbeditions/test2.proto

@@ -0,0 +1,269 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test Protobuf definitions with proto2 syntax.
+edition = "2023";
+
+package pbeditions;
+
+import "google/protobuf/any.proto";
+import "google/protobuf/duration.proto";
+import "google/protobuf/empty.proto";
+import "google/protobuf/field_mask.proto";
+import "google/protobuf/struct.proto";
+import "google/protobuf/timestamp.proto";
+import "google/protobuf/wrappers.proto";
+
+option go_package = "google.golang.org/protobuf/internal/testprotos/textpbeditions";
+option features.enum_type = CLOSED;
+
+// Scalars contains scalar fields.
+message Scalars {
+  bool opt_bool = 1;
+  int32 opt_int32 = 2;
+  int64 opt_int64 = 3;
+  uint32 opt_uint32 = 4;
+  uint64 opt_uint64 = 5;
+  sint32 opt_sint32 = 6;
+  sint64 opt_sint64 = 7;
+  fixed32 opt_fixed32 = 8;
+  fixed64 opt_fixed64 = 9;
+  sfixed32 opt_sfixed32 = 10;
+  sfixed64 opt_sfixed64 = 11;
+
+  // Textproto marshal outputs fields in the same order as this proto
+  // definition regardless of field number. Following fields are intended to
+  // test that assumption.
+
+  float opt_float = 20;
+  double opt_double = 21;
+
+  bytes opt_bytes = 14;
+  string opt_string = 13;
+}
+
+// ImplicitScalars contains scalar field types with implicit field_presence
+message ImplicitScalars {
+  bool s_bool = 1 [features.field_presence = IMPLICIT];
+  int32 s_int32 = 2 [features.field_presence = IMPLICIT];
+  int64 s_int64 = 3 [features.field_presence = IMPLICIT];
+  uint32 s_uint32 = 4 [features.field_presence = IMPLICIT];
+  uint64 s_uint64 = 5 [features.field_presence = IMPLICIT];
+  sint32 s_sint32 = 6 [features.field_presence = IMPLICIT];
+  sint64 s_sint64 = 7 [features.field_presence = IMPLICIT];
+  fixed32 s_fixed32 = 8 [features.field_presence = IMPLICIT];
+  fixed64 s_fixed64 = 9 [features.field_presence = IMPLICIT];
+  sfixed32 s_sfixed32 = 10 [features.field_presence = IMPLICIT];
+  sfixed64 s_sfixed64 = 11 [features.field_presence = IMPLICIT];
+
+  // Textproto marshal outputs fields in the same order as this proto
+  // definition regardless of field number. Following fields are intended to
+  // test that assumption.
+
+  float s_float = 20 [features.field_presence = IMPLICIT];
+  double s_double = 21 [features.field_presence = IMPLICIT];
+
+  bytes s_bytes = 14 [features.field_presence = IMPLICIT];
+  string s_string = 13 [features.field_presence = IMPLICIT];
+}
+
+enum Enum {
+  ONE = 1;
+  TWO = 2;
+  TEN = 10;
+}
+
+// Message contains enum fields.
+message Enums {
+  Enum opt_enum = 1;
+  repeated Enum rpt_enum = 2;
+
+  enum NestedEnum {
+    UNO = 1;
+    DOS = 2;
+    DIEZ = 10;
+  }
+  NestedEnum opt_nested_enum = 3;
+  repeated NestedEnum rpt_nested_enum = 4;
+}
+
+// Message contains repeated fields.
+message Repeats {
+  repeated bool rpt_bool = 1;
+  repeated int32 rpt_int32 = 2;
+  repeated int64 rpt_int64 = 3;
+  repeated uint32 rpt_uint32 = 4;
+  repeated uint64 rpt_uint64 = 5;
+  repeated float rpt_float = 6;
+  repeated double rpt_double = 7;
+  repeated string rpt_string = 8;
+  repeated bytes rpt_bytes = 9;
+}
+
+// Message contains map fields.
+message Maps {
+  map<int32, string> int32_to_str = 1;
+  map<string, Nested> str_to_nested = 4;
+}
+
+// Message type used as submessage.
+message Nested {
+  string opt_string = 1;
+  Nested opt_nested = 2;
+}
+
+// Message contains message and group fields.
+message Nests {
+  Nested opt_nested = 1;
+  message OptGroup {
+    string opt_string = 1;
+    Nested opt_nested = 2;
+
+    message OptNestedGroup {
+      fixed32 opt_fixed32 = 1;
+    }
+    OptNestedGroup optnestedgroup = 3 [features.message_encoding = DELIMITED];
+  }
+  OptGroup optgroup = 2 [features.message_encoding = DELIMITED];
+
+  repeated Nested rpt_nested = 4;
+  message RptGroup {
+    repeated string rpt_string = 1;
+  }
+
+  repeated RptGroup rptgroup = 5 [
+    features.message_encoding = DELIMITED,
+    features.repeated_field_encoding = EXPANDED
+  ];
+
+  reserved reserved_field;
+}
+
+// Message contains required fields.
+message Requireds {
+  bool req_bool = 1 [features.field_presence = LEGACY_REQUIRED];
+  sfixed64 req_sfixed64 = 2 [features.field_presence = LEGACY_REQUIRED];
+  double req_double = 3 [features.field_presence = LEGACY_REQUIRED];
+  string req_string = 4 [features.field_presence = LEGACY_REQUIRED];
+  Enum req_enum = 5 [features.field_presence = LEGACY_REQUIRED];
+  Nested req_nested = 6 [features.field_presence = LEGACY_REQUIRED];
+}
+
+// Message contains both required and optional fields.
+message PartialRequired {
+  string req_string = 1 [features.field_presence = LEGACY_REQUIRED];
+  string opt_string = 2;
+}
+
+// Following messages are for testing required field nested in optional,
+// repeated and map fields.
+
+message NestedWithRequired {
+  string req_string = 1 [features.field_presence = LEGACY_REQUIRED];
+}
+
+message IndirectRequired {
+  NestedWithRequired opt_nested = 1 [features.field_presence = LEGACY_REQUIRED];
+  repeated NestedWithRequired rpt_nested = 2;
+  map<string, NestedWithRequired> str_to_nested = 3;
+
+  oneof union {
+    NestedWithRequired oneof_nested = 4;
+  }
+}
+
+// Following messages are for testing extensions.
+
+message Extensions {
+  string opt_string = 1;
+  extensions 20 to 100;
+  bool opt_bool = 101;
+  int32 opt_int32 = 2;
+}
+
+extend Extensions {
+  bool opt_ext_bool = 21;
+  string opt_ext_string = 22;
+  Enum opt_ext_enum = 23;
+  Nested opt_ext_nested = 24;
+  PartialRequired opt_ext_partial = 25;
+
+  repeated fixed32 rpt_ext_fixed32 = 31;
+  repeated Enum rpt_ext_enum = 32;
+  repeated Nested rpt_ext_nested = 33;
+}
+
+message ExtensionsContainer {
+  extend Extensions {
+    bool opt_ext_bool = 51;
+    string opt_ext_string = 52;
+    Enum opt_ext_enum = 53;
+    Nested opt_ext_nested = 54;
+    PartialRequired opt_ext_partial = 55;
+
+    repeated string rpt_ext_string = 61;
+    repeated Enum rpt_ext_enum = 62;
+    repeated Nested rpt_ext_nested = 63;
+  }
+}
+
+// Following messages are for testing MessageSet.
+
+message MessageSet {
+  option message_set_wire_format = true;
+
+  extensions 4 to max;
+}
+
+message MessageSetExtension {
+  string opt_string = 1;
+
+  extend MessageSet {
+    MessageSetExtension message_set_extension = 10;
+    MessageSetExtension not_message_set_extension = 20;
+    Nested ext_nested = 30;
+  }
+}
+
+message FakeMessageSet {
+  extensions 4 to max;
+}
+
+message FakeMessageSetExtension {
+  string opt_string = 1;
+
+  extend FakeMessageSet {
+    FakeMessageSetExtension message_set_extension = 10;
+  }
+}
+
+extend MessageSet {
+  FakeMessageSetExtension message_set_extension = 50;
+}
+
+// Message contains well-known type fields.
+message KnownTypes {
+  google.protobuf.BoolValue opt_bool = 1;
+  google.protobuf.Int32Value opt_int32 = 2;
+  google.protobuf.Int64Value opt_int64 = 3;
+  google.protobuf.UInt32Value opt_uint32 = 4;
+  google.protobuf.UInt64Value opt_uint64 = 5;
+  google.protobuf.FloatValue opt_float = 6;
+  google.protobuf.DoubleValue opt_double = 7;
+  google.protobuf.StringValue opt_string = 8;
+  google.protobuf.BytesValue opt_bytes = 9;
+
+  google.protobuf.Duration opt_duration = 20;
+  google.protobuf.Timestamp opt_timestamp = 21;
+
+  google.protobuf.Struct opt_struct = 25;
+  google.protobuf.ListValue opt_list = 26;
+  google.protobuf.Value opt_value = 27;
+  google.protobuf.NullValue opt_null = 28;
+
+  google.protobuf.Empty opt_empty = 30;
+  google.protobuf.Any opt_any = 32;
+
+  google.protobuf.FieldMask opt_fieldmask = 40;
+}

reflect/protodesc/desc_validate.go

@@ -107,7 +107,7 @@ func validateMessageDeclarations(ms []filedesc.Message, mds []*descriptorpb.Desc
 		if isMessageSet && !flags.ProtoLegacy {
 			return errors.New("message %q is a MessageSet, which is a legacy proto1 feature that is no longer supported", m.FullName())
 		}
-		if isMessageSet && (m.Syntax() != protoreflect.Proto2 || m.Fields().Len() > 0 || m.ExtensionRanges().Len() == 0) {
+		if isMessageSet && (m.Syntax() == protoreflect.Proto3 || m.Fields().Len() > 0 || m.ExtensionRanges().Len() == 0) {
 			return errors.New("message %q is an invalid proto1 MessageSet", m.FullName())
 		}
 		if m.Syntax() == protoreflect.Proto3 {

testing/prototest/prototest_test.go

@@ -29,6 +29,7 @@ func Test(t *testing.T) {
 		(*test3pb.TestAllTypes)(nil),
 		(*testeditionspb.TestAllTypes)(nil),
 		(*testpb.TestRequired)(nil),
+		(*testeditionspb.TestRequired)(nil),
 		(*irregularpb.Message)(nil),
 		(*testpb.TestAllExtensions)(nil),
 		(*testeditionspb.TestAllExtensions)(nil),