From e7f88ff1294ada0fca19334ed2c844cdb98ea2f6 Mon Sep 17 00:00:00 2001
From: Jon Skeet <jonskeet@google.com>
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.)
---
 csharp/src/Google.Protobuf/CodedInputStream.cs | 55 ++++++++++++++++++++------
 1 file changed, 42 insertions(+), 13 deletions(-)

(limited to 'csharp/src/Google.Protobuf/CodedInputStream.cs')

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
         /// <summary>
-        /// 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.
         /// </summary>
-        /// <exception cref="InvalidProtocolBufferException">The last
+        /// <exception cref="InvalidProtocolBufferException">The 
         /// tag read was not the one specified</exception>
-        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
         /// <summary>
         /// Reads a field tag, returning the tag of 0 for "end of stream".
         /// </summary>
+        /// <remarks>
+        /// 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.
+        /// </remarks>
         /// <returns>The next field tag, or 0 for end of stream. (0 is never a valid tag.)</returns>
         public uint ReadTag()
         {
@@ -329,22 +333,24 @@ namespace Google.Protobuf
         }
 
         /// <summary>
-        /// 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 <see cref="ReadTag"/>, when
         /// the caller wishes to skip an unknown field.
         /// </summary>
-        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--;
+        }
+
         /// <summary>
         /// Reads a double field from the stream.
         /// </summary>
@@ -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)
             {
-- 
cgit v1.2.3