From 6e16037c9933e175f62feb445ff8bd22d7727285 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Sat, 8 Aug 2015 07:24:28 +0100 Subject: [PATCH] Address review comments. --- .../GeneratedMessageTest.cs | 22 +++++++++++++++++++ .../src/Google.Protobuf/CodedInputStream.cs | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs index 6fdd10664d..575d4586d9 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 a37fefc105..dcd19e48ee 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...