diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index d6a8f52570b..829effc9a22 100644 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -82,6 +82,7 @@ + diff --git a/src/csharp/Grpc.Core.Tests/MetadataTest.cs b/src/csharp/Grpc.Core.Tests/MetadataTest.cs new file mode 100644 index 00000000000..c00f945d6a7 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/MetadataTest.cs @@ -0,0 +1,120 @@ +#region Copyright notice and license + +// Copyright 2015, Google Inc. +// All rights reserved. +// +// 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 + +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; +using Grpc.Core; +using Grpc.Core.Internal; +using Grpc.Core.Utils; +using NUnit.Framework; + +namespace Grpc.Core.Tests +{ + public class MetadataTest + { + [Test] + public void AsciiEntry() + { + var entry = new Metadata.Entry("ABC", "XYZ"); + Assert.IsFalse(entry.IsBinary); + Assert.AreEqual("abc", entry.Key); // key is in lowercase. + Assert.AreEqual("XYZ", entry.Value); + CollectionAssert.AreEqual(new[] { (byte)'X', (byte)'Y', (byte)'Z' }, entry.ValueBytes); + + Assert.Throws(typeof(ArgumentException), () => new Metadata.Entry("abc-bin", "xyz")); + + Assert.AreEqual("[Entry: key=abc, value=XYZ]", entry.ToString()); + } + + [Test] + public void BinaryEntry() + { + var bytes = new byte[] { 1, 2, 3 }; + var entry = new Metadata.Entry("ABC-BIN", bytes); + Assert.IsTrue(entry.IsBinary); + Assert.AreEqual("abc-bin", entry.Key); // key is in lowercase. + Assert.Throws(typeof(InvalidOperationException), () => { var v = entry.Value; }); + CollectionAssert.AreEqual(bytes, entry.ValueBytes); + + Assert.Throws(typeof(ArgumentException), () => new Metadata.Entry("abc", bytes)); + + Assert.AreEqual("[Entry: key=abc-bin, valueBytes=System.Byte[]]", entry.ToString()); + } + + [Test] + public void Entry_ConstructionPreconditions() + { + Assert.Throws(typeof(ArgumentNullException), () => new Metadata.Entry(null, "xyz")); + Assert.Throws(typeof(ArgumentNullException), () => new Metadata.Entry("abc", (string)null)); + Assert.Throws(typeof(ArgumentNullException), () => new Metadata.Entry("abc-bin", (byte[])null)); + } + + [Test] + public void Entry_Immutable() + { + var origBytes = new byte[] { 1, 2, 3 }; + var bytes = new byte[] { 1, 2, 3 }; + var entry = new Metadata.Entry("ABC-BIN", bytes); + bytes[0] = 255; // changing the array passed to constructor should have any effect. + CollectionAssert.AreEqual(origBytes, entry.ValueBytes); + + entry.ValueBytes[0] = 255; + CollectionAssert.AreEqual(origBytes, entry.ValueBytes); + } + + [Test] + public void Entry_CreateUnsafe_Ascii() + { + var bytes = new byte[] { (byte)'X', (byte)'y' }; + var entry = Metadata.Entry.CreateUnsafe("abc", bytes); + Assert.IsFalse(entry.IsBinary); + Assert.AreEqual("abc", entry.Key); + Assert.AreEqual("Xy", entry.Value); + CollectionAssert.AreEqual(bytes, entry.ValueBytes); + } + + [Test] + public void Entry_CreateUnsafe_Binary() + { + var bytes = new byte[] { 1, 2, 3 }; + var entry = Metadata.Entry.CreateUnsafe("abc-bin", bytes); + Assert.IsTrue(entry.IsBinary); + Assert.AreEqual("abc-bin", entry.Key); + Assert.Throws(typeof(InvalidOperationException), () => { var v = entry.Value; }); + CollectionAssert.AreEqual(bytes, entry.ValueBytes); + } + } +} diff --git a/src/csharp/Grpc.Core/ClientBase.cs b/src/csharp/Grpc.Core/ClientBase.cs index 7bc100ca603..903449439b4 100644 --- a/src/csharp/Grpc.Core/ClientBase.cs +++ b/src/csharp/Grpc.Core/ClientBase.cs @@ -119,7 +119,8 @@ namespace Grpc.Core internal static string GetAuthUriBase(string target) { var match = ChannelTargetPattern.Match(target); - if (!match.Success) { + if (!match.Success) + { return null; } return "https://" + match.Groups[2].Value + "/"; diff --git a/src/csharp/Grpc.Core/ContextPropagationToken.cs b/src/csharp/Grpc.Core/ContextPropagationToken.cs index 2e4bfc9e477..a5bf1b5a703 100644 --- a/src/csharp/Grpc.Core/ContextPropagationToken.cs +++ b/src/csharp/Grpc.Core/ContextPropagationToken.cs @@ -132,7 +132,6 @@ namespace Grpc.Core bool propagateDeadline; bool propagateCancellation; - /// /// Creates new context propagation options. /// diff --git a/src/csharp/Grpc.Core/Internal/MetadataArraySafeHandle.cs b/src/csharp/Grpc.Core/Internal/MetadataArraySafeHandle.cs index 427c16fac60..83994f67629 100644 --- a/src/csharp/Grpc.Core/Internal/MetadataArraySafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/MetadataArraySafeHandle.cs @@ -70,7 +70,8 @@ namespace Grpc.Core.Internal var metadataArray = grpcsharp_metadata_array_create(new UIntPtr((ulong)metadata.Count)); for (int i = 0; i < metadata.Count; i++) { - grpcsharp_metadata_array_add(metadataArray, metadata[i].Key, metadata[i].ValueBytes, new UIntPtr((ulong)metadata[i].ValueBytes.Length)); + var valueBytes = metadata[i].GetSerializedValueUnsafe(); + grpcsharp_metadata_array_add(metadataArray, metadata[i].Key, valueBytes, new UIntPtr((ulong)valueBytes.Length)); } return metadataArray; } @@ -94,7 +95,7 @@ namespace Grpc.Core.Internal string key = Marshal.PtrToStringAnsi(grpcsharp_metadata_array_get_key(metadataArray, index)); var bytes = new byte[grpcsharp_metadata_array_get_value_length(metadataArray, index).ToUInt64()]; Marshal.Copy(grpcsharp_metadata_array_get_value(metadataArray, index), bytes, 0, bytes.Length); - metadata.Add(new Metadata.Entry(key, bytes)); + metadata.Add(Metadata.Entry.CreateUnsafe(key, bytes)); } return metadata; } diff --git a/src/csharp/Grpc.Core/Metadata.cs b/src/csharp/Grpc.Core/Metadata.cs index 9db2abf46ef..a589b50caad 100644 --- a/src/csharp/Grpc.Core/Metadata.cs +++ b/src/csharp/Grpc.Core/Metadata.cs @@ -45,6 +45,11 @@ namespace Grpc.Core /// public sealed class Metadata : IList { + /// + /// All binary headers should have this suffix. + /// + public const string BinaryHeaderSuffix = "-bin"; + /// /// An read-only instance of metadata containing no entries. /// @@ -181,23 +186,49 @@ namespace Grpc.Core private static readonly Encoding Encoding = Encoding.ASCII; readonly string key; - string value; - byte[] valueBytes; + readonly string value; + readonly byte[] valueBytes; + + private Entry(string key, string value, byte[] valueBytes) + { + this.key = key; + this.value = value; + this.valueBytes = valueBytes; + } + /// + /// Initializes a new instance of the struct with a binary value. + /// + /// Metadata key, needs to have suffix indicating a binary valued metadata entry. + /// Value bytes. public Entry(string key, byte[] valueBytes) { - this.key = Preconditions.CheckNotNull(key, "key"); + this.key = NormalizeKey(key); + Preconditions.CheckArgument(this.key.EndsWith(BinaryHeaderSuffix), + "Key for binary valued metadata entry needs to have suffix indicating binary value."); this.value = null; - this.valueBytes = Preconditions.CheckNotNull(valueBytes, "valueBytes"); + Preconditions.CheckNotNull(valueBytes, "valueBytes"); + this.valueBytes = new byte[valueBytes.Length]; + Buffer.BlockCopy(valueBytes, 0, this.valueBytes, 0, valueBytes.Length); // defensive copy to guarantee immutability } + /// + /// Initializes a new instance of the struct holding an ASCII value. + /// + /// Metadata key, must not use suffix indicating a binary valued metadata entry. + /// Value string. Only ASCII characters are allowed. public Entry(string key, string value) { - this.key = Preconditions.CheckNotNull(key, "key"); + this.key = NormalizeKey(key); + Preconditions.CheckArgument(!this.key.EndsWith(BinaryHeaderSuffix), + "Key for ASCII valued metadata entry cannot have suffix indicating binary value."); this.value = Preconditions.CheckNotNull(value, "value"); this.valueBytes = null; } + /// + /// Gets the metadata entry key. + /// public string Key { get @@ -206,33 +237,86 @@ namespace Grpc.Core } } + /// + /// Gets the binary value of this metadata entry. + /// public byte[] ValueBytes { get { if (valueBytes == null) { - valueBytes = Encoding.GetBytes(value); + return Encoding.GetBytes(value); } - return valueBytes; + + // defensive copy to guarantee immutability + var bytes = new byte[valueBytes.Length]; + Buffer.BlockCopy(valueBytes, 0, bytes, 0, valueBytes.Length); + return bytes; } } + /// + /// Gets the string value of this metadata entry. + /// public string Value { get { - if (value == null) - { - value = Encoding.GetString(valueBytes); - } - return value; + Preconditions.CheckState(!IsBinary, "Cannot access string value of a binary metadata entry"); + return value ?? Encoding.GetString(valueBytes); } } - + + /// + /// Returns true if this entry is a binary-value entry. + /// + public bool IsBinary + { + get + { + return value == null; + } + } + + /// + /// Returns a that represents the current . + /// public override string ToString() { - return string.Format("[Entry: key={0}, value={1}]", Key, Value); + if (IsBinary) + { + return string.Format("[Entry: key={0}, valueBytes={1}]", key, valueBytes); + } + + return string.Format("[Entry: key={0}, value={1}]", key, value); + } + + /// + /// Gets the serialized value for this entry. For binary metadata entries, this leaks + /// the internal valueBytes byte array and caller must not change contents of it. + /// + internal byte[] GetSerializedValueUnsafe() + { + return valueBytes ?? Encoding.GetBytes(value); + } + + /// + /// Creates a binary value or ascii value metadata entry from data received from the native layer. + /// We trust C core to give us well-formed data, so we don't perform any checks or defensive copying. + /// + internal static Entry CreateUnsafe(string key, byte[] valueBytes) + { + if (key.EndsWith(BinaryHeaderSuffix)) + { + return new Entry(key, null, valueBytes); + } + return new Entry(key, Encoding.GetString(valueBytes), null); + } + + private static string NormalizeKey(string key) + { + return Preconditions.CheckNotNull(key, "key").ToLower(); } } } diff --git a/src/csharp/Grpc.IntegrationTesting/InteropClient.cs b/src/csharp/Grpc.IntegrationTesting/InteropClient.cs index f4b0a1028f9..423da2801e0 100644 --- a/src/csharp/Grpc.IntegrationTesting/InteropClient.cs +++ b/src/csharp/Grpc.IntegrationTesting/InteropClient.cs @@ -37,13 +37,15 @@ using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; +using Google.Apis.Auth.OAuth2; using Google.ProtocolBuffers; + using grpc.testing; using Grpc.Auth; using Grpc.Core; using Grpc.Core.Utils; + using NUnit.Framework; -using Google.Apis.Auth.OAuth2; namespace Grpc.IntegrationTesting {