From e7f88ff1294ada0fca19334ed2c844cdb98ea2f6 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Thu, 6 Aug 2015 11:40:32 +0100 Subject: Skip groups properly. Now the generated code doesn't need to check for end group tags, as it will skip whole groups at a time. Currently it will ignore extraneous end group tags, which may or may not be a good thing. Renamed ConsumeLastField to SkipLastField as it felt more natural. Removed WireFormat.IsEndGroupTag as it's no longer useful. This mostly fixes issue 688. (Generated code changes coming in next commit.) --- .../Google.Protobuf.Test/CodedInputStreamTest.cs | 87 ++++++++++++++++++++++ csharp/src/Google.Protobuf/CodedInputStream.cs | 55 ++++++++++---- csharp/src/Google.Protobuf/Collections/MapField.cs | 5 +- csharp/src/Google.Protobuf/FieldCodec.cs | 7 +- csharp/src/Google.Protobuf/MessageExtensions.cs | 6 +- csharp/src/Google.Protobuf/WireFormat.cs | 10 --- 6 files changed, 138 insertions(+), 32 deletions(-) (limited to 'csharp/src') diff --git a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs index c4c92efd..42c740ac 100644 --- a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs +++ b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs @@ -442,5 +442,92 @@ namespace Google.Protobuf var input = new CodedInputStream(new byte[] { 0 }); Assert.Throws(() => input.ReadTag()); } + + [Test] + public void SkipGroup() + { + // Create an output stream with a group in: + // Field 1: string "field 1" + // Field 2: group containing: + // Field 1: fixed int32 value 100 + // Field 2: string "ignore me" + // Field 3: nested group containing + // Field 1: fixed int64 value 1000 + // Field 3: string "field 3" + var stream = new MemoryStream(); + var output = new CodedOutputStream(stream); + output.WriteTag(1, WireFormat.WireType.LengthDelimited); + output.WriteString("field 1"); + + // The outer group... + output.WriteTag(2, WireFormat.WireType.StartGroup); + output.WriteTag(1, WireFormat.WireType.Fixed32); + output.WriteFixed32(100); + output.WriteTag(2, WireFormat.WireType.LengthDelimited); + output.WriteString("ignore me"); + // The nested group... + output.WriteTag(3, WireFormat.WireType.StartGroup); + output.WriteTag(1, WireFormat.WireType.Fixed64); + output.WriteFixed64(1000); + // Note: Not sure the field number is relevant for end group... + output.WriteTag(3, WireFormat.WireType.EndGroup); + + // End the outer group + output.WriteTag(2, WireFormat.WireType.EndGroup); + + output.WriteTag(3, WireFormat.WireType.LengthDelimited); + output.WriteString("field 3"); + output.Flush(); + stream.Position = 0; + + // Now act like a generated client + var input = new CodedInputStream(stream); + Assert.AreEqual(WireFormat.MakeTag(1, WireFormat.WireType.LengthDelimited), input.ReadTag()); + Assert.AreEqual("field 1", input.ReadString()); + Assert.AreEqual(WireFormat.MakeTag(2, WireFormat.WireType.StartGroup), input.ReadTag()); + input.SkipLastField(); // Should consume the whole group, including the nested one. + Assert.AreEqual(WireFormat.MakeTag(3, WireFormat.WireType.LengthDelimited), input.ReadTag()); + Assert.AreEqual("field 3", input.ReadString()); + } + + [Test] + public void EndOfStreamReachedWhileSkippingGroup() + { + var stream = new MemoryStream(); + var output = new CodedOutputStream(stream); + output.WriteTag(1, WireFormat.WireType.StartGroup); + output.WriteTag(2, WireFormat.WireType.StartGroup); + output.WriteTag(2, WireFormat.WireType.EndGroup); + + output.Flush(); + stream.Position = 0; + + // Now act like a generated client + var input = new CodedInputStream(stream); + input.ReadTag(); + Assert.Throws(() => input.SkipLastField()); + } + + [Test] + public void RecursionLimitAppliedWhileSkippingGroup() + { + var stream = new MemoryStream(); + var output = new CodedOutputStream(stream); + for (int i = 0; i < CodedInputStream.DefaultRecursionLimit + 1; i++) + { + output.WriteTag(1, WireFormat.WireType.StartGroup); + } + for (int i = 0; i < CodedInputStream.DefaultRecursionLimit + 1; i++) + { + output.WriteTag(1, WireFormat.WireType.EndGroup); + } + output.Flush(); + stream.Position = 0; + + // Now act like a generated client + var input = new CodedInputStream(stream); + Assert.AreEqual(WireFormat.MakeTag(1, WireFormat.WireType.StartGroup), input.ReadTag()); + Assert.Throws(() => input.SkipLastField()); + } } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/CodedInputStream.cs b/csharp/src/Google.Protobuf/CodedInputStream.cs index 0e2495f1..a37fefc1 100644 --- a/csharp/src/Google.Protobuf/CodedInputStream.cs +++ b/csharp/src/Google.Protobuf/CodedInputStream.cs @@ -236,17 +236,16 @@ namespace Google.Protobuf #region Validation /// - /// Verifies that the last call to ReadTag() returned the given tag value. - /// This is used to verify that a nested group ended with the correct - /// end tag. + /// Verifies that the last call to ReadTag() returned tag 0 - in other words, + /// we've reached the end of the stream when we expected to. /// - /// The last + /// The /// tag read was not the one specified - internal void CheckLastTagWas(uint value) + internal void CheckReadEndOfStreamTag() { - if (lastTag != value) + if (lastTag != 0) { - throw InvalidProtocolBufferException.InvalidEndTag(); + throw InvalidProtocolBufferException.MoreDataAvailable(); } } #endregion @@ -275,6 +274,11 @@ namespace Google.Protobuf /// /// Reads a field tag, returning the tag of 0 for "end of stream". /// + /// + /// If this method returns 0, it doesn't necessarily mean the end of all + /// the data in this CodedInputStream; it may be the end of the logical stream + /// for an embedded message, for example. + /// /// The next field tag, or 0 for end of stream. (0 is never a valid tag.) public uint ReadTag() { @@ -329,22 +333,24 @@ namespace Google.Protobuf } /// - /// Consumes the data for the field with the tag we've just read. + /// Skips the data for the field with the tag we've just read. /// This should be called directly after , when /// the caller wishes to skip an unknown field. /// - public void ConsumeLastField() + public void SkipLastField() { if (lastTag == 0) { - throw new InvalidOperationException("ConsumeLastField cannot be called at the end of a stream"); + throw new InvalidOperationException("SkipLastField cannot be called at the end of a stream"); } switch (WireFormat.GetTagWireType(lastTag)) { case WireFormat.WireType.StartGroup: + ConsumeGroup(); + break; case WireFormat.WireType.EndGroup: - // TODO: Work out how to skip them instead? See issue 688. - throw new InvalidProtocolBufferException("Group tags not supported by proto3 C# implementation"); + // Just ignore; there's no data following the tag. + break; case WireFormat.WireType.Fixed32: ReadFixed32(); break; @@ -361,6 +367,29 @@ namespace Google.Protobuf } } + private void ConsumeGroup() + { + // Note: Currently we expect this to be the way that groups are read. We could put the recursion + // depth changes into the ReadTag method instead, potentially... + recursionDepth++; + if (recursionDepth >= recursionLimit) + { + throw InvalidProtocolBufferException.RecursionLimitExceeded(); + } + uint tag; + do + { + tag = ReadTag(); + if (tag == 0) + { + throw InvalidProtocolBufferException.TruncatedMessage(); + } + // This recursion will allow us to handle nested groups. + SkipLastField(); + } while (WireFormat.GetTagWireType(tag) != WireFormat.WireType.EndGroup); + recursionDepth--; + } + /// /// Reads a double field from the stream. /// @@ -475,7 +504,7 @@ namespace Google.Protobuf int oldLimit = PushLimit(length); ++recursionDepth; builder.MergeFrom(this); - CheckLastTagWas(0); + CheckReadEndOfStreamTag(); // Check that we've read exactly as much data as expected. if (!ReachedLimit) { diff --git a/csharp/src/Google.Protobuf/Collections/MapField.cs b/csharp/src/Google.Protobuf/Collections/MapField.cs index 5eb2c2fc..dc4b04cb 100644 --- a/csharp/src/Google.Protobuf/Collections/MapField.cs +++ b/csharp/src/Google.Protobuf/Collections/MapField.cs @@ -637,10 +637,9 @@ namespace Google.Protobuf.Collections { Value = codec.valueCodec.Read(input); } - else if (WireFormat.IsEndGroupTag(tag)) + else { - // TODO(jonskeet): Do we need this? (Given that we don't support groups...) - return; + input.SkipLastField(); } } } diff --git a/csharp/src/Google.Protobuf/FieldCodec.cs b/csharp/src/Google.Protobuf/FieldCodec.cs index 15d52c7d..20a1f438 100644 --- a/csharp/src/Google.Protobuf/FieldCodec.cs +++ b/csharp/src/Google.Protobuf/FieldCodec.cs @@ -304,12 +304,13 @@ namespace Google.Protobuf { value = codec.Read(input); } - if (WireFormat.IsEndGroupTag(tag)) + else { - break; + input.SkipLastField(); } + } - input.CheckLastTagWas(0); + input.CheckReadEndOfStreamTag(); input.PopLimit(oldLimit); return value; diff --git a/csharp/src/Google.Protobuf/MessageExtensions.cs b/csharp/src/Google.Protobuf/MessageExtensions.cs index ee78dc8d..d2d057c0 100644 --- a/csharp/src/Google.Protobuf/MessageExtensions.cs +++ b/csharp/src/Google.Protobuf/MessageExtensions.cs @@ -50,7 +50,7 @@ namespace Google.Protobuf Preconditions.CheckNotNull(data, "data"); CodedInputStream input = new CodedInputStream(data); message.MergeFrom(input); - input.CheckLastTagWas(0); + input.CheckReadEndOfStreamTag(); } /// @@ -64,7 +64,7 @@ namespace Google.Protobuf Preconditions.CheckNotNull(data, "data"); CodedInputStream input = data.CreateCodedInput(); message.MergeFrom(input); - input.CheckLastTagWas(0); + input.CheckReadEndOfStreamTag(); } /// @@ -78,7 +78,7 @@ namespace Google.Protobuf Preconditions.CheckNotNull(input, "input"); CodedInputStream codedInput = new CodedInputStream(input); message.MergeFrom(codedInput); - codedInput.CheckLastTagWas(0); + codedInput.CheckReadEndOfStreamTag(); } /// diff --git a/csharp/src/Google.Protobuf/WireFormat.cs b/csharp/src/Google.Protobuf/WireFormat.cs index bbd7e4f9..b0e4a41f 100644 --- a/csharp/src/Google.Protobuf/WireFormat.cs +++ b/csharp/src/Google.Protobuf/WireFormat.cs @@ -98,16 +98,6 @@ namespace Google.Protobuf return (WireType) (tag & TagTypeMask); } - /// - /// Determines whether the given tag is an end group tag. - /// - /// The tag to check. - /// true if the given tag is an end group tag; false otherwise. - public static bool IsEndGroupTag(uint tag) - { - return (WireType) (tag & TagTypeMask) == WireType.EndGroup; - } - /// /// Given a tag value, determines the field number (the upper 29 bits). /// -- cgit v1.2.3 From 5bdc57292f2a21706ff1dab21652fb8e1f058eb7 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Thu, 6 Aug 2015 11:40:43 +0100 Subject: Generated code for previous commit. --- csharp/src/AddressBook/Addressbook.cs | 15 +- .../src/Google.Protobuf.Conformance/Conformance.cs | 25 +-- .../TestProtos/MapUnittestProto3.cs | 35 +---- .../TestProtos/UnittestImportProto3.cs | 5 +- .../TestProtos/UnittestImportPublicProto3.cs | 5 +- .../TestProtos/UnittestIssues.cs | 50 ++---- .../TestProtos/UnittestProto3.cs | 175 +++++---------------- .../TestProtos/UnittestWellKnownTypes.cs | 20 +-- .../WellKnownTypes/WrappersTest.cs | 23 +++ .../InvalidProtocolBufferException.cs | 6 + .../Reflection/DescriptorProtoFile.cs | 110 +++---------- csharp/src/Google.Protobuf/WellKnownTypes/Any.cs | 5 +- csharp/src/Google.Protobuf/WellKnownTypes/Api.cs | 10 +- .../src/Google.Protobuf/WellKnownTypes/Duration.cs | 5 +- csharp/src/Google.Protobuf/WellKnownTypes/Empty.cs | 5 +- .../Google.Protobuf/WellKnownTypes/FieldMask.cs | 5 +- .../WellKnownTypes/SourceContext.cs | 5 +- .../src/Google.Protobuf/WellKnownTypes/Struct.cs | 15 +- .../Google.Protobuf/WellKnownTypes/Timestamp.cs | 5 +- csharp/src/Google.Protobuf/WellKnownTypes/Type.cs | 25 +-- .../src/Google.Protobuf/WellKnownTypes/Wrappers.cs | 45 ++---- 21 files changed, 142 insertions(+), 452 deletions(-) (limited to 'csharp/src') diff --git a/csharp/src/AddressBook/Addressbook.cs b/csharp/src/AddressBook/Addressbook.cs index 85fa2977..f2be5bae 100644 --- a/csharp/src/AddressBook/Addressbook.cs +++ b/csharp/src/AddressBook/Addressbook.cs @@ -189,10 +189,7 @@ namespace Google.Protobuf.Examples.AddressBook { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -335,10 +332,7 @@ namespace Google.Protobuf.Examples.AddressBook { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Number = input.ReadString(); @@ -441,10 +435,7 @@ namespace Google.Protobuf.Examples.AddressBook { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { people_.AddEntriesFrom(input, _repeated_people_codec); diff --git a/csharp/src/Google.Protobuf.Conformance/Conformance.cs b/csharp/src/Google.Protobuf.Conformance/Conformance.cs index 50a6756c..e905d4e4 100644 --- a/csharp/src/Google.Protobuf.Conformance/Conformance.cs +++ b/csharp/src/Google.Protobuf.Conformance/Conformance.cs @@ -321,10 +321,7 @@ namespace Conformance { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { ProtobufPayload = input.ReadBytes(); @@ -557,10 +554,7 @@ namespace Conformance { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { ParseError = input.ReadString(); @@ -1829,10 +1823,7 @@ namespace Conformance { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { OptionalInt32 = input.ReadInt32(); @@ -2256,10 +2247,7 @@ namespace Conformance { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { A = input.ReadInt32(); @@ -2373,10 +2361,7 @@ namespace Conformance { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { C = input.ReadInt32(); diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/MapUnittestProto3.cs b/csharp/src/Google.Protobuf.Test/TestProtos/MapUnittestProto3.cs index f6835c4b..e9e18193 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/MapUnittestProto3.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/MapUnittestProto3.cs @@ -476,10 +476,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { mapInt32Int32_.AddEntriesFrom(input, _map_mapInt32Int32_codec); @@ -648,10 +645,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { if (testMap_ == null) { @@ -748,10 +742,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { mapInt32Message_.AddEntriesFrom(input, _map_mapInt32Message_codec); @@ -859,10 +850,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { map1_.AddEntriesFrom(input, _map_map1_codec); @@ -1156,10 +1144,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { mapInt32Int32_.AddEntriesFrom(input, _map_mapInt32Int32_codec); @@ -1309,10 +1294,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { type_.AddEntriesFrom(input, _map_type_codec); @@ -1416,10 +1398,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { entry_.AddEntriesFrom(input, _map_entry_codec); diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestImportProto3.cs b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestImportProto3.cs index 646a01a2..bf527ac5 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestImportProto3.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestImportProto3.cs @@ -139,10 +139,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { D = input.ReadInt32(); diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestImportPublicProto3.cs b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestImportPublicProto3.cs index 225775a3..ec460906 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestImportPublicProto3.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestImportPublicProto3.cs @@ -125,10 +125,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { E = input.ReadInt32(); diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs index 1bf40ead..63119a34 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs @@ -142,10 +142,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -222,10 +219,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -302,10 +296,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -441,10 +432,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { value_ = (global::UnitTest.Issues.TestProtos.NegativeEnum) input.ReadEnum(); @@ -534,10 +522,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -730,10 +715,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { PrimitiveValue = input.ReadInt32(); @@ -860,10 +842,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Item = input.ReadInt32(); @@ -987,10 +966,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Types_ = input.ReadInt32(); @@ -1075,10 +1051,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -1343,10 +1316,7 @@ namespace UnitTest.Issues.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { PlainString = input.ReadString(); diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestProto3.cs b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestProto3.cs index 58e5be65..bf4590ad 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestProto3.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestProto3.cs @@ -1211,10 +1211,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { SingleInt32 = input.ReadInt32(); @@ -1546,10 +1543,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Bb = input.ReadInt32(); @@ -1698,10 +1692,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { if (child_ == null) { @@ -1818,10 +1809,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { DeprecatedInt32 = input.ReadInt32(); @@ -1923,10 +1911,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { C = input.ReadInt32(); @@ -2006,10 +1991,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -2110,10 +2092,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { if (foreignNested_ == null) { @@ -2240,10 +2219,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { A = input.ReadInt32(); @@ -2374,10 +2350,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { if (a_ == null) { @@ -2489,10 +2462,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { if (bb_ == null) { @@ -2622,10 +2592,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { if (a_ == null) { @@ -2859,10 +2826,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { PrimitiveField = input.ReadInt32(); @@ -3066,10 +3030,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { MyInt = input.ReadInt64(); @@ -3209,10 +3170,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Bb = input.ReadInt32(); @@ -3323,10 +3281,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { sparseEnum_ = (global::Google.Protobuf.TestProtos.TestSparseEnum) input.ReadEnum(); @@ -3428,10 +3383,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Data = input.ReadString(); @@ -3525,10 +3477,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { data_.AddEntriesFrom(input, _repeated_data_codec); @@ -3630,10 +3579,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Data = input.ReadBytes(); @@ -3735,10 +3681,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Data = input.ReadBytes(); @@ -3840,10 +3783,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Data = input.ReadInt32(); @@ -3945,10 +3885,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Data = input.ReadUInt32(); @@ -4050,10 +3987,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Data = input.ReadInt64(); @@ -4155,10 +4089,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Data = input.ReadUInt64(); @@ -4260,10 +4191,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Data = input.ReadBool(); @@ -4438,10 +4366,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { FooInt = input.ReadInt32(); @@ -4730,10 +4655,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 722: case 720: { @@ -5075,10 +4997,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 722: case 720: { @@ -5308,10 +5227,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 98: case 101: { @@ -5439,10 +5355,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { A = input.ReadString(); @@ -5522,10 +5435,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -5601,10 +5511,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -5680,10 +5587,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -5759,10 +5663,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -5838,10 +5739,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } @@ -5917,10 +5815,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs index 0840fa27..16634e03 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs @@ -679,10 +679,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { if (anyField_ == null) { @@ -1136,10 +1133,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { anyField_.AddEntriesFrom(input, _repeated_anyField_codec); @@ -1757,10 +1751,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { global::Google.Protobuf.WellKnownTypes.Any subBuilder = new global::Google.Protobuf.WellKnownTypes.Any(); @@ -2205,10 +2196,7 @@ namespace Google.Protobuf.TestProtos { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { anyField_.AddEntriesFrom(input, _map_anyField_codec); diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs index d2a07057..fbc0ff07 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs @@ -322,5 +322,28 @@ namespace Google.Protobuf.WellKnownTypes // A normal implementation would have 0 now, as the explicit default would have been overwritten the 5. Assert.AreEqual(5, message.Int32Field); } + + [Test] + public void UnknownFieldInWrapper() + { + var stream = new MemoryStream(); + var output = new CodedOutputStream(stream); + var wrapperTag = WireFormat.MakeTag(TestWellKnownTypes.Int32FieldFieldNumber, WireFormat.WireType.LengthDelimited); + var unknownTag = WireFormat.MakeTag(15, WireFormat.WireType.Varint); + var valueTag = WireFormat.MakeTag(Int32Value.ValueFieldNumber, WireFormat.WireType.Varint); + + output.WriteTag(wrapperTag); + output.WriteLength(4); // unknownTag + value 5 + valueType + value 6, each 1 byte + output.WriteTag(unknownTag); + output.WriteInt32((int) valueTag); // Sneakily "pretend" it's a tag when it's really a value + output.WriteTag(valueTag); + output.WriteInt32(6); + + output.Flush(); + stream.Position = 0; + + var message = TestWellKnownTypes.Parser.ParseFrom(stream); + Assert.AreEqual(6, message.Int32Field); + } } } diff --git a/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs b/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs index 6905a6a3..01d55395 100644 --- a/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs +++ b/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs @@ -45,6 +45,12 @@ namespace Google.Protobuf { } + internal static InvalidProtocolBufferException MoreDataAvailable() + { + return new InvalidProtocolBufferException( + "Completed reading a message while more data was available in the stream."); + } + internal static InvalidProtocolBufferException TruncatedMessage() { return new InvalidProtocolBufferException( diff --git a/csharp/src/Google.Protobuf/Reflection/DescriptorProtoFile.cs b/csharp/src/Google.Protobuf/Reflection/DescriptorProtoFile.cs index d66bdb80..59c5e69b 100644 --- a/csharp/src/Google.Protobuf/Reflection/DescriptorProtoFile.cs +++ b/csharp/src/Google.Protobuf/Reflection/DescriptorProtoFile.cs @@ -242,10 +242,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { file_.AddEntriesFrom(input, _repeated_file_codec); @@ -539,10 +536,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -833,10 +827,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -1000,10 +991,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Start = input.ReadInt32(); @@ -1131,10 +1119,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Start = input.ReadInt32(); @@ -1424,10 +1409,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -1597,10 +1579,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -1741,10 +1720,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -1904,10 +1880,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -2059,10 +2032,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -2288,10 +2258,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -2716,10 +2683,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { JavaPackage = input.ReadString(); @@ -2969,10 +2933,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { MessageSetWireFormat = input.ReadBool(); @@ -3214,10 +3175,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { ctype_ = (global::Google.Protobuf.Reflection.FieldOptions.Types.CType) input.ReadEnum(); @@ -3397,10 +3355,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 16: { AllowAlias = input.ReadBool(); @@ -3524,10 +3479,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Deprecated = input.ReadBool(); @@ -3647,10 +3599,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 264: { Deprecated = input.ReadBool(); @@ -3770,10 +3719,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 264: { Deprecated = input.ReadBool(); @@ -4003,10 +3949,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 18: { name_.AddEntriesFrom(input, _repeated_name_codec); @@ -4155,10 +4098,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { NamePart_ = input.ReadString(); @@ -4261,10 +4201,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { location_.AddEntriesFrom(input, _repeated_location_codec); @@ -4431,10 +4368,7 @@ namespace Google.Protobuf.Reflection { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: case 8: { diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Any.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Any.cs index 9fc653b0..204b37cf 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Any.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Any.cs @@ -150,10 +150,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { TypeUrl = input.ReadString(); diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Api.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Api.cs index 8a94e7b0..a5f95093 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Api.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Api.cs @@ -213,10 +213,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -439,10 +436,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Duration.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Duration.cs index d74636d4..aa34f2d8 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Duration.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Duration.cs @@ -151,10 +151,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Seconds = input.ReadInt64(); diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Empty.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Empty.cs index 0f1d7f50..7d699c1d 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Empty.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Empty.cs @@ -106,10 +106,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; } } diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/FieldMask.cs b/csharp/src/Google.Protobuf/WellKnownTypes/FieldMask.cs index 9bd47a96..0dac4403 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/FieldMask.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/FieldMask.cs @@ -120,10 +120,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { paths_.AddEntriesFrom(input, _repeated_paths_codec); diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/SourceContext.cs b/csharp/src/Google.Protobuf/WellKnownTypes/SourceContext.cs index ae79884f..8347999d 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/SourceContext.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/SourceContext.cs @@ -129,10 +129,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { FileName = input.ReadString(); diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs index ea8b1055..1e8a8236 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs @@ -139,10 +139,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { fields_.AddEntriesFrom(input, _map_fields_codec); @@ -392,10 +389,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { kind_ = input.ReadEnum(); @@ -520,10 +514,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { values_.AddEntriesFrom(input, _repeated_values_codec); diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Timestamp.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Timestamp.cs index 89355bdc..d7c0954f 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Timestamp.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Timestamp.cs @@ -151,10 +151,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Seconds = input.ReadInt64(); diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Type.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Type.cs index 36116a65..ff2ecc57 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Type.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Type.cs @@ -226,10 +226,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -496,10 +493,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { kind_ = (global::Google.Protobuf.WellKnownTypes.Field.Types.Kind) input.ReadEnum(); @@ -716,10 +710,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -872,10 +863,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); @@ -1010,10 +998,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Name = input.ReadString(); diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Wrappers.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Wrappers.cs index 19ed599d..9ecaf47c 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Wrappers.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Wrappers.cs @@ -138,10 +138,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 9: { Value = input.ReadDouble(); @@ -243,10 +240,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 13: { Value = input.ReadFloat(); @@ -348,10 +342,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Value = input.ReadInt64(); @@ -453,10 +444,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Value = input.ReadUInt64(); @@ -558,10 +546,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Value = input.ReadInt32(); @@ -663,10 +648,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Value = input.ReadUInt32(); @@ -768,10 +750,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 8: { Value = input.ReadBool(); @@ -873,10 +852,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Value = input.ReadString(); @@ -978,10 +954,7 @@ namespace Google.Protobuf.WellKnownTypes { while ((tag = input.ReadTag()) != 0) { switch(tag) { default: - if (pb::WireFormat.IsEndGroupTag(tag)) { - return; - } - input.ConsumeLastField(); + input.SkipLastField(); break; case 10: { Value = input.ReadBytes(); -- cgit v1.2.3 From 6e16037c9933e175f62feb445ff8bd22d7727285 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Sat, 8 Aug 2015 07:24:28 +0100 Subject: Address review comments. --- .../Google.Protobuf.Test/GeneratedMessageTest.cs | 22 ++++++++++++++++++++++ csharp/src/Google.Protobuf/CodedInputStream.cs | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) (limited to 'csharp/src') diff --git a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs index 6fdd1066..575d4586 100644 --- a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs +++ b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs @@ -629,5 +629,27 @@ namespace Google.Protobuf var data = new byte[] { 130, 3, 1 }; Assert.Throws(() => TestAllTypes.Parser.ParseFrom(data)); } + + /// + /// Demonstrates current behaviour with an extraneous end group tag - see issue 688 + /// for details; we may want to change this. + /// + [Test] + public void ExtraEndGroupSkipped() + { + var message = SampleMessages.CreateFullTestAllTypes(); + var stream = new MemoryStream(); + var output = new CodedOutputStream(stream); + + output.WriteTag(100, WireFormat.WireType.EndGroup); + output.WriteTag(TestAllTypes.SingleFixed32FieldNumber, WireFormat.WireType.Fixed32); + output.WriteFixed32(123); + + output.Flush(); + + stream.Position = 0; + var parsed = TestAllTypes.Parser.ParseFrom(stream); + Assert.AreEqual(new TestAllTypes { SingleFixed32 = 123 }, parsed); + } } } diff --git a/csharp/src/Google.Protobuf/CodedInputStream.cs b/csharp/src/Google.Protobuf/CodedInputStream.cs index a37fefc1..dcd19e48 100644 --- a/csharp/src/Google.Protobuf/CodedInputStream.cs +++ b/csharp/src/Google.Protobuf/CodedInputStream.cs @@ -346,7 +346,7 @@ namespace Google.Protobuf switch (WireFormat.GetTagWireType(lastTag)) { case WireFormat.WireType.StartGroup: - ConsumeGroup(); + SkipGroup(); break; case WireFormat.WireType.EndGroup: // Just ignore; there's no data following the tag. @@ -367,7 +367,7 @@ namespace Google.Protobuf } } - private void ConsumeGroup() + private void SkipGroup() { // Note: Currently we expect this to be the way that groups are read. We could put the recursion // depth changes into the ReadTag method instead, potentially... -- cgit v1.2.3