diff options
17 files changed, 537 insertions, 233 deletions
diff --git a/conformance/failure_list_csharp.txt b/conformance/failure_list_csharp.txt index feb17689..a46cee47 100644 --- a/conformance/failure_list_csharp.txt +++ b/conformance/failure_list_csharp.txt @@ -1,89 +1,16 @@ -DurationProtoInputTooLarge.JsonOutput -DurationProtoInputTooSmall.JsonOutput -FieldMaskNumbersDontRoundTrip.JsonOutput -FieldMaskPathsDontRoundTrip.JsonOutput -FieldMaskTooManyUnderscore.JsonOutput JsonInput.AnyWithValueForInteger.JsonOutput JsonInput.AnyWithValueForJsonObject.JsonOutput -JsonInput.BoolFieldAllCapitalFalse -JsonInput.BoolFieldAllCapitalTrue -JsonInput.BoolFieldCamelCaseFalse -JsonInput.BoolFieldCamelCaseTrue -JsonInput.BoolMapFieldKeyNotQuoted -JsonInput.BytesFieldInvalidBase64Characters -JsonInput.BytesFieldNoPadding -JsonInput.DoubleFieldInfinityNotQuoted -JsonInput.DoubleFieldNanNotQuoted -JsonInput.DoubleFieldNegativeInfinityNotQuoted -JsonInput.DoubleFieldTooLarge -JsonInput.DoubleFieldTooSmall -JsonInput.DurationHas3FractionalDigits.Validator -JsonInput.DurationHas6FractionalDigits.Validator -JsonInput.DurationHas9FractionalDigits.Validator -JsonInput.DurationMaxValue.JsonOutput -JsonInput.DurationMaxValue.ProtobufOutput -JsonInput.DurationMinValue.JsonOutput -JsonInput.DurationMinValue.ProtobufOutput -JsonInput.EnumFieldNotQuoted -JsonInput.EnumFieldNumericValueNonZero.JsonOutput -JsonInput.EnumFieldNumericValueNonZero.ProtobufOutput -JsonInput.EnumFieldNumericValueZero.JsonOutput -JsonInput.EnumFieldNumericValueZero.ProtobufOutput -JsonInput.EnumFieldUnknownValue.Validator -JsonInput.FieldMaskInvalidCharacter -JsonInput.FieldNameDuplicate -JsonInput.FieldNameDuplicateDifferentCasing2 JsonInput.FieldNameInLowerCamelCase.Validator JsonInput.FieldNameInSnakeCase.JsonOutput JsonInput.FieldNameInSnakeCase.ProtobufOutput -JsonInput.FieldNameNotQuoted JsonInput.FieldNameWithMixedCases.JsonOutput JsonInput.FieldNameWithMixedCases.ProtobufOutput JsonInput.FieldNameWithMixedCases.Validator -JsonInput.FloatFieldInfinityNotQuoted -JsonInput.FloatFieldNanNotQuoted -JsonInput.FloatFieldNegativeInfinityNotQuoted -JsonInput.Int32FieldLeadingZero JsonInput.Int32FieldMinFloatValue.JsonOutput JsonInput.Int32FieldMinValue.JsonOutput -JsonInput.Int32FieldNegativeWithLeadingZero -JsonInput.Int32FieldPlusSign -JsonInput.Int32MapFieldKeyNotQuoted JsonInput.Int64FieldMaxValueNotQuoted.JsonOutput JsonInput.Int64FieldMaxValueNotQuoted.ProtobufOutput -JsonInput.Int64MapFieldKeyNotQuoted -JsonInput.JsonWithComments -JsonInput.MapFieldKeyIsNull -JsonInput.MapFieldValueIsNull -JsonInput.OneofFieldDuplicate JsonInput.OriginalProtoFieldName.JsonOutput -JsonInput.OriginalProtoFieldName.ProtobufOutput -JsonInput.RepeatedBoolWrapper.ProtobufOutput -JsonInput.RepeatedDoubleWrapper.ProtobufOutput -JsonInput.RepeatedFieldMessageElementIsNull -JsonInput.RepeatedFieldPrimitiveElementIsNull -JsonInput.RepeatedFieldTrailingComma -JsonInput.RepeatedFloatWrapper.ProtobufOutput -JsonInput.RepeatedInt32Wrapper.ProtobufOutput -JsonInput.RepeatedInt64Wrapper.ProtobufOutput -JsonInput.RepeatedUint32Wrapper.ProtobufOutput -JsonInput.RepeatedUint64Wrapper.ProtobufOutput -JsonInput.StringFieldInvalidEscape -JsonInput.StringFieldSurrogateInWrongOrder JsonInput.StringFieldSurrogatePair.JsonOutput -JsonInput.StringFieldUnpairedHighSurrogate -JsonInput.StringFieldUnpairedLowSurrogate -JsonInput.StringFieldUnterminatedEscape -JsonInput.StringFieldUppercaseEscapeLetter -JsonInput.TimestampHas3FractionalDigits.Validator -JsonInput.TimestampHas6FractionalDigits.Validator -JsonInput.TimestampHas9FractionalDigits.Validator -JsonInput.TrailingCommaInAnObject -JsonInput.Uint32MapFieldKeyNotQuoted JsonInput.Uint64FieldMaxValueNotQuoted.JsonOutput JsonInput.Uint64FieldMaxValueNotQuoted.ProtobufOutput -JsonInput.Uint64MapFieldKeyNotQuoted -JsonInput.ValueAcceptNull.JsonOutput -JsonInput.ValueAcceptNull.ProtobufOutput -TimestampProtoInputTooLarge.JsonOutput -TimestampProtoInputTooSmall.JsonOutput diff --git a/csharp/src/Google.Protobuf.Conformance/Program.cs b/csharp/src/Google.Protobuf.Conformance/Program.cs index 8f72c8f9..f3f7e295 100644 --- a/csharp/src/Google.Protobuf.Conformance/Program.cs +++ b/csharp/src/Google.Protobuf.Conformance/Program.cs @@ -101,15 +101,26 @@ namespace Google.Protobuf.Conformance { return new ConformanceResponse { ParseError = e.Message }; } - switch (request.RequestedOutputFormat) + catch (InvalidJsonException e) { - case global::Conformance.WireFormat.JSON: - var formatter = new JsonFormatter(new JsonFormatter.Settings(false, typeRegistry)); - return new ConformanceResponse { JsonPayload = formatter.Format(message) }; - case global::Conformance.WireFormat.PROTOBUF: - return new ConformanceResponse { ProtobufPayload = message.ToByteString() }; - default: - throw new Exception("Unsupported request output format: " + request.PayloadCase); + return new ConformanceResponse { ParseError = e.Message }; + } + try + { + switch (request.RequestedOutputFormat) + { + case global::Conformance.WireFormat.JSON: + var formatter = new JsonFormatter(new JsonFormatter.Settings(false, typeRegistry)); + return new ConformanceResponse { JsonPayload = formatter.Format(message) }; + case global::Conformance.WireFormat.PROTOBUF: + return new ConformanceResponse { ProtobufPayload = message.ToByteString() }; + default: + throw new Exception("Unsupported request output format: " + request.PayloadCase); + } + } + catch (InvalidOperationException e) + { + return new ConformanceResponse { SerializeError = e.Message }; } } diff --git a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs index ace70b00..9e994a6a 100644 --- a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs @@ -165,34 +165,31 @@ namespace Google.Protobuf } [Test] - public void UnknownEnumValueOmitted_SingleField() + public void UnknownEnumValueNumeric_SingleField() { var message = new TestAllTypes { SingleForeignEnum = (ForeignEnum) 100 }; - AssertJson("{ }", JsonFormatter.Default.Format(message)); + AssertJson("{ 'singleForeignEnum': 100 }", JsonFormatter.Default.Format(message)); } [Test] - public void UnknownEnumValueOmitted_RepeatedField() + public void UnknownEnumValueNumeric_RepeatedField() { var message = new TestAllTypes { RepeatedForeignEnum = { ForeignEnum.FOREIGN_BAZ, (ForeignEnum) 100, ForeignEnum.FOREIGN_FOO } }; - AssertJson("{ 'repeatedForeignEnum': [ 'FOREIGN_BAZ', 'FOREIGN_FOO' ] }", JsonFormatter.Default.Format(message)); + AssertJson("{ 'repeatedForeignEnum': [ 'FOREIGN_BAZ', 100, 'FOREIGN_FOO' ] }", JsonFormatter.Default.Format(message)); } [Test] - public void UnknownEnumValueOmitted_MapField() + public void UnknownEnumValueNumeric_MapField() { - // This matches the C++ behaviour. var message = new TestMap { MapInt32Enum = { { 1, MapEnum.MAP_ENUM_FOO }, { 2, (MapEnum) 100 }, { 3, MapEnum.MAP_ENUM_BAR } } }; - AssertJson("{ 'mapInt32Enum': { '1': 'MAP_ENUM_FOO', '3': 'MAP_ENUM_BAR' } }", JsonFormatter.Default.Format(message)); + AssertJson("{ 'mapInt32Enum': { '1': 'MAP_ENUM_FOO', '2': 100, '3': 'MAP_ENUM_BAR' } }", JsonFormatter.Default.Format(message)); } [Test] - public void UnknownEnumValueOmitted_RepeatedField_AllEntriesUnknown() + public void UnknownEnumValue_RepeatedField_AllEntriesUnknown() { - // *Maybe* we should hold off on writing the "[" until we find that we've got at least one value to write... - // but this is what happens at the moment, and it doesn't seem too awful. var message = new TestAllTypes { RepeatedForeignEnum = { (ForeignEnum) 200, (ForeignEnum) 100 } }; - AssertJson("{ 'repeatedForeignEnum': [ ] }", JsonFormatter.Default.Format(message)); + AssertJson("{ 'repeatedForeignEnum': [ 200, 100 ] }", JsonFormatter.Default.Format(message)); } [Test] @@ -315,6 +312,15 @@ namespace Google.Protobuf [Test] [TestCase("1970-01-01T00:00:00Z", 0)] + [TestCase("1970-01-01T00:00:00.000000001Z", 1)] + [TestCase("1970-01-01T00:00:00.000000010Z", 10)] + [TestCase("1970-01-01T00:00:00.000000100Z", 100)] + [TestCase("1970-01-01T00:00:00.000001Z", 1000)] + [TestCase("1970-01-01T00:00:00.000010Z", 10000)] + [TestCase("1970-01-01T00:00:00.000100Z", 100000)] + [TestCase("1970-01-01T00:00:00.001Z", 1000000)] + [TestCase("1970-01-01T00:00:00.010Z", 10000000)] + [TestCase("1970-01-01T00:00:00.100Z", 100000000)] [TestCase("1970-01-01T00:00:00.100Z", 100000000)] [TestCase("1970-01-01T00:00:00.120Z", 120000000)] [TestCase("1970-01-01T00:00:00.123Z", 123000000)] @@ -350,6 +356,14 @@ namespace Google.Protobuf [TestCase(0, 0, "0s")] [TestCase(1, 0, "1s")] [TestCase(-1, 0, "-1s")] + [TestCase(0, 1, "0.000000001s")] + [TestCase(0, 10, "0.000000010s")] + [TestCase(0, 100, "0.000000100s")] + [TestCase(0, 1000, "0.000001s")] + [TestCase(0, 10000, "0.000010s")] + [TestCase(0, 100000, "0.000100s")] + [TestCase(0, 1000000, "0.001s")] + [TestCase(0, 10000000, "0.010s")] [TestCase(0, 100000000, "0.100s")] [TestCase(0, 120000000, "0.120s")] [TestCase(0, 123000000, "0.123s")] @@ -362,15 +376,20 @@ namespace Google.Protobuf [TestCase(0, -100000000, "-0.100s")] [TestCase(1, 100000000, "1.100s")] [TestCase(-1, -100000000, "-1.100s")] - // Non-normalized examples - [TestCase(1, 2123456789, "3.123456789s")] - [TestCase(1, -100000000, "0.900s")] public void DurationStandalone(long seconds, int nanoseconds, string expected) { Assert.AreEqual(WrapInQuotes(expected), new Duration { Seconds = seconds, Nanos = nanoseconds }.ToString()); } [Test] + [TestCase(1, 2123456789)] + [TestCase(1, -100000000)] + public void DurationStandalone_NonNormalized(long seconds, int nanoseconds) + { + Assert.Throws<InvalidOperationException>(() => new Duration { Seconds = seconds, Nanos = nanoseconds }.ToString()); + } + + [Test] public void DurationField() { var message = new TestWellKnownTypes { DurationField = new Duration() }; @@ -396,6 +415,16 @@ namespace Google.Protobuf } [Test] + [TestCase("foo__bar")] + [TestCase("foo_3_ar")] + [TestCase("fooBar")] + public void FieldMaskInvalid(string input) + { + var mask = new FieldMask { Paths = { input } }; + Assert.Throws<InvalidOperationException>(() => JsonFormatter.Default.Format(mask)); + } + + [Test] public void FieldMaskStandalone() { var fieldMask = new FieldMask { Paths = { "", "single", "with_underscore", "nested.field.name", "nested..double_dot" } }; diff --git a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs index 303baacc..612a7b12 100644 --- a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs @@ -72,6 +72,14 @@ namespace Google.Protobuf } [Test] + public void OriginalFieldNameAccepted() + { + var json = "{ \"single_int32\": 10 }"; + var expected = new TestAllTypes { SingleInt32 = 10 }; + Assert.AreEqual(expected, TestAllTypes.Parser.ParseJson(json)); + } + + [Test] public void SourceContextRoundtrip() { AssertRoundtrip(new SourceContext { FileName = "foo.proto" }); @@ -116,7 +124,9 @@ namespace Google.Protobuf [Test] public void SingularWrappers_ExplicitNulls() { - var message = new TestWellKnownTypes(); + // When we parse the "valueField": null part, we remember it... basically, it's one case + // where explicit default values don't fully roundtrip. + var message = new TestWellKnownTypes { ValueField = Value.ForNull() }; var json = new JsonFormatter(new JsonFormatter.Settings(true)).Format(message); var parsed = JsonParser.Default.Parse<TestWellKnownTypes>(json); Assert.AreEqual(message, parsed); @@ -143,6 +153,14 @@ namespace Google.Protobuf } [Test] + public void ExplicitNullValue() + { + string json = "{\"valueField\": null}"; + var message = JsonParser.Default.Parse<TestWellKnownTypes>(json); + Assert.AreEqual(new TestWellKnownTypes { ValueField = Value.ForNull() }, message); + } + + [Test] public void BytesWrapper_Standalone() { ByteString data = ByteString.CopyFrom(1, 2, 3); @@ -171,6 +189,36 @@ namespace Google.Protobuf } [Test] + public void RepeatedField_NullElementProhibited() + { + string json = "{ \"repeated_foreign_message\": [null] }"; + Assert.Throws<InvalidProtocolBufferException>(() => TestAllTypes.Parser.ParseJson(json)); + } + + [Test] + public void RepeatedField_NullOverallValueAllowed() + { + string json = "{ \"repeated_foreign_message\": null }"; + Assert.AreEqual(new TestAllTypes(), TestAllTypes.Parser.ParseJson(json)); + } + + [Test] + [TestCase("{ \"mapInt32Int32\": { \"10\": null }")] + [TestCase("{ \"mapStringString\": { \"abc\": null }")] + [TestCase("{ \"mapInt32ForeignMessage\": { \"10\": null }")] + public void MapField_NullValueProhibited(string json) + { + Assert.Throws<InvalidProtocolBufferException>(() => TestMap.Parser.ParseJson(json)); + } + + [Test] + public void MapField_NullOverallValueAllowed() + { + string json = "{ \"mapInt32Int32\": null }"; + Assert.AreEqual(new TestMap(), TestMap.Parser.ParseJson(json)); + } + + [Test] public void IndividualWrapperTypes() { Assert.AreEqual(new StringValue { Value = "foo" }, StringValue.Parser.ParseJson("\"foo\"")); @@ -715,7 +763,6 @@ namespace Google.Protobuf [TestCase("--0.123456789s", Description = "Double minus sign")] // Violate upper/lower bounds in various ways [TestCase("315576000001s", Description = "Integer part too large")] - [TestCase("315576000000.000000001s", Description = "Integer part is upper bound; non-zero fraction")] [TestCase("3155760000000s", Description = "Integer part too long (positive)")] [TestCase("-3155760000000s", Description = "Integer part too long (negative)")] public void Duration_Invalid(string jsonValue) @@ -742,6 +789,14 @@ namespace Google.Protobuf } [Test] + [TestCase("foo_bar")] + public void FieldMask_Invalid(string jsonValue) + { + string json = WrapInQuotes(jsonValue); + Assert.Throws<InvalidProtocolBufferException>(() => FieldMask.Parser.ParseJson(json)); + } + + [Test] public void Any_RegularMessage() { var registry = TypeRegistry.FromMessages(TestAllTypes.Descriptor); @@ -763,6 +818,13 @@ namespace Google.Protobuf } [Test] + public void Any_NoTypeUrl() + { + string json = "{ \"foo\": \"bar\" }"; + Assert.Throws<InvalidProtocolBufferException>(() => Any.Parser.ParseJson(json)); + } + + [Test] public void Any_WellKnownType() { var registry = TypeRegistry.FromMessages(Timestamp.Descriptor); @@ -814,6 +876,42 @@ namespace Google.Protobuf Assert.Throws<InvalidProtocolBufferException>(() => parser63.Parse<TestRecursiveMessage>(data64)); } + [Test] + [TestCase("AQI")] + [TestCase("_-==")] + public void Bytes_InvalidBase64(string badBase64) + { + string json = "{ \"singleBytes\": \"" + badBase64 + "\" }"; + Assert.Throws<InvalidProtocolBufferException>(() => TestAllTypes.Parser.ParseJson(json)); + } + + [Test] + [TestCase("\"FOREIGN_BAR\"", ForeignEnum.FOREIGN_BAR)] + [TestCase("5", ForeignEnum.FOREIGN_BAR)] + [TestCase("100", (ForeignEnum) 100)] + public void EnumValid(string value, ForeignEnum expectedValue) + { + string json = "{ \"singleForeignEnum\": " + value + " }"; + var parsed = TestAllTypes.Parser.ParseJson(json); + Assert.AreEqual(new TestAllTypes { SingleForeignEnum = expectedValue }, parsed); + } + + [Test] + [TestCase("\"NOT_A_VALID_VALUE\"")] + [TestCase("5.5")] + public void Enum_Invalid(string value) + { + string json = "{ \"singleForeignEnum\": " + value + " }"; + Assert.Throws<InvalidProtocolBufferException>(() => TestAllTypes.Parser.ParseJson(json)); + } + + [Test] + public void OneofDuplicate_Invalid() + { + string json = "{ \"oneofString\": \"x\", \"oneofUint32\": 10 }"; + Assert.Throws<InvalidProtocolBufferException>(() => TestAllTypes.Parser.ParseJson(json)); + } + /// <summary> /// Various tests use strings which have quotes round them for parsing or as the result /// of formatting, but without those quotes being specified in the tests (for the sake of readability). diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs index dfada6bd..2079a960 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs @@ -31,7 +31,7 @@ namespace Google.Protobuf.TestProtos { "L3Byb3RvYnVmL3NvdXJjZV9jb250ZXh0LnByb3RvGhxnb29nbGUvcHJvdG9i", "dWYvc3RydWN0LnByb3RvGh9nb29nbGUvcHJvdG9idWYvdGltZXN0YW1wLnBy", "b3RvGhpnb29nbGUvcHJvdG9idWYvdHlwZS5wcm90bxoeZ29vZ2xlL3Byb3Rv", - "YnVmL3dyYXBwZXJzLnByb3RvIpEHChJUZXN0V2VsbEtub3duVHlwZXMSJwoJ", + "YnVmL3dyYXBwZXJzLnByb3RvIr4HChJUZXN0V2VsbEtub3duVHlwZXMSJwoJ", "YW55X2ZpZWxkGAEgASgLMhQuZ29vZ2xlLnByb3RvYnVmLkFueRInCglhcGlf", "ZmllbGQYAiABKAsyFC5nb29nbGUucHJvdG9idWYuQXBpEjEKDmR1cmF0aW9u", "X2ZpZWxkGAMgASgLMhkuZ29vZ2xlLnByb3RvYnVmLkR1cmF0aW9uEisKC2Vt", @@ -51,7 +51,8 @@ namespace Google.Protobuf.TestProtos { "cm90b2J1Zi5VSW50MzJWYWx1ZRIuCgpib29sX2ZpZWxkGBAgASgLMhouZ29v", "Z2xlLnByb3RvYnVmLkJvb2xWYWx1ZRIyCgxzdHJpbmdfZmllbGQYESABKAsy", "HC5nb29nbGUucHJvdG9idWYuU3RyaW5nVmFsdWUSMAoLYnl0ZXNfZmllbGQY", - "EiABKAsyGy5nb29nbGUucHJvdG9idWYuQnl0ZXNWYWx1ZSKVBwoWUmVwZWF0", + "EiABKAsyGy5nb29nbGUucHJvdG9idWYuQnl0ZXNWYWx1ZRIrCgt2YWx1ZV9m", + "aWVsZBgTIAEoCzIWLmdvb2dsZS5wcm90b2J1Zi5WYWx1ZSKVBwoWUmVwZWF0", "ZWRXZWxsS25vd25UeXBlcxInCglhbnlfZmllbGQYASADKAsyFC5nb29nbGUu", "cHJvdG9idWYuQW55EicKCWFwaV9maWVsZBgCIAMoCzIULmdvb2dsZS5wcm90", "b2J1Zi5BcGkSMQoOZHVyYXRpb25fZmllbGQYAyADKAsyGS5nb29nbGUucHJv", @@ -162,7 +163,7 @@ namespace Google.Protobuf.TestProtos { descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData, new pbr::FileDescriptor[] { global::Google.Protobuf.WellKnownTypes.AnyReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.ApiReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.DurationReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.EmptyReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.FieldMaskReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.SourceContextReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.StructReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.TimestampReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.TypeReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.WrappersReflection.Descriptor, }, new pbr::GeneratedCodeInfo(null, new pbr::GeneratedCodeInfo[] { - new pbr::GeneratedCodeInfo(typeof(global::Google.Protobuf.TestProtos.TestWellKnownTypes), global::Google.Protobuf.TestProtos.TestWellKnownTypes.Parser, new[]{ "AnyField", "ApiField", "DurationField", "EmptyField", "FieldMaskField", "SourceContextField", "StructField", "TimestampField", "TypeField", "DoubleField", "FloatField", "Int64Field", "Uint64Field", "Int32Field", "Uint32Field", "BoolField", "StringField", "BytesField" }, null, null, null), + new pbr::GeneratedCodeInfo(typeof(global::Google.Protobuf.TestProtos.TestWellKnownTypes), global::Google.Protobuf.TestProtos.TestWellKnownTypes.Parser, new[]{ "AnyField", "ApiField", "DurationField", "EmptyField", "FieldMaskField", "SourceContextField", "StructField", "TimestampField", "TypeField", "DoubleField", "FloatField", "Int64Field", "Uint64Field", "Int32Field", "Uint32Field", "BoolField", "StringField", "BytesField", "ValueField" }, null, null, null), new pbr::GeneratedCodeInfo(typeof(global::Google.Protobuf.TestProtos.RepeatedWellKnownTypes), global::Google.Protobuf.TestProtos.RepeatedWellKnownTypes.Parser, new[]{ "AnyField", "ApiField", "DurationField", "EmptyField", "FieldMaskField", "SourceContextField", "StructField", "TimestampField", "TypeField", "DoubleField", "FloatField", "Int64Field", "Uint64Field", "Int32Field", "Uint32Field", "BoolField", "StringField", "BytesField" }, null, null, null), new pbr::GeneratedCodeInfo(typeof(global::Google.Protobuf.TestProtos.OneofWellKnownTypes), global::Google.Protobuf.TestProtos.OneofWellKnownTypes.Parser, new[]{ "AnyField", "ApiField", "DurationField", "EmptyField", "FieldMaskField", "SourceContextField", "StructField", "TimestampField", "TypeField", "DoubleField", "FloatField", "Int64Field", "Uint64Field", "Int32Field", "Uint32Field", "BoolField", "StringField", "BytesField" }, new[]{ "OneofField" }, null, null), new pbr::GeneratedCodeInfo(typeof(global::Google.Protobuf.TestProtos.MapWellKnownTypes), global::Google.Protobuf.TestProtos.MapWellKnownTypes.Parser, new[]{ "AnyField", "ApiField", "DurationField", "EmptyField", "FieldMaskField", "SourceContextField", "StructField", "TimestampField", "TypeField", "DoubleField", "FloatField", "Int64Field", "Uint64Field", "Int32Field", "Uint32Field", "BoolField", "StringField", "BytesField" }, null, null, new pbr::GeneratedCodeInfo[] { null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, }) @@ -215,6 +216,7 @@ namespace Google.Protobuf.TestProtos { BoolField = other.BoolField; StringField = other.StringField; BytesField = other.BytesField; + ValueField = other.valueField_ != null ? other.ValueField.Clone() : null; } public TestWellKnownTypes Clone() { @@ -410,6 +412,19 @@ namespace Google.Protobuf.TestProtos { } } + /// <summary>Field number for the "value_field" field.</summary> + public const int ValueFieldFieldNumber = 19; + private global::Google.Protobuf.WellKnownTypes.Value valueField_; + /// <summary> + /// Part of struct, but useful to be able to test separately + /// </summary> + public global::Google.Protobuf.WellKnownTypes.Value ValueField { + get { return valueField_; } + set { + valueField_ = value; + } + } + public override bool Equals(object other) { return Equals(other as TestWellKnownTypes); } @@ -439,6 +454,7 @@ namespace Google.Protobuf.TestProtos { if (BoolField != other.BoolField) return false; if (StringField != other.StringField) return false; if (BytesField != other.BytesField) return false; + if (!object.Equals(ValueField, other.ValueField)) return false; return true; } @@ -462,6 +478,7 @@ namespace Google.Protobuf.TestProtos { if (boolField_ != null) hash ^= BoolField.GetHashCode(); if (stringField_ != null) hash ^= StringField.GetHashCode(); if (bytesField_ != null) hash ^= BytesField.GetHashCode(); + if (valueField_ != null) hash ^= ValueField.GetHashCode(); return hash; } @@ -533,6 +550,10 @@ namespace Google.Protobuf.TestProtos { if (bytesField_ != null) { _single_bytesField_codec.WriteTagAndValue(output, BytesField); } + if (valueField_ != null) { + output.WriteRawTag(154, 1); + output.WriteMessage(ValueField); + } } public int CalculateSize() { @@ -591,6 +612,9 @@ namespace Google.Protobuf.TestProtos { if (bytesField_ != null) { size += _single_bytesField_codec.CalculateSizeWithTag(BytesField); } + if (valueField_ != null) { + size += 2 + pb::CodedOutputStream.ComputeMessageSize(ValueField); + } return size; } @@ -697,6 +721,12 @@ namespace Google.Protobuf.TestProtos { BytesField = other.BytesField; } } + if (other.valueField_ != null) { + if (valueField_ == null) { + valueField_ = new global::Google.Protobuf.WellKnownTypes.Value(); + } + ValueField.MergeFrom(other.ValueField); + } } public void MergeFrom(pb::CodedInputStream input) { @@ -832,6 +862,13 @@ namespace Google.Protobuf.TestProtos { } break; } + case 154: { + if (valueField_ == null) { + valueField_ = new global::Google.Protobuf.WellKnownTypes.Value(); + } + input.ReadMessage(valueField_); + break; + } } } } diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/DurationTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/DurationTest.cs index 36012e63..1aa02e16 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/DurationTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/DurationTest.cs @@ -50,11 +50,6 @@ namespace Google.Protobuf.WellKnownTypes // Rounding is towards 0 Assert.AreEqual(TimeSpan.FromTicks(2), new Duration { Nanos = 250 }.ToTimeSpan()); Assert.AreEqual(TimeSpan.FromTicks(-2), new Duration { Nanos = -250 }.ToTimeSpan()); - - // Non-normalized durations - Assert.AreEqual(TimeSpan.FromSeconds(3), new Duration { Seconds = 1, Nanos = 2 * Duration.NanosecondsPerSecond }.ToTimeSpan()); - Assert.AreEqual(TimeSpan.FromSeconds(1), new Duration { Seconds = 3, Nanos = -2 * Duration.NanosecondsPerSecond }.ToTimeSpan()); - Assert.AreEqual(TimeSpan.FromSeconds(-1), new Duration { Seconds = 1, Nanos = -2 * Duration.NanosecondsPerSecond }.ToTimeSpan()); } [Test] @@ -100,5 +95,30 @@ namespace Google.Protobuf.WellKnownTypes Assert.AreEqual(new Duration { Seconds = 1 }, Duration.FromTimeSpan(TimeSpan.FromSeconds(1))); Assert.AreEqual(new Duration { Nanos = Duration.NanosecondsPerTick }, Duration.FromTimeSpan(TimeSpan.FromTicks(1))); } + + [Test] + [TestCase(0, Duration.MaxNanoseconds + 1)] + [TestCase(0, Duration.MinNanoseconds - 1)] + [TestCase(Duration.MinSeconds - 1, 0)] + [TestCase(Duration.MaxSeconds + 1, 0)] + [TestCase(1, -1)] + [TestCase(-1, 1)] + public void ToTimeSpan_Invalid(long seconds, int nanoseconds) + { + var duration = new Duration { Seconds = seconds, Nanos = nanoseconds }; + Assert.Throws<InvalidOperationException>(() => duration.ToTimeSpan()); + } + + [Test] + [TestCase(0, Duration.MaxNanoseconds)] + [TestCase(0, Duration.MinNanoseconds)] + [TestCase(Duration.MinSeconds, Duration.MinNanoseconds)] + [TestCase(Duration.MaxSeconds, Duration.MaxNanoseconds)] + public void ToTimeSpan_Valid(long seconds, int nanoseconds) + { + // Only testing that these values don't throw, unlike their similar tests in ToTimeSpan_Invalid + var duration = new Duration { Seconds = seconds, Nanos = nanoseconds }; + duration.ToTimeSpan(); + } } } diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/TimestampTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/TimestampTest.cs index 597539eb..84717d66 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/TimestampTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/TimestampTest.cs @@ -61,6 +61,29 @@ namespace Google.Protobuf.WellKnownTypes Assert.AreEqual(new DateTime(1969, 12, 31, 23, 59, 59).AddMilliseconds(1), t2.ToDateTime()); } + [Test] + [TestCase(Timestamp.UnixSecondsAtBclMinValue - 1, Timestamp.MaxNanos)] + [TestCase(Timestamp.UnixSecondsAtBclMaxValue + 1, 0)] + [TestCase(0, -1)] + [TestCase(0, Timestamp.MaxNanos + 1)] + public void ToDateTime_OutOfRange(long seconds, int nanoseconds) + { + var value = new Timestamp { Seconds = seconds, Nanos = nanoseconds }; + Assert.Throws<InvalidOperationException>(() => value.ToDateTime()); + } + + // 1ns larger or smaller than the above values + [Test] + [TestCase(Timestamp.UnixSecondsAtBclMinValue, 0)] + [TestCase(Timestamp.UnixSecondsAtBclMaxValue, Timestamp.MaxNanos)] + [TestCase(0, 0)] + [TestCase(0, Timestamp.MaxNanos)] + public void ToDateTime_ValidBoundaries(long seconds, int nanoseconds) + { + var value = new Timestamp { Seconds = seconds, Nanos = nanoseconds }; + value.ToDateTime(); + } + private static void AssertRoundtrip(Timestamp timestamp, DateTime dateTime) { Assert.AreEqual(timestamp, Timestamp.FromDateTime(dateTime)); diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs index b72ef982..a2c833fe 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs @@ -149,6 +149,30 @@ namespace Google.Protobuf.WellKnownTypes } [Test] + public void RepeatedWrappersBinaryFormat() + { + // At one point we accidentally used a packed format for repeated wrappers, which is wrong (and weird). + // This test is just to prove that we use the right format. + + var rawOutput = new MemoryStream(); + var output = new CodedOutputStream(rawOutput); + // Write a value of 5 + output.WriteTag(RepeatedWellKnownTypes.Int32FieldFieldNumber, WireFormat.WireType.LengthDelimited); + output.WriteLength(2); + output.WriteTag(WrappersReflection.WrapperValueFieldNumber, WireFormat.WireType.Varint); + output.WriteInt32(5); + // Write a value of 0 (empty message) + output.WriteTag(RepeatedWellKnownTypes.Int32FieldFieldNumber, WireFormat.WireType.LengthDelimited); + output.WriteLength(0); + output.Flush(); + var expectedBytes = rawOutput.ToArray(); + + var message = new RepeatedWellKnownTypes { Int32Field = { 5, 0 } }; + var actualBytes = message.ToByteArray(); + Assert.AreEqual(expectedBytes, actualBytes); + } + + [Test] public void MapWrappersSerializeDeserialize() { // Note: no null values here, as they are prohibited in map fields diff --git a/csharp/src/Google.Protobuf/Collections/RepeatedField.cs b/csharp/src/Google.Protobuf/Collections/RepeatedField.cs index e3f65afe..1cde03bc 100644 --- a/csharp/src/Google.Protobuf/Collections/RepeatedField.cs +++ b/csharp/src/Google.Protobuf/Collections/RepeatedField.cs @@ -34,7 +34,6 @@ using System; using System.Collections; using System.Collections.Generic; using System.Text; -using Google.Protobuf.Compatibility; namespace Google.Protobuf.Collections { @@ -96,8 +95,8 @@ namespace Google.Protobuf.Collections // iteration. uint tag = input.LastTag; var reader = codec.ValueReader; - // Value types can be packed or not. - if (typeof(T).IsValueType() && WireFormat.GetTagWireType(tag) == WireFormat.WireType.LengthDelimited) + // Non-nullable value types can be packed or not. + if (FieldCodec<T>.IsPackedRepeatedField(tag)) { int length = input.ReadLength(); if (length > 0) @@ -134,7 +133,7 @@ namespace Google.Protobuf.Collections return 0; } uint tag = codec.Tag; - if (typeof(T).IsValueType() && WireFormat.GetTagWireType(tag) == WireFormat.WireType.LengthDelimited) + if (codec.PackedRepeatedField) { int dataSize = CalculatePackedDataSize(codec); return CodedOutputStream.ComputeRawVarint32Size(tag) + @@ -186,7 +185,7 @@ namespace Google.Protobuf.Collections } var writer = codec.ValueWriter; var tag = codec.Tag; - if (typeof(T).IsValueType() && WireFormat.GetTagWireType(tag) == WireFormat.WireType.LengthDelimited) + if (codec.PackedRepeatedField) { // Packed primitive type uint size = (uint)CalculatePackedDataSize(codec); diff --git a/csharp/src/Google.Protobuf/FieldCodec.cs b/csharp/src/Google.Protobuf/FieldCodec.cs index c2b8d268..98313088 100644 --- a/csharp/src/Google.Protobuf/FieldCodec.cs +++ b/csharp/src/Google.Protobuf/FieldCodec.cs @@ -30,6 +30,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using Google.Protobuf.Compatibility; using Google.Protobuf.WellKnownTypes; using System; using System.Collections.Generic; @@ -329,17 +330,24 @@ namespace Google.Protobuf } /// <summary> + /// <para> /// An encode/decode pair for a single field. This effectively encapsulates /// all the information needed to read or write the field value from/to a coded /// stream. + /// </para> + /// <para> + /// This class is public and has to be as it is used by generated code, but its public + /// API is very limited - just what the generated code needs to call directly. + /// </para> /// </summary> /// <remarks> - /// This never writes default values to the stream, and is not currently designed - /// to play well with packed arrays. + /// This never writes default values to the stream, and does not address "packedness" + /// in repeated fields itself, other than to know whether or not the field *should* be packed. /// </remarks> public sealed class FieldCodec<T> { private static readonly T DefaultDefault; + private static readonly bool TypeSupportsPacking = typeof(T).IsValueType() && Nullable.GetUnderlyingType(typeof(T)) == null; static FieldCodec() { @@ -354,75 +362,31 @@ namespace Google.Protobuf // Otherwise it's the default value of the CLR type } - private readonly Func<CodedInputStream, T> reader; - private readonly Action<CodedOutputStream, T> writer; - private readonly Func<T, int> sizeCalculator; - private readonly uint tag; - private readonly int tagSize; - private readonly int fixedSize; - // Default value for this codec. Usually the same for every instance of the same type, but - // for string/ByteString wrapper fields the codec's default value is null, whereas for - // other string/ByteString fields it's "" or ByteString.Empty. - private readonly T defaultValue; + internal static bool IsPackedRepeatedField(uint tag) => + TypeSupportsPacking && WireFormat.GetTagWireType(tag) == WireFormat.WireType.LengthDelimited; - internal FieldCodec( - Func<CodedInputStream, T> reader, - Action<CodedOutputStream, T> writer, - Func<T, int> sizeCalculator, - uint tag) : this(reader, writer, sizeCalculator, tag, DefaultDefault) - { - } - - internal FieldCodec( - Func<CodedInputStream, T> reader, - Action<CodedOutputStream, T> writer, - Func<T, int> sizeCalculator, - uint tag, - T defaultValue) - { - this.reader = reader; - this.writer = writer; - this.sizeCalculator = sizeCalculator; - this.fixedSize = 0; - this.tag = tag; - this.defaultValue = defaultValue; - tagSize = CodedOutputStream.ComputeRawVarint32Size(tag); - } - - internal FieldCodec( - Func<CodedInputStream, T> reader, - Action<CodedOutputStream, T> writer, - int fixedSize, - uint tag) - { - this.reader = reader; - this.writer = writer; - this.sizeCalculator = _ => fixedSize; - this.fixedSize = fixedSize; - this.tag = tag; - tagSize = CodedOutputStream.ComputeRawVarint32Size(tag); - } + internal bool PackedRepeatedField { get; } /// <summary> - /// Returns the size calculator for just a value. + /// Returns a delegate to write a value (unconditionally) to a coded output stream. /// </summary> - internal Func<T, int> ValueSizeCalculator { get { return sizeCalculator; } } + internal Action<CodedOutputStream, T> ValueWriter { get; } /// <summary> - /// Returns a delegate to write a value (unconditionally) to a coded output stream. + /// Returns the size calculator for just a value. /// </summary> - internal Action<CodedOutputStream, T> ValueWriter { get { return writer; } } + internal Func<T, int> ValueSizeCalculator { get; } /// <summary> /// Returns a delegate to read a value from a coded input stream. It is assumed that /// the stream is already positioned on the appropriate tag. /// </summary> - internal Func<CodedInputStream, T> ValueReader { get { return reader; } } + internal Func<CodedInputStream, T> ValueReader { get; } /// <summary> /// Returns the fixed size for an entry, or 0 if sizes vary. /// </summary> - internal int FixedSize { get { return fixedSize; } } + internal int FixedSize { get; } /// <summary> /// Gets the tag of the codec. @@ -430,15 +394,54 @@ namespace Google.Protobuf /// <value> /// The tag of the codec. /// </value> - public uint Tag { get { return tag; } } + internal uint Tag { get; } /// <summary> - /// Gets the default value of the codec's type. + /// Default value for this codec. Usually the same for every instance of the same type, but + /// for string/ByteString wrapper fields the codec's default value is null, whereas for + /// other string/ByteString fields it's "" or ByteString.Empty. /// </summary> /// <value> /// The default value of the codec's type. /// </value> - public T DefaultValue { get { return defaultValue; } } + internal T DefaultValue { get; } + + private readonly int tagSize; + + internal FieldCodec( + Func<CodedInputStream, T> reader, + Action<CodedOutputStream, T> writer, + int fixedSize, + uint tag) : this(reader, writer, _ => fixedSize, tag) + { + FixedSize = fixedSize; + } + + internal FieldCodec( + Func<CodedInputStream, T> reader, + Action<CodedOutputStream, T> writer, + Func<T, int> sizeCalculator, + uint tag) : this(reader, writer, sizeCalculator, tag, DefaultDefault) + { + } + + internal FieldCodec( + Func<CodedInputStream, T> reader, + Action<CodedOutputStream, T> writer, + Func<T, int> sizeCalculator, + uint tag, + T defaultValue) + { + ValueReader = reader; + ValueWriter = writer; + ValueSizeCalculator = sizeCalculator; + FixedSize = 0; + Tag = tag; + DefaultValue = defaultValue; + tagSize = CodedOutputStream.ComputeRawVarint32Size(tag); + // Detect packed-ness once, so we can check for it within RepeatedField<T>. + PackedRepeatedField = IsPackedRepeatedField(tag); + } /// <summary> /// Write a tag and the given value, *if* the value is not the default. @@ -447,8 +450,8 @@ namespace Google.Protobuf { if (!IsDefault(value)) { - output.WriteTag(tag); - writer(output, value); + output.WriteTag(Tag); + ValueWriter(output, value); } } @@ -457,23 +460,14 @@ namespace Google.Protobuf /// </summary> /// <param name="input">The input stream to read from.</param> /// <returns>The value read from the stream.</returns> - public T Read(CodedInputStream input) - { - return reader(input); - } + public T Read(CodedInputStream input) => ValueReader(input); /// <summary> /// Calculates the size required to write the given value, with a tag, /// if the value is not the default. /// </summary> - public int CalculateSizeWithTag(T value) - { - return IsDefault(value) ? 0 : sizeCalculator(value) + tagSize; - } + public int CalculateSizeWithTag(T value) => IsDefault(value) ? 0 : ValueSizeCalculator(value) + tagSize; - private bool IsDefault(T value) - { - return EqualityComparer<T>.Default.Equals(value, defaultValue); - } + private bool IsDefault(T value) => EqualityComparer<T>.Default.Equals(value, DefaultValue); } } diff --git a/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs b/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs index cacda648..eeb0f13a 100644 --- a/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs +++ b/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs @@ -30,6 +30,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#endregion
+using System;
using System.IO;
namespace Google.Protobuf
@@ -45,6 +46,11 @@ namespace Google.Protobuf {
}
+ internal InvalidProtocolBufferException(string message, Exception innerException)
+ : base(message, innerException)
+ {
+ }
+
internal static InvalidProtocolBufferException MoreDataAvailable()
{
return new InvalidProtocolBufferException(
@@ -82,6 +88,11 @@ namespace Google.Protobuf "Protocol message contained an invalid tag (zero).");
}
+ internal static InvalidProtocolBufferException InvalidBase64(Exception innerException)
+ {
+ return new InvalidProtocolBufferException("Invalid base64 data", innerException);
+ }
+
internal static InvalidProtocolBufferException InvalidEndTag()
{
return new InvalidProtocolBufferException(
diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index bde17903..61961c4c 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -205,11 +205,6 @@ namespace Google.Protobuf { continue; } - // Omit awkward (single) values such as unknown enum values - if (!field.IsRepeated && !field.IsMap && !CanWriteSingleValue(value)) - { - continue; - } // Okay, all tests complete: let's write the field value... if (!first) @@ -224,6 +219,31 @@ namespace Google.Protobuf return !first; } + /// <summary> + /// Camel-case converter with added strictness for field mask formatting. + /// </summary> + /// <exception cref="InvalidOperationException">The field mask is invalid for JSON representation</exception> + private static string ToCamelCaseForFieldMask(string input) + { + for (int i = 0; i < input.Length; i++) + { + char c = input[i]; + if (c >= 'A' && c <= 'Z') + { + throw new InvalidOperationException($"Invalid field mask to be converted to JSON: {input}"); + } + if (c == '_' && i < input.Length - 1) + { + char next = input[i + 1]; + if (next < 'a' || next > 'z') + { + throw new InvalidOperationException($"Invalid field mask to be converted to JSON: {input}"); + } + } + } + return ToCamelCase(input); + } + // Converted from src/google/protobuf/util/internal/utility.cc ToCamelCase // TODO: Use the new field in FieldDescriptor. internal static string ToCamelCase(string input) @@ -372,7 +392,14 @@ namespace Google.Protobuf } else if (value is System.Enum) { - WriteString(builder, value.ToString()); + if (System.Enum.IsDefined(value.GetType(), value)) + { + WriteString(builder, value.ToString()); + } + else + { + WriteValue(builder, (int) value); + } } else if (value is float || value is double) { @@ -485,13 +512,14 @@ namespace Google.Protobuf int nanos = (int) value.Descriptor.Fields[Timestamp.NanosFieldNumber].Accessor.GetValue(value); long seconds = (long) value.Descriptor.Fields[Timestamp.SecondsFieldNumber].Accessor.GetValue(value); - // Even if the original message isn't using the built-in classes, we can still build one... and then - // rely on it being normalized. - Timestamp normalized = Timestamp.Normalize(seconds, nanos); + // Even if the original message isn't using the built-in classes, we can still build one... and its + // conversion will check whether or not it's normalized. + // TODO: Perhaps the diagnostic-only formatter should not throw for non-normalized values? + Timestamp ts = new Timestamp { Seconds = seconds, Nanos = nanos }; // Use .NET's formatting for the value down to the second, including an opening double quote (as it's a string value) - DateTime dateTime = normalized.ToDateTime(); + DateTime dateTime = ts.ToDateTime(); builder.Append(dateTime.ToString("yyyy'-'MM'-'dd'T'HH:mm:ss", CultureInfo.InvariantCulture)); - AppendNanoseconds(builder, Math.Abs(normalized.Nanos)); + AppendNanoseconds(builder, Math.Abs(ts.Nanos)); builder.Append("Z\""); } @@ -502,25 +530,29 @@ namespace Google.Protobuf int nanos = (int) value.Descriptor.Fields[Duration.NanosFieldNumber].Accessor.GetValue(value); long seconds = (long) value.Descriptor.Fields[Duration.SecondsFieldNumber].Accessor.GetValue(value); + // TODO: Perhaps the diagnostic-only formatter should not throw for non-normalized values? // Even if the original message isn't using the built-in classes, we can still build one... and then // rely on it being normalized. - Duration normalized = Duration.Normalize(seconds, nanos); + if (!Duration.IsNormalized(seconds, nanos)) + { + throw new InvalidOperationException("Non-normalized duration value"); + } // The seconds part will normally provide the minus sign if we need it, but not if it's 0... - if (normalized.Seconds == 0 && normalized.Nanos < 0) + if (seconds == 0 && nanos < 0) { builder.Append('-'); } - builder.Append(normalized.Seconds.ToString("d", CultureInfo.InvariantCulture)); - AppendNanoseconds(builder, Math.Abs(normalized.Nanos)); + builder.Append(seconds.ToString("d", CultureInfo.InvariantCulture)); + AppendNanoseconds(builder, Math.Abs(nanos)); builder.Append("s\""); } private void WriteFieldMask(StringBuilder builder, IMessage value) { IList paths = (IList) value.Descriptor.Fields[FieldMask.PathsFieldNumber].Accessor.GetValue(value); - WriteString(builder, string.Join(",", paths.Cast<string>().Select(ToCamelCase))); + WriteString(builder, string.Join(",", paths.Cast<string>().Select(ToCamelCaseForFieldMask))); } private void WriteAny(StringBuilder builder, IMessage value) @@ -598,15 +630,15 @@ namespace Google.Protobuf // Output to 3, 6 or 9 digits. if (nanos % 1000000 == 0) { - builder.Append((nanos / 1000000).ToString("d", CultureInfo.InvariantCulture)); + builder.Append((nanos / 1000000).ToString("d3", CultureInfo.InvariantCulture)); } else if (nanos % 1000 == 0) { - builder.Append((nanos / 1000).ToString("d", CultureInfo.InvariantCulture)); + builder.Append((nanos / 1000).ToString("d6", CultureInfo.InvariantCulture)); } else { - builder.Append(nanos.ToString("d", CultureInfo.InvariantCulture)); + builder.Append(nanos.ToString("d9", CultureInfo.InvariantCulture)); } } } @@ -674,10 +706,6 @@ namespace Google.Protobuf bool first = true; foreach (var value in list) { - if (!CanWriteSingleValue(value)) - { - continue; - } if (!first) { builder.Append(PropertySeparator); @@ -695,10 +723,6 @@ namespace Google.Protobuf // This will box each pair. Could use IDictionaryEnumerator, but that's ugly in terms of disposal. foreach (DictionaryEntry pair in dictionary) { - if (!CanWriteSingleValue(pair.Value)) - { - continue; - } if (!first) { builder.Append(PropertySeparator); diff --git a/csharp/src/Google.Protobuf/JsonParser.cs b/csharp/src/Google.Protobuf/JsonParser.cs index 300a66b4..c07b16ea 100644 --- a/csharp/src/Google.Protobuf/JsonParser.cs +++ b/csharp/src/Google.Protobuf/JsonParser.cs @@ -168,6 +168,10 @@ namespace Google.Protobuf } var descriptor = message.Descriptor; var jsonFieldMap = descriptor.Fields.ByJsonName(); + // All the oneof fields we've already accounted for - we can only see each of them once. + // The set is created lazily to avoid the overhead of creating a set for every message + // we parsed, when oneofs are relatively rare. + HashSet<OneofDescriptor> seenOneofs = null; while (true) { token = tokenizer.Next(); @@ -183,6 +187,17 @@ namespace Google.Protobuf FieldDescriptor field; if (jsonFieldMap.TryGetValue(name, out field)) { + if (field.ContainingOneof != null) + { + if (seenOneofs == null) + { + seenOneofs = new HashSet<OneofDescriptor>(); + } + if (!seenOneofs.Add(field.ContainingOneof)) + { + throw new InvalidProtocolBufferException($"Multiple values specified for oneof {field.ContainingOneof.Name}"); + } + } MergeField(message, field, tokenizer); } else @@ -200,10 +215,15 @@ namespace Google.Protobuf var token = tokenizer.Next(); if (token.Type == JsonToken.TokenType.Null) { + // Clear the field if we see a null token, unless it's for a singular field of type + // google.protobuf.Value. // Note: different from Java API, which just ignores it. // TODO: Bring it more in line? Discuss... - field.Accessor.Clear(message); - return; + if (field.IsMap || field.IsRepeated || !IsGoogleProtobufValueField(field)) + { + field.Accessor.Clear(message); + return; + } } tokenizer.PushBack(token); @@ -239,6 +259,10 @@ namespace Google.Protobuf return; } tokenizer.PushBack(token); + if (token.Type == JsonToken.TokenType.Null) + { + throw new InvalidProtocolBufferException("Repeated field elements cannot be null"); + } list.Add(ParseSingleValue(field, tokenizer)); } } @@ -270,19 +294,30 @@ namespace Google.Protobuf } object key = ParseMapKey(keyField, token.StringValue); object value = ParseSingleValue(valueField, tokenizer); - // TODO: Null handling + if (value == null) + { + throw new InvalidProtocolBufferException("Map values must not be null"); + } dictionary[key] = value; } } + private static bool IsGoogleProtobufValueField(FieldDescriptor field) + { + return field.FieldType == FieldType.Message && + field.MessageType.FullName == Value.Descriptor.FullName; + } + private object ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer) { var token = tokenizer.Next(); if (token.Type == JsonToken.TokenType.Null) { - if (field.FieldType == FieldType.Message && field.MessageType.FullName == Value.Descriptor.FullName) + // TODO: In order to support dynamic messages, we should really build this up + // dynamically. + if (IsGoogleProtobufValueField(field)) { - return new Value { NullValue = NullValue.NULL_VALUE }; + return Value.ForNull(); } return null; } @@ -464,6 +499,11 @@ namespace Google.Protobuf { tokens.Add(token); token = tokenizer.Next(); + + if (tokenizer.ObjectDepth < typeUrlObjectDepth) + { + throw new InvalidProtocolBufferException("Any message with no @type"); + } } // Don't add the @type property or its value to the recorded token list @@ -603,6 +643,11 @@ namespace Google.Protobuf throw new InvalidProtocolBufferException($"Value out of range: {value}"); } return (float) value; + case FieldType.Enum: + CheckInteger(value); + // Just return it as an int, and let the CLR convert it. + // Note that we deliberately don't check that it's a known value. + return (int) value; default: throw new InvalidProtocolBufferException($"Unsupported conversion from JSON number for field type {field.FieldType}"); } @@ -633,7 +678,14 @@ namespace Google.Protobuf case FieldType.String: return text; case FieldType.Bytes: - return ByteString.FromBase64(text); + try + { + return ByteString.FromBase64(text); + } + catch (FormatException e) + { + throw InvalidProtocolBufferException.InvalidBase64(e); + } case FieldType.Int32: case FieldType.SInt32: case FieldType.SFixed32: @@ -826,28 +878,24 @@ namespace Google.Protobuf try { - long seconds = long.Parse(secondsText, CultureInfo.InvariantCulture); + long seconds = long.Parse(secondsText, CultureInfo.InvariantCulture) * multiplier; int nanos = 0; if (subseconds != "") { // This should always work, as we've got 1-9 digits. int parsedFraction = int.Parse(subseconds.Substring(1)); - nanos = parsedFraction * SubsecondScalingFactors[subseconds.Length]; + nanos = parsedFraction * SubsecondScalingFactors[subseconds.Length] * multiplier; } - if (seconds >= Duration.MaxSeconds) + if (!Duration.IsNormalized(seconds, nanos)) { - // Allow precisely 315576000000 seconds, but prohibit even 1ns more. - if (seconds > Duration.MaxSeconds || nanos > 0) - { - throw new InvalidProtocolBufferException("Invalid Duration value: " + token.StringValue); - } + throw new InvalidProtocolBufferException($"Invalid Duration value: {token.StringValue}"); } - message.Descriptor.Fields[Duration.SecondsFieldNumber].Accessor.SetValue(message, seconds * multiplier); - message.Descriptor.Fields[Duration.NanosFieldNumber].Accessor.SetValue(message, nanos * multiplier); + message.Descriptor.Fields[Duration.SecondsFieldNumber].Accessor.SetValue(message, seconds); + message.Descriptor.Fields[Duration.NanosFieldNumber].Accessor.SetValue(message, nanos); } catch (FormatException) { - throw new InvalidProtocolBufferException("Invalid Duration value: " + token.StringValue); + throw new InvalidProtocolBufferException($"Invalid Duration value: {token.StringValue}"); } } @@ -870,6 +918,8 @@ namespace Google.Protobuf private static string ToSnakeCase(string text) { var builder = new StringBuilder(text.Length * 2); + // Note: this is probably unnecessary now, but currently retained to be as close as possible to the + // C++, whilst still throwing an exception on underscores. bool wasNotUnderscore = false; // Initialize to false for case 1 (below) bool wasNotCap = false; @@ -903,7 +953,11 @@ namespace Google.Protobuf else { builder.Append(c); - wasNotUnderscore = c != '_'; + if (c == '_') + { + throw new InvalidProtocolBufferException($"Invalid field mask: {text}"); + } + wasNotUnderscore = true; wasNotCap = true; } } diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index d1732b2e..f43803a6 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -92,11 +92,22 @@ namespace Google.Protobuf.Reflection new FieldDescriptor(field, file, this, index, generatedCodeInfo?.PropertyNames[index])); fieldsInNumberOrder = new ReadOnlyCollection<FieldDescriptor>(fieldsInDeclarationOrder.OrderBy(field => field.FieldNumber).ToArray()); // TODO: Use field => field.Proto.JsonName when we're confident it's appropriate. (And then use it in the formatter, too.) - jsonFieldMap = new ReadOnlyDictionary<string, FieldDescriptor>(fieldsInNumberOrder.ToDictionary(field => JsonFormatter.ToCamelCase(field.Name))); + jsonFieldMap = CreateJsonFieldMap(fieldsInNumberOrder); file.DescriptorPool.AddSymbol(this); Fields = new FieldCollection(this); } + private static ReadOnlyDictionary<string, FieldDescriptor> CreateJsonFieldMap(IList<FieldDescriptor> fields) + { + var map = new Dictionary<string, FieldDescriptor>(); + foreach (var field in fields) + { + map[JsonFormatter.ToCamelCase(field.Name)] = field; + map[field.Name] = field; + } + return new ReadOnlyDictionary<string, FieldDescriptor>(map); + } + /// <summary> /// The brief name of the descriptor's target. /// </summary> @@ -255,9 +266,10 @@ namespace Google.Protobuf.Reflection // TODO: consider making this public in the future. (Being conservative for now...) /// <value> - /// Returns a read-only dictionary mapping the field names in this message as they're used + /// Returns a read-only dictionary mapping the field names in this message as they're available /// in the JSON representation to the field descriptors. For example, a field <c>foo_bar</c> - /// in the message would result in an entry with a key <c>fooBar</c>. + /// in the message would result two entries, one with a key <c>fooBar</c> and one with a key + /// <c>foo_bar</c>, both referring to the same field. /// </value> internal IDictionary<string, FieldDescriptor> ByJsonName() => messageDescriptor.jsonFieldMap; diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/DurationPartial.cs b/csharp/src/Google.Protobuf/WellKnownTypes/DurationPartial.cs index 324f48fc..b8eba9d3 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/DurationPartial.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/DurationPartial.cs @@ -57,15 +57,38 @@ namespace Google.Protobuf.WellKnownTypes /// </summary> public const long MinSeconds = -315576000000L; + internal const int MaxNanoseconds = NanosecondsPerSecond - 1; + internal const int MinNanoseconds = -NanosecondsPerSecond + 1; + + internal static bool IsNormalized(long seconds, int nanoseconds) + { + // Simple boundaries + if (seconds < MinSeconds || seconds > MaxSeconds || + nanoseconds < MinNanoseconds || nanoseconds > MaxNanoseconds) + { + return false; + } + // We only have a problem is one is strictly negative and the other is + // strictly positive. + return Math.Sign(seconds) * Math.Sign(nanoseconds) != -1; + } + + /// <summary> /// Converts this <see cref="Duration"/> to a <see cref="TimeSpan"/>. /// </summary> /// <remarks>If the duration is not a precise number of ticks, it is truncated towards 0.</remarks> /// <returns>The value of this duration, as a <c>TimeSpan</c>.</returns> + /// <exception cref="InvalidOperationException">This value isn't a valid normalized duration, as + /// described in the documentation.</exception> public TimeSpan ToTimeSpan() { checked { + if (!IsNormalized(Seconds, Nanos)) + { + throw new InvalidOperationException("Duration was not a valid normalized duration"); + } long ticks = Seconds * TimeSpan.TicksPerSecond + Nanos / NanosecondsPerTick; return TimeSpan.FromTicks(ticks); } diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/TimestampPartial.cs b/csharp/src/Google.Protobuf/WellKnownTypes/TimestampPartial.cs index d284acd6..7c50a3d7 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/TimestampPartial.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/TimestampPartial.cs @@ -37,9 +37,17 @@ namespace Google.Protobuf.WellKnownTypes public partial class Timestamp { private static readonly DateTime UnixEpoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc); - private static readonly long BclSecondsAtUnixEpoch = UnixEpoch.Ticks / TimeSpan.TicksPerSecond; - internal static readonly long UnixSecondsAtBclMinValue = -BclSecondsAtUnixEpoch; - internal static readonly long UnixSecondsAtBclMaxValue = (DateTime.MaxValue.Ticks / TimeSpan.TicksPerSecond) - BclSecondsAtUnixEpoch; + // Constants determined programmatically, but then hard-coded so they can be constant expressions. + private const long BclSecondsAtUnixEpoch = 62135596800; + internal const long UnixSecondsAtBclMaxValue = 253402300799; + internal const long UnixSecondsAtBclMinValue = -BclSecondsAtUnixEpoch; + internal const int MaxNanos = Duration.NanosecondsPerSecond - 1; + + private bool IsNormalized => + Nanos >= 0 && + Nanos <= MaxNanos && + Seconds >= UnixSecondsAtBclMinValue && + Seconds <= UnixSecondsAtBclMaxValue; /// <summary> /// Returns the difference between one <see cref="Timestamp"/> and another, as a <see cref="Duration"/>. @@ -99,8 +107,14 @@ namespace Google.Protobuf.WellKnownTypes /// <see cref="DateTime"/> value precisely on a second. /// </remarks> /// <returns>This timestamp as a <c>DateTime</c>.</returns> + /// <exception cref="InvalidOperationException">The timestamp contains invalid values; either it is + /// incorrectly normalized or is outside the valid range.</exception> public DateTime ToDateTime() { + if (!IsNormalized) + { + throw new InvalidOperationException(@"Timestamp contains invalid values: Seconds={Seconds}; Nanos={Nanos}"); + } return UnixEpoch.AddSeconds(Seconds).AddTicks(Nanos / Duration.NanosecondsPerTick); } @@ -114,6 +128,8 @@ namespace Google.Protobuf.WellKnownTypes /// <see cref="DateTimeOffset"/> value precisely on a second. /// </remarks> /// <returns>This timestamp as a <c>DateTimeOffset</c>.</returns> + /// <exception cref="InvalidOperationException">The timestamp contains invalid values; either it is + /// incorrectly normalized or is outside the valid range.</exception> public DateTimeOffset ToDateTimeOffset() { return new DateTimeOffset(ToDateTime(), TimeSpan.Zero); diff --git a/src/google/protobuf/unittest_well_known_types.proto b/src/google/protobuf/unittest_well_known_types.proto index 2cb7775c..c9075244 100644 --- a/src/google/protobuf/unittest_well_known_types.proto +++ b/src/google/protobuf/unittest_well_known_types.proto @@ -39,6 +39,8 @@ message TestWellKnownTypes { google.protobuf.BoolValue bool_field = 16; google.protobuf.StringValue string_field = 17; google.protobuf.BytesValue bytes_field = 18; + // Part of struct, but useful to be able to test separately + google.protobuf.Value value_field = 19; } // A repeated field for each well-known type. |