New tests, double build errors, and a few miscellaneous fixes
diff --git a/src/ProtocolBuffers.Test/CodedInputStreamTest.cs b/src/ProtocolBuffers.Test/CodedInputStreamTest.cs
index ce4a668..2ad8c45 100644
--- a/src/ProtocolBuffers.Test/CodedInputStreamTest.cs
+++ b/src/ProtocolBuffers.Test/CodedInputStreamTest.cs
@@ -61,6 +61,7 @@
input = CodedInputStream.CreateInstance(data);
Assert.AreEqual(value, input.ReadRawVarint64());
+ Assert.IsTrue(input.IsAtEnd);
// Try different block sizes.
for (int bufferSize = 1; bufferSize <= 16; bufferSize *= 2) {
@@ -69,7 +70,18 @@
input = CodedInputStream.CreateInstance(new SmallBlockInputStream(data, bufferSize));
Assert.AreEqual(value, input.ReadRawVarint64());
+ Assert.IsTrue(input.IsAtEnd);
}
+
+ // Try reading directly from a MemoryStream. We want to verify that it
+ // doesn't read past the end of the input, so write an extra byte - this
+ // lets us test the position at the end.
+ MemoryStream memoryStream = new MemoryStream();
+ memoryStream.Write(data, 0, data.Length);
+ memoryStream.WriteByte(0);
+ memoryStream.Position = 0;
+ Assert.AreEqual((uint)value, CodedInputStream.ReadRawVarint32(memoryStream));
+ Assert.AreEqual(data.Length, memoryStream.Position);
}
/// <summary>
@@ -77,7 +89,7 @@
/// expects them to fail with an InvalidProtocolBufferException whose
/// description matches the given one.
/// </summary>
- private void AssertReadVarintFailure(InvalidProtocolBufferException expected, byte[] data) {
+ private static void AssertReadVarintFailure(InvalidProtocolBufferException expected, byte[] data) {
CodedInputStream input = CodedInputStream.CreateInstance(data);
try {
input.ReadRawVarint32();
@@ -93,6 +105,14 @@
} catch (InvalidProtocolBufferException e) {
Assert.AreEqual(expected.Message, e.Message);
}
+
+ // Make sure we get the same error when reading directly from a Stream.
+ try {
+ CodedInputStream.ReadRawVarint32(new MemoryStream(data));
+ Assert.Fail("Should have thrown an exception.");
+ } catch (InvalidProtocolBufferException e) {
+ Assert.AreEqual(expected.Message, e.Message);
+ }
}
[Test]
@@ -139,12 +159,14 @@
private static void AssertReadLittleEndian32(byte[] data, uint value) {
CodedInputStream input = CodedInputStream.CreateInstance(data);
Assert.AreEqual(value, input.ReadRawLittleEndian32());
+ Assert.IsTrue(input.IsAtEnd);
// Try different block sizes.
for (int blockSize = 1; blockSize <= 16; blockSize *= 2) {
input = CodedInputStream.CreateInstance(
new SmallBlockInputStream(data, blockSize));
Assert.AreEqual(value, input.ReadRawLittleEndian32());
+ Assert.IsTrue(input.IsAtEnd);
}
}
@@ -155,12 +177,14 @@
private static void AssertReadLittleEndian64(byte[] data, ulong value) {
CodedInputStream input = CodedInputStream.CreateInstance(data);
Assert.AreEqual(value, input.ReadRawLittleEndian64());
+ Assert.IsTrue(input.IsAtEnd);
// Try different block sizes.
for (int blockSize = 1; blockSize <= 16; blockSize *= 2) {
input = CodedInputStream.CreateInstance(
new SmallBlockInputStream(data, blockSize));
Assert.AreEqual(value, input.ReadRawLittleEndian64());
+ Assert.IsTrue(input.IsAtEnd);
}
}
@@ -239,6 +263,21 @@
}
}
+ /// <summary>
+ /// Test that a bug in SkipRawBytes has been fixed: if the skip
+ /// skips exactly up to a limit, this should bnot break things
+ /// </summary>
+ [Test]
+ public void SkipRawBytesBug() {
+ byte[] rawBytes = new byte[] { 1, 2 };
+ CodedInputStream input = CodedInputStream.CreateInstance(rawBytes);
+
+ int limit = input.PushLimit(1);
+ input.SkipRawBytes(1);
+ input.PopLimit(limit);
+ Assert.AreEqual(2, input.ReadRawByte());
+ }
+
public void ReadHugeBlob() {
// Allocate and initialize a 1MB blob.
byte[] blob = new byte[1 << 20];
@@ -348,6 +387,31 @@
}
}
+ [Test]
+ public void ResetSizeCounter() {
+ CodedInputStream input = CodedInputStream.CreateInstance(
+ new SmallBlockInputStream(new byte[256], 8));
+ input.SetSizeLimit(16);
+ input.ReadRawBytes(16);
+
+ try {
+ input.ReadRawByte();
+ Assert.Fail("Should have thrown an exception!");
+ } catch (InvalidProtocolBufferException e) {
+ // Success.
+ }
+
+ input.ResetSizeCounter();
+ input.ReadRawByte(); // No exception thrown.
+
+ try {
+ input.ReadRawBytes(16); // Hits limit again.
+ Assert.Fail("Should have thrown an exception!");
+ } catch (InvalidProtocolBufferException e) {
+ // Success.
+ }
+ }
+
/// <summary>
/// Tests that if we read an string that contains invalid UTF-8, no exception
/// is thrown. Instead, the invalid bytes are replaced with the Unicode
diff --git a/src/ProtocolBuffers.Test/DynamicMessageTest.cs b/src/ProtocolBuffers.Test/DynamicMessageTest.cs
index edcce38..2f3fefa 100644
--- a/src/ProtocolBuffers.Test/DynamicMessageTest.cs
+++ b/src/ProtocolBuffers.Test/DynamicMessageTest.cs
@@ -32,6 +32,7 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
using NUnit.Framework;
+using System;
namespace Google.ProtocolBuffers {
[TestFixture]
@@ -57,6 +58,18 @@
}
[Test]
+ public void DoubleBuildError() {
+ DynamicMessage.Builder builder = DynamicMessage.CreateBuilder(TestAllTypes.Descriptor);
+ builder.Build();
+ try {
+ builder.Build();
+ Assert.Fail("Should have thrown exception.");
+ } catch (InvalidOperationException e) {
+ // Success.
+ }
+ }
+
+ [Test]
public void DynamicMessageSettersRejectNull() {
IBuilder builder = DynamicMessage.CreateBuilder(TestAllTypes.Descriptor);
reflectionTester.AssertReflectionSettersRejectNull(builder);
diff --git a/src/ProtocolBuffers.Test/GeneratedMessageTest.cs b/src/ProtocolBuffers.Test/GeneratedMessageTest.cs
index 3abf5bf..1980bbc 100644
--- a/src/ProtocolBuffers.Test/GeneratedMessageTest.cs
+++ b/src/ProtocolBuffers.Test/GeneratedMessageTest.cs
@@ -91,6 +91,18 @@
}
[Test]
+ public void DoubleBuildError() {
+ TestAllTypes.Builder builder = new TestAllTypes.Builder();
+ builder.Build();
+ try {
+ builder.Build();
+ Assert.Fail("Should have thrown exception.");
+ } catch (InvalidOperationException e) {
+ // Success.
+ }
+ }
+
+ [Test]
public void DefaultInstance() {
Assert.AreSame(TestAllTypes.DefaultInstance, TestAllTypes.DefaultInstance.DefaultInstanceForType);
Assert.AreSame(TestAllTypes.DefaultInstance, TestAllTypes.CreateBuilder().DefaultInstanceForType);
@@ -367,7 +379,7 @@
}
[Test]
- public void TestOptimizedForSizeMergeUsesAllFieldsFromTarget() {
+ public void OptimizedForSizeMergeUsesAllFieldsFromTarget() {
TestOptimizedForSize withFieldSet = new TestOptimizedForSize.Builder { I = 10 }.Build();
TestOptimizedForSize.Builder builder = new TestOptimizedForSize.Builder();
builder.MergeFrom(withFieldSet);
diff --git a/src/ProtocolBuffers/AbstractBuilder.cs b/src/ProtocolBuffers/AbstractBuilder.cs
index d7831cd..8c85251 100644
--- a/src/ProtocolBuffers/AbstractBuilder.cs
+++ b/src/ProtocolBuffers/AbstractBuilder.cs
@@ -226,7 +226,7 @@
}
public TBuilder MergeDelimitedFrom(Stream input, ExtensionRegistry extensionRegistry) {
- int size = CodedInputStream.ReadRawVarint32(input);
+ int size = (int) CodedInputStream.ReadRawVarint32(input);
Stream limitedStream = new LimitedInputStream(input, size);
return MergeFrom(limitedStream, extensionRegistry);
}
diff --git a/src/ProtocolBuffers/CodedInputStream.cs b/src/ProtocolBuffers/CodedInputStream.cs
index 89d8e33..0430855 100644
--- a/src/ProtocolBuffers/CodedInputStream.cs
+++ b/src/ProtocolBuffers/CodedInputStream.cs
@@ -466,7 +466,7 @@
/// </summary>
/// <param name="input"></param>
/// <returns></returns>
- internal static int ReadRawVarint32(Stream input) {
+ internal static uint ReadRawVarint32(Stream input) {
int result = 0;
int offset = 0;
for (; offset < 32; offset += 7) {
@@ -476,7 +476,7 @@
}
result |= (b & 0x7f) << offset;
if ((b & 0x80) == 0) {
- return result;
+ return (uint) result;
}
}
// Keep reading up to 64 bits.
@@ -486,7 +486,7 @@
throw InvalidProtocolBufferException.TruncatedMessage();
}
if ((b & 0x80) == 0) {
- return result;
+ return (uint) result;
}
}
throw InvalidProtocolBufferException.MalformedVarint();
@@ -918,16 +918,33 @@
// Then skip directly from the InputStream for the rest.
if (pos < size) {
- // TODO(jonskeet): Java implementation uses skip(). Not sure whether this is really equivalent...
if (input == null) {
throw InvalidProtocolBufferException.TruncatedMessage();
}
- long previousPosition = input.Position;
- input.Position += size - pos;
- if (input.Position != previousPosition + size - pos) {
+ SkipImpl(size - pos);
+ totalBytesRetired += size - pos;
+ }
+ }
+ }
+
+ /// <summary>
+ /// Abstraction of skipping to cope with streams which can't really skip.
+ /// </summary>
+ private void SkipImpl(int amountToSkip) {
+ if (input.CanSeek) {
+ long previousPosition = input.Position;
+ input.Position += amountToSkip;
+ if (input.Position != previousPosition + amountToSkip) {
+ throw InvalidProtocolBufferException.TruncatedMessage();
+ }
+ } else {
+ byte[] skipBuffer = new byte[1024];
+ while (amountToSkip > 0) {
+ int bytesRead = input.Read(skipBuffer, 0, skipBuffer.Length);
+ if (bytesRead <= 0) {
throw InvalidProtocolBufferException.TruncatedMessage();
}
- totalBytesRetired += size - pos;
+ amountToSkip -= bytesRead;
}
}
}
diff --git a/src/ProtocolBuffers/Descriptors/DescriptorPool.cs b/src/ProtocolBuffers/Descriptors/DescriptorPool.cs
index 0e90770..72dd189 100644
--- a/src/ProtocolBuffers/Descriptors/DescriptorPool.cs
+++ b/src/ProtocolBuffers/Descriptors/DescriptorPool.cs
@@ -109,8 +109,7 @@
"package) in file \"" + old.File.Name + "\".");
}
}
- // TODO(jonskeet): Check issue 25 wrt the ordering of these parameters
- descriptorsByName[fullName] = new PackageDescriptor(fullName, name, file);
+ descriptorsByName[fullName] = new PackageDescriptor(name, fullName, file);
}
/// <summary>
diff --git a/src/ProtocolBuffers/DynamicMessage.cs b/src/ProtocolBuffers/DynamicMessage.cs
index 4b3a406..16a7f31 100644
--- a/src/ProtocolBuffers/DynamicMessage.cs
+++ b/src/ProtocolBuffers/DynamicMessage.cs
@@ -296,7 +296,7 @@
}
public override DynamicMessage Build() {
- if (!IsInitialized) {
+ if (fields != null && !IsInitialized) {
throw new UninitializedMessageException(new DynamicMessage(type, fields, unknownFields));
}
return BuildPartial();
@@ -315,6 +315,9 @@
}
public override DynamicMessage BuildPartial() {
+ if (fields == null) {
+ throw new InvalidOperationException("Build() has already been called on this Builder.");
+ }
fields.MakeImmutable();
DynamicMessage result = new DynamicMessage(type, fields, unknownFields);
fields = null;
diff --git a/src/ProtocolBuffers/GeneratedBuilder.cs b/src/ProtocolBuffers/GeneratedBuilder.cs
index c3543be..cdef93f 100644
--- a/src/ProtocolBuffers/GeneratedBuilder.cs
+++ b/src/ProtocolBuffers/GeneratedBuilder.cs
@@ -187,7 +187,8 @@
/// Implementation of <see cref="IBuilder{TMessage, TBuilder}.Build" />.
/// </summary>
public override TMessage Build() {
- if (!IsInitialized) {
+ // If the message is null, we'll throw a more appropriate exception in BuildPartial.
+ if (MessageBeingBuilt != null && !IsInitialized) {
throw new UninitializedMessageException(MessageBeingBuilt);
}
return BuildPartial();