Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary branch from ReadTag #7289

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions csharp/src/Google.Protobuf.Test/IssuesTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
using Google.Protobuf.Reflection;
using UnitTest.Issues.TestProtos;
using NUnit.Framework;
using System.IO;
using static UnitTest.Issues.TestProtos.OneofMerging.Types;

namespace Google.Protobuf
Expand Down Expand Up @@ -90,5 +91,25 @@ public void OneofMerging()
merged.MergeFrom(message2);
Assert.AreEqual(expected, merged);
}

// See https://github.com/protocolbuffers/protobuf/pull/7289
jtattermusch marked this conversation as resolved.
Show resolved Hide resolved
[Test]
public void CodedInputStream_LimitReachedRightAfterTag()
{
MemoryStream ms = new MemoryStream();
var cos = new CodedOutputStream(ms);
cos.WriteTag(11, WireFormat.WireType.Varint);
Assert.AreEqual(1, cos.Position);
cos.WriteString("some extra padding"); // ensure is currentLimit distinct from the end of the buffer.
cos.Flush();

var cis = new CodedInputStream(ms.ToArray());
cis.PushLimit(1); // make sure we reach the limit right after reading the tag.

// we still must read the tag correctly, even though the tag is at the very end of our limited input
// (which is a corner case and will most likely result in an error when trying to read value of the field
// decribed by this tag, but it would be a logical error not to read the tag that's actually present).
cis.AssertNextTag(WireFormat.MakeTag(11, WireFormat.WireType.Varint));
}
}
}
22 changes: 9 additions & 13 deletions csharp/src/Google.Protobuf/CodedInputStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -308,16 +308,16 @@ internal void CheckReadEndOfStreamTag()
}
}

internal void CheckLastTagWas(uint expectedTag)
{
if (lastTag != expectedTag) {
throw InvalidProtocolBufferException.InvalidEndTag();
}
}
internal void CheckLastTagWas(uint expectedTag)
{
if (lastTag != expectedTag) {
throw InvalidProtocolBufferException.InvalidEndTag();
}
}
#endregion

#region Reading of tags etc

/// <summary>
/// Peeks at the next field tag. This is like calling <see cref="ReadTag"/>, but the
/// tag is not consumed. (So a subsequent call to <see cref="ReadTag"/> will return the
Expand Down Expand Up @@ -395,10 +395,6 @@ public uint ReadTag()
// If we actually read a tag with a field of 0, that's not a valid tag.
throw InvalidProtocolBufferException.InvalidTag();
}
if (ReachedLimit)
{
return 0;
}
return lastTag;
}

Expand Down Expand Up @@ -644,7 +640,7 @@ public void ReadGroup(IMessage builder)
}
++recursionDepth;

uint tag = lastTag;
uint tag = lastTag;
int fieldNumber = WireFormat.GetTagFieldNumber(tag);

builder.MergeFrom(this);
Expand Down