diff options
8 files changed, 53 insertions, 65 deletions
diff --git a/csharp/src/Google.Protobuf.Test/CodedInputStreamExtensions.cs b/csharp/src/Google.Protobuf.Test/CodedInputStreamExtensions.cs index 408c7cb9..23af2887 100644 --- a/csharp/src/Google.Protobuf.Test/CodedInputStreamExtensions.cs +++ b/csharp/src/Google.Protobuf.Test/CodedInputStreamExtensions.cs @@ -38,8 +38,7 @@ namespace Google.Protobuf { public static void AssertNextTag(this CodedInputStream input, uint expectedTag) { - uint tag; - Assert.IsTrue(input.ReadTag(out tag)); + uint tag = input.ReadTag(); Assert.AreEqual(expectedTag, tag); } diff --git a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs index 6e25fa37..c4c92efd 100644 --- a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs +++ b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs @@ -279,9 +279,7 @@ namespace Google.Protobuf ms.Position = 0;
CodedInputStream input = new CodedInputStream(ms);
- uint testtag;
- Assert.IsTrue(input.ReadTag(out testtag));
- Assert.AreEqual(tag, testtag);
+ Assert.AreEqual(tag, input.ReadTag());
// TODO(jonskeet): Should this be ArgumentNullException instead?
Assert.Throws<InvalidProtocolBufferException>(() => input.ReadBytes());
@@ -377,9 +375,7 @@ namespace Google.Protobuf CodedInputStream input = new CodedInputStream(ms);
- uint actualTag;
- Assert.IsTrue(input.ReadTag(out actualTag));
- Assert.AreEqual(tag, actualTag);
+ Assert.AreEqual(tag, input.ReadTag());
string text = input.ReadString();
Assert.AreEqual('\ufffd', text[0]);
}
@@ -430,15 +426,21 @@ namespace Google.Protobuf ms.Position = 0;
CodedInputStream input = new CodedInputStream(ms, new byte[ms.Length / 2]);
- uint tag;
- Assert.IsTrue(input.ReadTag(out tag));
+ uint tag = input.ReadTag();
Assert.AreEqual(1, WireFormat.GetTagFieldNumber(tag));
Assert.AreEqual(100, input.ReadBytes().Length);
- Assert.IsTrue(input.ReadTag(out tag));
+ tag = input.ReadTag();
Assert.AreEqual(2, WireFormat.GetTagFieldNumber(tag));
Assert.AreEqual(100, input.ReadBytes().Length);
}
}
+
+ [Test]
+ public void Tag0Throws()
+ {
+ var input = new CodedInputStream(new byte[] { 0 });
+ Assert.Throws<InvalidProtocolBufferException>(() => input.ReadTag());
+ }
}
}
\ No newline at end of file diff --git a/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs b/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs index af329174..c1bf7bd6 100644 --- a/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs +++ b/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs @@ -335,15 +335,16 @@ namespace Google.Protobuf // Now test Input stream:
{
CodedInputStream cin = new CodedInputStream(new MemoryStream(bytes), new byte[50]);
- uint tag;
Assert.AreEqual(0, cin.Position);
// Field 1:
- Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 1);
+ uint tag = cin.ReadTag();
+ Assert.AreEqual(1, tag >> 3);
Assert.AreEqual(1, cin.Position);
Assert.AreEqual(500, cin.ReadInt32());
Assert.AreEqual(3, cin.Position);
//Field 2:
- Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 2);
+ tag = cin.ReadTag();
+ Assert.AreEqual(2, tag >> 3);
Assert.AreEqual(4, cin.Position);
int childlen = cin.ReadLength();
Assert.AreEqual(120, childlen);
@@ -353,19 +354,22 @@ namespace Google.Protobuf // Now we are reading child message
{
// Field 11: numeric value: 500
- Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 11);
+ tag = cin.ReadTag();
+ Assert.AreEqual(11, tag >> 3);
Assert.AreEqual(6, cin.Position);
Assert.AreEqual(500, cin.ReadInt32());
Assert.AreEqual(8, cin.Position);
//Field 12: length delimited 120 bytes
- Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 12);
+ tag = cin.ReadTag();
+ Assert.AreEqual(12, tag >> 3);
Assert.AreEqual(9, cin.Position);
ByteString bstr = cin.ReadBytes();
Assert.AreEqual(110, bstr.Length);
Assert.AreEqual((byte) 109, bstr[109]);
Assert.AreEqual(120, cin.Position);
// Field 13: fixed numeric value: 501
- Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 13);
+ tag = cin.ReadTag();
+ Assert.AreEqual(13, tag >> 3);
// ROK - Previously broken here, this returned 126 failing to account for bufferSizeAfterLimit
Assert.AreEqual(121, cin.Position);
Assert.AreEqual(501, cin.ReadSFixed32());
@@ -375,7 +379,8 @@ namespace Google.Protobuf cin.PopLimit(oldlimit);
Assert.AreEqual(125, cin.Position);
// Field 3: fixed numeric value: 501
- Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 3);
+ tag = cin.ReadTag();
+ Assert.AreEqual(3, tag >> 3);
Assert.AreEqual(126, cin.Position);
Assert.AreEqual(501, cin.ReadSFixed32());
Assert.AreEqual(130, cin.Position);
diff --git a/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs b/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs index 322100d0..8c804fdd 100644 --- a/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs +++ b/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs @@ -455,7 +455,7 @@ namespace Google.Protobuf.Collections Assert.AreEqual(0, output.SpaceLeft); CodedInputStream input = new CodedInputStream(bytes); - Assert.IsTrue(input.ReadTag(out tag)); + tag = input.ReadTag(); RepeatedField<SampleEnum> values = new RepeatedField<SampleEnum>(); values.AddEntriesFrom(input, FieldCodec.ForEnum(tag, x => (int)x, x => (SampleEnum)x)); @@ -493,7 +493,7 @@ namespace Google.Protobuf.Collections Assert.AreEqual(0, output.SpaceLeft); CodedInputStream input = new CodedInputStream(bytes); - Assert.IsTrue(input.ReadTag(out tag)); + tag = input.ReadTag(); RepeatedField<SampleEnum> values = new RepeatedField<SampleEnum>(); values.AddEntriesFrom(input, FieldCodec.ForEnum(tag, x => (int)x, x => (SampleEnum)x)); diff --git a/csharp/src/Google.Protobuf/CodedInputStream.cs b/csharp/src/Google.Protobuf/CodedInputStream.cs index 5da03b5c..0e2495f1 100644 --- a/csharp/src/Google.Protobuf/CodedInputStream.cs +++ b/csharp/src/Google.Protobuf/CodedInputStream.cs @@ -254,37 +254,35 @@ namespace Google.Protobuf #region Reading of tags etc
/// <summary>
- /// Attempts to peek at the next field tag.
+ /// 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
+ /// same value.)
/// </summary>
- public bool PeekNextTag(out uint fieldTag)
+ public uint PeekTag()
{
if (hasNextTag)
{
- fieldTag = nextTag;
- return true;
+ return nextTag;
}
uint savedLast = lastTag;
- hasNextTag = ReadTag(out nextTag);
- lastTag = savedLast;
- fieldTag = nextTag;
- return hasNextTag;
+ nextTag = ReadTag();
+ hasNextTag = true;
+ lastTag = savedLast; // Undo the side effect of ReadTag
+ return nextTag;
}
/// <summary>
- /// Attempts to read a field tag, returning false if we have reached the end
- /// of the input data.
+ /// Reads a field tag, returning the tag of 0 for "end of stream".
/// </summary>
- /// <param name="fieldTag">The 'tag' of the field (id * 8 + wire-format)</param>
- /// <returns>true if the next fieldTag was read</returns>
- public bool ReadTag(out uint fieldTag)
+ /// <returns>The next field tag, or 0 for end of stream. (0 is never a valid tag.)</returns>
+ public uint ReadTag()
{
if (hasNextTag)
{
- fieldTag = nextTag;
- lastTag = fieldTag;
+ lastTag = nextTag;
hasNextTag = false;
- return true;
+ return lastTag;
}
// Optimize for the incredibly common case of having at least two bytes left in the buffer,
@@ -294,7 +292,7 @@ namespace Google.Protobuf int tmp = buffer[bufferPos++];
if (tmp < 128)
{
- fieldTag = (uint)tmp;
+ lastTag = (uint)tmp;
}
else
{
@@ -302,13 +300,13 @@ namespace Google.Protobuf if ((tmp = buffer[bufferPos++]) < 128)
{
result |= tmp << 7;
- fieldTag = (uint) result;
+ lastTag = (uint) result;
}
else
{
// Nope, rewind and go the potentially slow route.
bufferPos -= 2;
- fieldTag = ReadRawVarint32();
+ lastTag = ReadRawVarint32();
}
}
}
@@ -316,20 +314,18 @@ namespace Google.Protobuf {
if (IsAtEnd)
{
- fieldTag = 0;
- lastTag = fieldTag;
- return false;
+ lastTag = 0;
+ return 0; // This is the only case in which we return 0.
}
- fieldTag = ReadRawVarint32();
+ lastTag = ReadRawVarint32();
}
- lastTag = fieldTag;
if (lastTag == 0)
{
// If we actually read zero, that's not a valid tag.
throw InvalidProtocolBufferException.InvalidTag();
}
- return true;
+ return lastTag;
}
/// <summary>
@@ -580,14 +576,10 @@ namespace Google.Protobuf /// </summary>
public bool MaybeConsumeTag(uint tag)
{
- uint next;
- if (PeekNextTag(out next))
+ if (PeekTag() == tag)
{
- if (next == tag)
- {
- hasNextTag = false;
- return true;
- }
+ hasNextTag = false;
+ return true;
}
return false;
}
diff --git a/csharp/src/Google.Protobuf/Collections/MapField.cs b/csharp/src/Google.Protobuf/Collections/MapField.cs index fed3d062..5eb2c2fc 100644 --- a/csharp/src/Google.Protobuf/Collections/MapField.cs +++ b/csharp/src/Google.Protobuf/Collections/MapField.cs @@ -627,12 +627,8 @@ namespace Google.Protobuf.Collections public void MergeFrom(CodedInputStream input) { uint tag; - while (input.ReadTag(out tag)) + while ((tag = input.ReadTag()) != 0) { - if (tag == 0) - { - throw InvalidProtocolBufferException.InvalidTag(); - } if (tag == codec.keyCodec.Tag) { Key = codec.keyCodec.Read(input); diff --git a/csharp/src/Google.Protobuf/FieldCodec.cs b/csharp/src/Google.Protobuf/FieldCodec.cs index 1076c2a9..15d52c7d 100644 --- a/csharp/src/Google.Protobuf/FieldCodec.cs +++ b/csharp/src/Google.Protobuf/FieldCodec.cs @@ -298,12 +298,8 @@ namespace Google.Protobuf uint tag; T value = codec.DefaultValue; - while (input.ReadTag(out tag)) + while ((tag = input.ReadTag()) != 0) { - if (tag == 0) - { - throw InvalidProtocolBufferException.InvalidTag(); - } if (tag == codec.Tag) { value = codec.Read(input); diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc index ea722455..40c13de5 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message.cc @@ -417,13 +417,11 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) { printer->Indent(); printer->Print( "uint tag;\n" - "while (input.ReadTag(out tag)) {\n" + "while ((tag = input.ReadTag()) != 0) {\n" " switch(tag) {\n"); printer->Indent(); printer->Indent(); printer->Print( - "case 0:\n" // 0 signals EOF / limit reached - " throw pb::InvalidProtocolBufferException.InvalidTag();\n" "default:\n" " if (pb::WireFormat.IsEndGroupTag(tag)) {\n" " return;\n" |