diff options
Diffstat (limited to 'csharp')
12 files changed, 366 insertions, 134 deletions
diff --git a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs index c8326d80..54c44e47 100644 --- a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs +++ b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs @@ -320,38 +320,18 @@ namespace Google.Protobuf Assert.Throws<InvalidProtocolBufferException>(() => TestRecursiveMessage.Parser.ParseFrom(data65));
- CodedInputStream input = data64.CreateCodedInput();
- input.SetRecursionLimit(8);
+ CodedInputStream input = CodedInputStream.CreateWithLimits(new MemoryStream(data64.ToByteArray()), 1000000, 63);
Assert.Throws<InvalidProtocolBufferException>(() => TestRecursiveMessage.Parser.ParseFrom(input));
}
- /*
[Test]
public void SizeLimit()
{
// Have to use a Stream rather than ByteString.CreateCodedInput as SizeLimit doesn't
// apply to the latter case.
- MemoryStream ms = new MemoryStream(TestUtil.GetAllSet().ToByteString().ToByteArray());
- CodedInputStream input = new CodedInputStream(ms);
- input.SetSizeLimit(16);
-
- Assert.Throws<InvalidProtocolBufferException>(() => TestAllTypes.ParseFrom(input));
- }*/
-
- [Test]
- public void ResetSizeCounter()
- {
- CodedInputStream input = new CodedInputStream(
- new SmallBlockInputStream(new byte[256], 8));
- input.SetSizeLimit(16);
- input.ReadRawBytes(16);
-
- Assert.Throws<InvalidProtocolBufferException>(() => input.ReadRawByte());
-
- input.ResetSizeCounter();
- input.ReadRawByte(); // No exception thrown.
-
- Assert.Throws<InvalidProtocolBufferException>(() => input.ReadRawBytes(16));
+ MemoryStream ms = new MemoryStream(SampleMessages.CreateFullTestAllTypes().ToByteArray());
+ CodedInputStream input = CodedInputStream.CreateWithLimits(ms, 16, 100);
+ Assert.Throws<InvalidProtocolBufferException>(() => TestAllTypes.Parser.ParseFrom(input));
}
/// <summary>
@@ -423,7 +403,7 @@ namespace Google.Protobuf output.Flush();
ms.Position = 0;
- CodedInputStream input = new CodedInputStream(ms, new byte[ms.Length / 2]);
+ CodedInputStream input = new CodedInputStream(ms, new byte[ms.Length / 2], 0, 0);
uint tag = input.ReadTag();
Assert.AreEqual(1, WireFormat.GetTagFieldNumber(tag));
@@ -528,5 +508,23 @@ namespace Google.Protobuf Assert.AreEqual(WireFormat.MakeTag(1, WireFormat.WireType.StartGroup), input.ReadTag());
Assert.Throws<InvalidProtocolBufferException>(() => input.SkipLastField());
}
+
+ [Test]
+ public void Construction_Invalid()
+ {
+ Assert.Throws<ArgumentNullException>(() => new CodedInputStream((byte[]) null));
+ Assert.Throws<ArgumentNullException>(() => new CodedInputStream(null, 0, 0));
+ Assert.Throws<ArgumentNullException>(() => new CodedInputStream((Stream) null));
+ Assert.Throws<ArgumentOutOfRangeException>(() => new CodedInputStream(new byte[10], 100, 0));
+ Assert.Throws<ArgumentOutOfRangeException>(() => new CodedInputStream(new byte[10], 5, 10));
+ }
+
+ [Test]
+ public void CreateWithLimits_InvalidLimits()
+ {
+ var stream = new MemoryStream();
+ Assert.Throws<ArgumentOutOfRangeException>(() => CodedInputStream.CreateWithLimits(stream, 0, 1));
+ Assert.Throws<ArgumentOutOfRangeException>(() => CodedInputStream.CreateWithLimits(stream, 1, 0));
+ }
}
}
\ 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 c1bf7bd6..3297fe87 100644 --- a/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs +++ b/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs @@ -334,7 +334,7 @@ namespace Google.Protobuf }
// Now test Input stream:
{
- CodedInputStream cin = new CodedInputStream(new MemoryStream(bytes), new byte[50]);
+ CodedInputStream cin = new CodedInputStream(new MemoryStream(bytes), new byte[50], 0, 0);
Assert.AreEqual(0, cin.Position);
// Field 1:
uint tag = cin.ReadTag();
diff --git a/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs b/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs index c62ac046..29c4c2a9 100644 --- a/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs +++ b/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs @@ -477,6 +477,91 @@ namespace Google.Protobuf.Collections Assert.IsTrue(new MapField<int, int?>(true).AllowsNullValues); } + [Test] + public void KeysReturnsLiveView() + { + var map = new MapField<string, string>(); + var keys = map.Keys; + CollectionAssert.AreEqual(new string[0], keys); + map["foo"] = "bar"; + map["x"] = "y"; + CollectionAssert.AreEqual(new[] { "foo", "x" }, keys); + } + + [Test] + public void ValuesReturnsLiveView() + { + var map = new MapField<string, string>(); + var values = map.Values; + CollectionAssert.AreEqual(new string[0], values); + map["foo"] = "bar"; + map["x"] = "y"; + CollectionAssert.AreEqual(new[] { "bar", "y" }, values); + } + + // Just test keys - we know the implementation is the same for values + [Test] + public void ViewsAreReadOnly() + { + var map = new MapField<string, string>(); + var keys = map.Keys; + Assert.IsTrue(keys.IsReadOnly); + Assert.Throws<NotSupportedException>(() => keys.Clear()); + Assert.Throws<NotSupportedException>(() => keys.Remove("a")); + Assert.Throws<NotSupportedException>(() => keys.Add("a")); + } + + // Just test keys - we know the implementation is the same for values + [Test] + public void ViewCopyTo() + { + var map = new MapField<string, string> { { "foo", "bar" }, { "x", "y" } }; + var keys = map.Keys; + var array = new string[4]; + Assert.Throws<ArgumentException>(() => keys.CopyTo(array, 3)); + Assert.Throws<ArgumentOutOfRangeException>(() => keys.CopyTo(array, -1)); + keys.CopyTo(array, 1); + CollectionAssert.AreEqual(new[] { null, "foo", "x", null }, array); + } + + // Just test keys - we know the implementation is the same for values + [Test] + public void NonGenericViewCopyTo() + { + IDictionary map = new MapField<string, string> { { "foo", "bar" }, { "x", "y" } }; + ICollection keys = map.Keys; + // Note the use of the Array type here rather than string[] + Array array = new string[4]; + Assert.Throws<ArgumentException>(() => keys.CopyTo(array, 3)); + Assert.Throws<ArgumentOutOfRangeException>(() => keys.CopyTo(array, -1)); + keys.CopyTo(array, 1); + CollectionAssert.AreEqual(new[] { null, "foo", "x", null }, array); + } + + [Test] + public void KeysContains() + { + var map = new MapField<string, string> { { "foo", "bar" }, { "x", "y" } }; + var keys = map.Keys; + Assert.IsTrue(keys.Contains("foo")); + Assert.IsFalse(keys.Contains("bar")); // It's a value! + Assert.IsFalse(keys.Contains("1")); + // Keys can't be null, so we should prevent contains check + Assert.Throws<ArgumentNullException>(() => keys.Contains(null)); + } + + [Test] + public void ValuesContains() + { + var map = new MapField<string, string> { { "foo", "bar" }, { "x", "y" } }; + var values = map.Values; + Assert.IsTrue(values.Contains("bar")); + Assert.IsFalse(values.Contains("foo")); // It's a key! + Assert.IsFalse(values.Contains("1")); + // Values can be null, so this makes sense + Assert.IsFalse(values.Contains(null)); + } + private static KeyValuePair<TKey, TValue> NewKeyValuePair<TKey, TValue>(TKey key, TValue value) { return new KeyValuePair<TKey, TValue>(key, value); diff --git a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs index 6e1d804e..936e41c6 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs @@ -213,6 +213,6 @@ namespace Google.Protobuf.Reflection var descriptor = TestAllTypes.Descriptor; Assert.Throws<KeyNotFoundException>(() => descriptor.Fields[999999].ToString()); Assert.Throws<KeyNotFoundException>(() => descriptor.Fields["not found"].ToString()); - } + } } } diff --git a/csharp/src/Google.Protobuf/CodedInputStream.cs b/csharp/src/Google.Protobuf/CodedInputStream.cs index c8b33b33..82c6ceab 100644 --- a/csharp/src/Google.Protobuf/CodedInputStream.cs +++ b/csharp/src/Google.Protobuf/CodedInputStream.cs @@ -53,16 +53,13 @@ namespace Google.Protobuf /// </remarks>
public sealed class CodedInputStream
{
- // TODO(jonskeet): Consider whether recursion and size limits shouldn't be readonly,
- // set at construction time.
-
/// <summary>
/// Buffer of data read from the stream or provided at construction time.
/// </summary>
private readonly byte[] buffer;
/// <summary>
- /// The number of valid bytes in the buffer.
+ /// The index of the buffer at which we need to refill from the stream (if there is one).
/// </summary>
private int bufferSize;
@@ -106,62 +103,104 @@ namespace Google.Protobuf /// </summary>
private int currentLimit = int.MaxValue;
- /// <summary>
- /// <see cref="SetRecursionLimit"/>
- /// </summary>
private int recursionDepth = 0;
- private int recursionLimit = DefaultRecursionLimit;
+ private readonly int recursionLimit;
+ private readonly int sizeLimit;
+
+ #region Construction
+ // Note that the checks are performed such that we don't end up checking obviously-valid things
+ // like non-null references for arrays we've just created.
/// <summary>
- /// <see cref="SetSizeLimit"/>
+ /// Creates a new CodedInputStream reading data from the given byte array.
/// </summary>
- private int sizeLimit = DefaultSizeLimit;
+ public CodedInputStream(byte[] buffer) : this(null, Preconditions.CheckNotNull(buffer, "buffer"), 0, buffer.Length)
+ {
+ }
- #region Construction
/// <summary>
- /// Creates a new CodedInputStream reading data from the given
- /// byte array.
+ /// Creates a new CodedInputStream that reads from the given byte array slice.
/// </summary>
- public CodedInputStream(byte[] buf) : this(buf, 0, buf.Length)
- {
+ public CodedInputStream(byte[] buffer, int offset, int length)
+ : this(null, Preconditions.CheckNotNull(buffer, "buffer"), offset, offset + length)
+ {
+ if (offset < 0 || offset > buffer.Length)
+ {
+ throw new ArgumentOutOfRangeException("offset", "Offset must be within the buffer");
+ }
+ if (length < 0 || offset + length > buffer.Length)
+ {
+ throw new ArgumentOutOfRangeException("length", "Length must be non-negative and within the buffer");
+ }
}
/// <summary>
- /// Creates a new CodedInputStream that reads from the given
- /// byte array slice.
+ /// Creates a new CodedInputStream reading data from the given stream.
/// </summary>
- public CodedInputStream(byte[] buffer, int offset, int length)
+ public CodedInputStream(Stream input) : this(input, new byte[BufferSize], 0, 0)
{
- this.buffer = buffer;
- this.bufferPos = offset;
- this.bufferSize = offset + length;
- this.input = null;
+ Preconditions.CheckNotNull(input, "input");
}
/// <summary>
- /// Creates a new CodedInputStream reading data from the given stream.
+ /// Creates a new CodedInputStream reading data from the given
+ /// stream and buffer, using the default limits.
/// </summary>
- public CodedInputStream(Stream input)
+ internal CodedInputStream(Stream input, byte[] buffer, int bufferPos, int bufferSize)
{
- this.buffer = new byte[BufferSize];
- this.bufferSize = 0;
this.input = input;
+ this.buffer = buffer;
+ this.bufferPos = bufferPos;
+ this.bufferSize = bufferSize;
+ this.sizeLimit = DefaultSizeLimit;
+ this.recursionLimit = DefaultRecursionLimit;
}
/// <summary>
/// Creates a new CodedInputStream reading data from the given
- /// stream, with a pre-allocated buffer.
+ /// stream and buffer, using the specified limits.
/// </summary>
- internal CodedInputStream(Stream input, byte[] buffer)
+ /// <remarks>
+ /// This chains to the version with the default limits instead of vice versa to avoid
+ /// having to check that the default values are valid every time.
+ /// </remarks>
+ internal CodedInputStream(Stream input, byte[] buffer, int bufferPos, int bufferSize, int sizeLimit, int recursionLimit)
+ : this(input, buffer, bufferPos, bufferSize)
{
- this.buffer = buffer;
- this.bufferSize = 0;
- this.input = input;
+ if (sizeLimit <= 0)
+ {
+ throw new ArgumentOutOfRangeException("sizeLimit", "Size limit must be positive");
+ }
+ if (recursionLimit <= 0)
+ {
+ throw new ArgumentOutOfRangeException("recursionLimit!", "Recursion limit must be positive");
+ }
+ this.sizeLimit = sizeLimit;
+ this.recursionLimit = recursionLimit;
}
#endregion
/// <summary>
+ /// Creates a <see cref="CodedInputStream"/> with the specified size and recursion limits, reading
+ /// from an input stream.
+ /// </summary>
+ /// <remarks>
+ /// This method exists separately from the constructor to reduce the number of constructor overloads.
+ /// It is likely to be used considerably less frequently than the constructors, as the default limits
+ /// are suitable for most use cases.
+ /// </remarks>
+ /// <param name="input">The input stream to read from</param>
+ /// <param name="sizeLimit">The total limit of data to read from the stream.</param>
+ /// <param name="recursionLimit">The maximum recursion depth to allow while reading.</param>
+ /// <returns>A <c>CodedInputStream</c> reading from <paramref name="input"/> with the specified size
+ /// and recursion limits.</returns>
+ public static CodedInputStream CreateWithLimits(Stream input, int sizeLimit, int recursionLimit)
+ {
+ return new CodedInputStream(input, new byte[BufferSize], 0, 0, sizeLimit, recursionLimit);
+ }
+
+ /// <summary>
/// Returns the current position in the input stream, or the position in the input buffer
/// </summary>
public long Position
@@ -182,59 +221,30 @@ namespace Google.Protobuf /// </summary>
internal uint LastTag { get { return lastTag; } }
- #region Limits for recursion and length
/// <summary>
- /// Set the maximum message recursion depth.
+ /// Returns the size limit for this stream.
/// </summary>
/// <remarks>
- /// In order to prevent malicious
- /// messages from causing stack overflows, CodedInputStream limits
- /// how deeply messages may be nested. The default limit is 64.
+ /// This limit is applied when reading from the underlying stream, as a sanity check. It is
+ /// not applied when reading from a byte array data source without an underlying stream.
+ /// The default value is 64MB.
/// </remarks>
- public int SetRecursionLimit(int limit)
- {
- if (limit < 0)
- {
- throw new ArgumentOutOfRangeException("Recursion limit cannot be negative: " + limit);
- }
- int oldLimit = recursionLimit;
- recursionLimit = limit;
- return oldLimit;
- }
+ /// <value>
+ /// The size limit.
+ /// </value>
+ public int SizeLimit { get { return sizeLimit; } }
/// <summary>
- /// Set the maximum message size.
+ /// Returns the recursion limit for this stream. This limit is applied whilst reading messages,
+ /// to avoid maliciously-recursive data.
/// </summary>
/// <remarks>
- /// In order to prevent malicious messages from exhausting memory or
- /// causing integer overflows, CodedInputStream limits how large a message may be.
- /// The default limit is 64MB. You should set this limit as small
- /// as you can without harming your app's functionality. Note that
- /// size limits only apply when reading from an InputStream, not
- /// when constructed around a raw byte array (nor with ByteString.NewCodedInput).
- /// If you want to read several messages from a single CodedInputStream, you
- /// can call ResetSizeCounter() after each message to avoid hitting the
- /// size limit.
+ /// The default limit is 64.
/// </remarks>
- public int SetSizeLimit(int limit)
- {
- if (limit < 0)
- {
- throw new ArgumentOutOfRangeException("Size limit cannot be negative: " + limit);
- }
- int oldLimit = sizeLimit;
- sizeLimit = limit;
- return oldLimit;
- }
-
- /// <summary>
- /// Resets the current size counter to zero (see <see cref="SetSizeLimit"/>).
- /// </summary>
- public void ResetSizeCounter()
- {
- totalBytesRetired = 0;
- }
- #endregion
+ /// <value>
+ /// The recursion limit for this stream.
+ /// </value>
+ public int RecursionLimit { get { return recursionLimit; } }
#region Validation
/// <summary>
diff --git a/csharp/src/Google.Protobuf/Collections/MapField.cs b/csharp/src/Google.Protobuf/Collections/MapField.cs index dc4b04cb..0fa63bef 100644 --- a/csharp/src/Google.Protobuf/Collections/MapField.cs +++ b/csharp/src/Google.Protobuf/Collections/MapField.cs @@ -136,6 +136,12 @@ namespace Google.Protobuf.Collections return map.ContainsKey(key); } + private bool ContainsValue(TValue value) + { + var comparer = EqualityComparer<TValue>.Default; + return list.Any(pair => comparer.Equals(pair.Value, value)); + } + /// <summary> /// Removes the entry identified by the given key from the map. /// </summary> @@ -221,17 +227,15 @@ namespace Google.Protobuf.Collections } } - // TODO: Make these views? - /// <summary> /// Gets a collection containing the keys in the map. /// </summary> - public ICollection<TKey> Keys { get { return list.Select(t => t.Key).ToList(); } } + public ICollection<TKey> Keys { get { return new MapView<TKey>(this, pair => pair.Key, ContainsKey); } } /// <summary> /// Gets a collection containing the values in the map. /// </summary> - public ICollection<TValue> Values { get { return list.Select(t => t.Value).ToList(); } } + public ICollection<TValue> Values { get { return new MapView<TValue>(this, pair => pair.Value, ContainsValue); } } /// <summary> /// Adds the specified entries to the map. @@ -658,5 +662,92 @@ namespace Google.Protobuf.Collections MessageDescriptor IMessage.Descriptor { get { return null; } } } } + + private class MapView<T> : ICollection<T>, ICollection + { + private readonly MapField<TKey, TValue> parent; + private readonly Func<KeyValuePair<TKey, TValue>, T> projection; + private readonly Func<T, bool> containsCheck; + + internal MapView( + MapField<TKey, TValue> parent, + Func<KeyValuePair<TKey, TValue>, T> projection, + Func<T, bool> containsCheck) + { + this.parent = parent; + this.projection = projection; + this.containsCheck = containsCheck; + } + + public int Count { get { return parent.Count; } } + + public bool IsReadOnly { get { return true; } } + + public bool IsSynchronized { get { return false; } } + + public object SyncRoot { get { return parent; } } + + public void Add(T item) + { + throw new NotSupportedException(); + } + + public void Clear() + { + throw new NotSupportedException(); + } + + public bool Contains(T item) + { + return containsCheck(item); + } + + public void CopyTo(T[] array, int arrayIndex) + { + if (arrayIndex < 0) + { + throw new ArgumentOutOfRangeException("arrayIndex"); + } + if (arrayIndex + Count >= array.Length) + { + throw new ArgumentException("Not enough space in the array", "array"); + } + foreach (var item in this) + { + array[arrayIndex++] = item; + } + } + + public IEnumerator<T> GetEnumerator() + { + return parent.list.Select(projection).GetEnumerator(); + } + + public bool Remove(T item) + { + throw new NotSupportedException(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + public void CopyTo(Array array, int index) + { + if (index < 0) + { + throw new ArgumentOutOfRangeException("index"); + } + if (index + Count >= array.Length) + { + throw new ArgumentException("Not enough space in the array", "array"); + } + foreach (var item in this) + { + array.SetValue(item, index++); + } + } + } } } diff --git a/csharp/src/Google.Protobuf/Google.Protobuf.csproj b/csharp/src/Google.Protobuf/Google.Protobuf.csproj index 033edfbb..a17bf81c 100644 --- a/csharp/src/Google.Protobuf/Google.Protobuf.csproj +++ b/csharp/src/Google.Protobuf/Google.Protobuf.csproj @@ -83,6 +83,7 @@ <Compile Include="Compatibility\TypeExtensions.cs" />
<Compile Include="FieldCodec.cs" />
<Compile Include="FrameworkPortability.cs" />
+ <Compile Include="IDeepCloneable.cs" />
<Compile Include="JsonFormatter.cs" />
<Compile Include="MessageExtensions.cs" />
<Compile Include="IMessage.cs" />
diff --git a/csharp/src/Google.Protobuf/IDeepCloneable.cs b/csharp/src/Google.Protobuf/IDeepCloneable.cs new file mode 100644 index 00000000..c9c71bbe --- /dev/null +++ b/csharp/src/Google.Protobuf/IDeepCloneable.cs @@ -0,0 +1,54 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2015 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#endregion + +namespace Google.Protobuf +{ + /// <summary> + /// Generic interface for a deeply cloneable type. + /// </summary> + /// <remarks> + /// <para> + /// All generated messages implement this interface, but so do some non-message types. + /// Additionally, due to the type constraint on <c>T</c> in <see cref="IMessage{T}"/>, + /// it is simpler to keep this as a separate interface. + /// </para> + /// </remarks> + /// <typeparam name="T">The type itself, returned by the <see cref="Clone"/> method.</typeparam> + public interface IDeepCloneable<T> + { + /// <summary> + /// Creates a deep clone of this object. + /// </summary> + /// <returns>A deep clone of this object.</returns> + T Clone(); + } +} diff --git a/csharp/src/Google.Protobuf/IMessage.cs b/csharp/src/Google.Protobuf/IMessage.cs index 773845ff..d089f946 100644 --- a/csharp/src/Google.Protobuf/IMessage.cs +++ b/csharp/src/Google.Protobuf/IMessage.cs @@ -35,8 +35,6 @@ using Google.Protobuf.Reflection; namespace Google.Protobuf
{
- // TODO(jonskeet): Split these interfaces into separate files when we're happy with them.
-
/// <summary>
/// Interface for a Protocol Buffers message, supporting
/// basic operations required for serialization.
@@ -86,24 +84,4 @@ namespace Google.Protobuf /// <param name="message">The message to merge with this one. Must not be null.</param>
void MergeFrom(T message);
}
-
- /// <summary>
- /// Generic interface for a deeply cloneable type.
- /// </summary>
- /// <remarks>
- /// <para>
- /// All generated messages implement this interface, but so do some non-message types.
- /// Additionally, due to the type constraint on <c>T</c> in <see cref="IMessage{T}"/>,
- /// it is simpler to keep this as a separate interface.
- /// </para>
- /// </remarks>
- /// <typeparam name="T">The type itself, returned by the <see cref="Clone"/> method.</typeparam>
- public interface IDeepCloneable<T>
- {
- /// <summary>
- /// Creates a deep clone of this object.
- /// </summary>
- /// <returns>A deep clone of this object.</returns>
- T Clone();
- }
}
diff --git a/csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs b/csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs index 157ea400..759955e6 100644 --- a/csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs +++ b/csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs @@ -257,10 +257,12 @@ namespace Google.Protobuf.Reflection /// or unqualified. C++-like name lookup semantics are used to search for the /// matching descriptor. /// </summary> + /// <remarks> + /// This isn't heavily optimized, but it's only used during cross linking anyway. + /// If it starts being used more widely, we should look at performance more carefully. + /// </remarks> internal IDescriptor LookupSymbol(string name, IDescriptor relativeTo) { - // TODO(jonskeet): This could be optimized in a number of ways. - IDescriptor result; if (name.StartsWith(".")) { @@ -282,7 +284,6 @@ namespace Google.Protobuf.Reflection { // Chop off the last component of the scope. - // TODO(jonskeet): Make this more efficient. May not be worth using StringBuilder at all int dotpos = scopeToTry.ToString().LastIndexOf("."); if (dotpos == -1) { diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index bb8e9bbb..901cbf47 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -168,14 +168,16 @@ namespace Google.Protobuf.Reflection get { return fieldType == FieldType.Message && messageType.Proto.Options != null && messageType.Proto.Options.MapEntry; } } - // TODO(jonskeet): Check whether this is correct with proto3, where we default to packed... - /// <summary> /// Returns <c>true</c> if this field is a packed, repeated field; <c>false</c> otherwise. /// </summary> public bool IsPacked { - get { return Proto.Options != null && Proto.Options.Packed; } + // Note the || rather than && here - we're effectively defaulting to packed, because that *is* + // the default in proto3, which is all we support. We may give the wrong result for the protos + // within descriptor.proto, but that's okay, as they're never exposed and we don't use IsPacked + // within the runtime. + get { return Proto.Options == null || Proto.Options.Packed; } } /// <summary> diff --git a/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs b/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs index c7ed4342..8c055d6d 100644 --- a/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs +++ b/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs @@ -44,4 +44,16 @@ namespace Google.Protobuf.Reflection OneofIndex = -1;
}
}
+
+ internal partial class FieldOptions
+ {
+ // We can't tell the difference between "explicitly set to false" and "not set"
+ // in proto3, but we need to tell the difference for FieldDescriptor.IsPacked.
+ // This won't work if we ever need to support proto2, but at that point we'll be
+ // able to remove this hack and use field presence instead.
+ partial void OnConstruction()
+ {
+ Packed = true;
+ }
+ }
}
\ No newline at end of file |