From 1a775737964c575fd743c97c3c47534825846910 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 19 Mar 2020 22:24:30 +0000 Subject: [PATCH 01/12] Implement indefinite length writes --- .../tests/Cbor.Tests/CborWriterTests.Array.cs | 33 ++++++++ .../Cbor.Tests/CborWriterTests.Helpers.cs | 75 +++++++++++++++-- .../tests/Cbor.Tests/CborWriterTests.Map.cs | 47 +++++++++++ .../Cbor.Tests/CborWriterTests.String.cs | 82 +++++++++++++++++++ .../tests/Cbor/CborInitialByte.cs | 2 + .../tests/Cbor/CborWriter.Array.cs | 15 ++++ .../tests/Cbor/CborWriter.Integer.cs | 29 +++---- .../tests/Cbor/CborWriter.Map.cs | 15 ++++ .../tests/Cbor/CborWriter.String.cs | 30 +++++++ .../tests/Cbor/CborWriter.cs | 78 +++++++++++++----- 10 files changed, 361 insertions(+), 45 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Array.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Array.cs index e690655b34072..d96346b5c77df 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Array.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Array.cs @@ -43,6 +43,39 @@ public static void WriteArray_NestedValues_HappyPath(object[] values, string exp AssertHelper.HexEqual(expectedEncoding, actualEncoding); } + [Theory] + [InlineData(new object[] { }, "9fff")] + [InlineData(new object[] { 42 }, "9f182aff")] + [InlineData(new object[] { 1, 2, 3 }, "9f010203ff")] + [InlineData(new object[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25 }, "9f0102030405060708090a0b0c0d0e0f101112131415161718181819ff")] + [InlineData(new object[] { 1, -1, "", new byte[] { 7 } }, "9f0120604107ff")] + [InlineData(new object[] { "lorem", "ipsum", "dolor" }, "9f656c6f72656d65697073756d65646f6c6f72ff")] + public static void WriteArray_IndefiniteLength_HappyPath(object[] values, string expectedHexEncoding) + { + byte[] expectedEncoding = expectedHexEncoding.HexToByteArray(); + + using var writer = new CborWriter(); + Helpers.WriteArray(writer, values, useDefiniteLengthCollections: false); + + byte[] actualEncoding = writer.ToArray(); + AssertHelper.HexEqual(expectedEncoding, actualEncoding); + } + + [Theory] + [InlineData(new object[] { new object[] { } }, "9f9fffff")] + [InlineData(new object[] { 1, new object[] { 2, 3 }, new object[] { 4, 5 } }, "9f019f0203ff9f0405ffff")] + [InlineData(new object[] { "", new object[] { new object[] { }, new object[] { 1, new byte[] { 10 } } } }, "9f609f9fff9f01410affffff")] + public static void WriteArray_IndefiniteLength_NestedValues_HappyPath(object[] values, string expectedHexEncoding) + { + byte[] expectedEncoding = expectedHexEncoding.HexToByteArray(); + + using var writer = new CborWriter(); + Helpers.WriteArray(writer, values, useDefiniteLengthCollections: false); + + byte[] actualEncoding = writer.ToArray(); + AssertHelper.HexEqual(expectedEncoding, actualEncoding); + } + [Theory] [InlineData(0)] [InlineData(1)] diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs index e276dc962f295..8ba20aef41e7e 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs @@ -22,7 +22,7 @@ public static bool IsCborMapRepresentation(object[] values) return values.Length % 2 == 1 && values[0] is string s && s == MapPrefixIdentifier; } - public static void WriteValue(CborWriter writer, object value) + public static void WriteValue(CborWriter writer, object value, bool useDefiniteLengthCollections = true) { switch (value) { @@ -31,38 +31,95 @@ public static void WriteValue(CborWriter writer, object value) case ulong i: writer.WriteUInt64(i); break; case string s: writer.WriteTextString(s); break; case byte[] b: writer.WriteByteString(b); break; - case object[] nested when IsCborMapRepresentation(nested): WriteMap(writer, nested); break; - case object[] nested: WriteArray(writer, nested); break; + case byte[][] chunks: WriteChunkedByteString(writer, chunks); break; + case string[] chunks: WriteChunkedTextString(writer, chunks); break; + case object[] nested when IsCborMapRepresentation(nested): WriteMap(writer, nested, useDefiniteLengthCollections); break; + case object[] nested: WriteArray(writer, nested, useDefiniteLengthCollections); break; default: throw new ArgumentException($"Unrecognized argument type {value.GetType()}"); }; } - public static void WriteArray(CborWriter writer, params object[] values) + public static void WriteArray(CborWriter writer, object[] values, bool useDefiniteLengthCollections = true) { - writer.WriteStartArray(values.Length); + if (useDefiniteLengthCollections) + { + writer.WriteStartArray(values.Length); + } + else + { + writer.WriteStartArray(); + } + foreach (object value in values) { - WriteValue(writer, value); + WriteValue(writer, value, useDefiniteLengthCollections); } + writer.WriteEndArray(); } - public static void WriteMap(CborWriter writer, params object[] keyValuePairs) + public static void WriteMap(CborWriter writer, object[] keyValuePairs, bool useDefiniteLengthCollections = true) { if (!IsCborMapRepresentation(keyValuePairs)) { throw new ArgumentException($"CBOR map representation must contain odd number of elements prepended with a '{MapPrefixIdentifier}' constant."); } - writer.WriteStartMap(keyValuePairs.Length / 2); + if (useDefiniteLengthCollections) + { + writer.WriteStartMap(keyValuePairs.Length / 2); + } + else + { + writer.WriteStartMap(); + } foreach (object value in keyValuePairs.Skip(1)) { - WriteValue(writer, value); + WriteValue(writer, value, useDefiniteLengthCollections); } writer.WriteEndMap(); } + + public static void WriteChunkedByteString(CborWriter writer, byte[][] chunks) + { + writer.WriteStartByteString(); + foreach (byte[] chunk in chunks) + { + writer.WriteByteString(chunk); + } + writer.WriteEndByteString(); + } + + public static void WriteChunkedTextString(CborWriter writer, string[] chunks) + { + writer.WriteStartTextString(); + foreach (string chunk in chunks) + { + writer.WriteTextString(chunk); + } + writer.WriteEndTextString(); + } + + public static void ExecOperation(CborWriter writer, string op) + { + switch (op) + { + case nameof(writer.WriteInt64): writer.WriteInt64(42); break; + case nameof(writer.WriteByteString): writer.WriteByteString(Array.Empty()); break; + case nameof(writer.WriteTextString): writer.WriteTextString(""); break; + case nameof(writer.WriteStartTextString): writer.WriteStartTextString(); break; + case nameof(writer.WriteStartByteString): writer.WriteStartByteString(); break; + case nameof(writer.WriteStartArray): writer.WriteStartArray(); break; + case nameof(writer.WriteStartMap): writer.WriteStartMap(); break; + case nameof(writer.WriteEndByteString): writer.WriteEndByteString(); break; + case nameof(writer.WriteEndTextString): writer.WriteEndTextString(); break; + case nameof(writer.WriteEndArray): writer.WriteEndArray(); break; + case nameof(writer.WriteEndMap): writer.WriteEndMap(); break; + default: throw new Exception($"Unrecognized CborWriter operation name {op}"); + } + } } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Map.cs index 46c54b0d8124d..18514ec646820 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Map.cs @@ -44,6 +44,33 @@ public static void WriteMap_NestedValues_HappyPath(object[] values, string expec AssertHelper.HexEqual(expectedEncoding, actualEncoding); } + [Theory] + [InlineData(new object[] { Map }, "bfff")] + [InlineData(new object[] { Map, 1, 2, 3, 4 }, "bf01020304ff")] + [InlineData(new object[] { Map, "a", "A", "b", "B", "c", "C", "d", "D", "e", "E" }, "bf6161614161626142616361436164614461656145ff")] + [InlineData(new object[] { Map, "a", "A", -1, 2, new byte[] { }, new byte[] { 1 } }, "bf616161412002404101ff")] + public static void WriteMap_IndefiniteLength_SimpleValues_HappyPath(object[] values, string expectedHexEncoding) + { + byte[] expectedEncoding = expectedHexEncoding.HexToByteArray(); + using var writer = new CborWriter(); + Helpers.WriteMap(writer, values, useDefiniteLengthCollections: false); + byte[] actualEncoding = writer.ToArray(); + AssertHelper.HexEqual(expectedEncoding, actualEncoding); + } + + [Theory] + [InlineData(new object[] { Map, "a", 1, "b", new object[] { Map, 2, 3 } }, "bf6161016162bf0203ffff")] + [InlineData(new object[] { Map, "a", new object[] { Map, 2, 3 }, "b", new object[] { Map, "x", -1, "y", new object[] { Map, "z", 0 } } }, "bf6161bf0203ff6162bf6178206179bf617a00ffffff")] + [InlineData(new object[] { Map, new object[] { Map, "x", 2 }, 42 }, "bfbf617802ff182aff")] // using maps as keys + public static void WriteMap_IndefiniteLength_NestedValues_HappyPath(object[] values, string expectedHexEncoding) + { + byte[] expectedEncoding = expectedHexEncoding.HexToByteArray(); + using var writer = new CborWriter(); + Helpers.WriteMap(writer, values, useDefiniteLengthCollections: false); + byte[] actualEncoding = writer.ToArray(); + AssertHelper.HexEqual(expectedEncoding, actualEncoding); + } + [Theory] [InlineData(new object[] { Map, "a", 1, "b", new object[] { 2, 3 } }, "a26161016162820203")] [InlineData(new object[] { Map, "a", new object[] { 2, 3, "b", new object[] { Map, "x", -1, "y", new object[] { "z", 0 } } } }, "a161618402036162a2617820617982617a00")] @@ -145,6 +172,26 @@ public static void EndWriteMap_DefiniteLengthNotMet_WithNestedData_ShouldThrowIn Assert.Throws(() => writer.WriteEndMap()); } + [Theory] + [InlineData(0)] + [InlineData(3)] + [InlineData(10)] + public static void EndWriteMap_IndefiniteLength_EvenItems_ShouldThrowInvalidOperationException(int length) + { + using var writer = new CborWriter(); + writer.WriteStartMap(); + + for (int i = 1; i < length; i++) + { + writer.WriteTextString($"key_{i}"); + writer.WriteInt64(i); + } + + writer.WriteInt64(0); + + Assert.Throws(() => writer.WriteEndMap()); + } + [Fact] public static void EndWriteMap_ImbalancedCall_ShouldThrowInvalidOperationException() { diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs index d5f5b219f5e21..b406bf7f631ab 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs @@ -4,6 +4,7 @@ #nullable enable using System; +using System.Linq; using Test.Cryptography; using Xunit; @@ -26,6 +27,21 @@ public static void WriteByteString_SingleValue_HappyPath(string hexInput, string AssertHelper.HexEqual(expectedEncoding, writer.ToArray()); } + [Theory] + [InlineData(new string[] { }, "5fff")] + [InlineData(new string[] { "" }, "5f40ff")] + [InlineData(new string[] { "ab", "" }, "5f41ab40ff")] + [InlineData(new string[] { "ab", "bc", "" }, "5f41ab41bc40ff")] + public static void WriteByteString_IndefiteLength_SingleValue_HappyPath(string[] hexChunkInputs, string hexExpectedEncoding) + { + byte[][] chunkInputs = hexChunkInputs.Select(ch => ch.HexToByteArray()).ToArray(); + byte[] expectedEncoding = hexExpectedEncoding.HexToByteArray(); + + using var writer = new CborWriter(); + Helpers.WriteChunkedByteString(writer, chunkInputs); + AssertHelper.HexEqual(expectedEncoding, writer.ToArray()); + } + [Theory] [InlineData("", "60")] [InlineData("a", "6161")] @@ -42,6 +58,19 @@ public static void WriteTextString_SingleValue_HappyPath(string input, string he AssertHelper.HexEqual(expectedEncoding, writer.ToArray()); } + [Theory] + [InlineData(new string[] { }, "7fff")] + [InlineData(new string[] { "" }, "7f60ff")] + [InlineData(new string[] { "ab", "" }, "7f62616260ff")] + [InlineData(new string[] { "ab", "bc", "" }, "7f62616262626360ff")] + public static void WriteTextString_IndefiteLength_SingleValue_HappyPath(string[] chunkInputs, string hexExpectedEncoding) + { + byte[] expectedEncoding = hexExpectedEncoding.HexToByteArray(); + using var writer = new CborWriter(); + Helpers.WriteChunkedTextString(writer, chunkInputs); + AssertHelper.HexEqual(expectedEncoding, writer.ToArray()); + } + [Fact] public static void WriteTextString_InvalidUnicodeString_ShouldThrowEncoderFallbackException() { @@ -50,5 +79,58 @@ public static void WriteTextString_InvalidUnicodeString_ShouldThrowEncoderFallba using var writer = new CborWriter(); Assert.Throws(() => writer.WriteTextString(invalidUnicodeString)); } + + [Theory] + [InlineData(nameof(CborWriter.WriteInt64))] + [InlineData(nameof(CborWriter.WriteByteString))] + [InlineData(nameof(CborWriter.WriteStartTextString))] + [InlineData(nameof(CborWriter.WriteStartByteString))] + [InlineData(nameof(CborWriter.WriteStartArray))] + [InlineData(nameof(CborWriter.WriteStartMap))] + public static void WriteTextString_IndefiteLength_NestedWrites_ShouldThrowInvalidOperationException(string opName) + { + using var writer = new CborWriter(); + writer.WriteStartTextString(); + Assert.Throws(() => Helpers.ExecOperation(writer, opName)); + } + + [Theory] + [InlineData(nameof(CborWriter.WriteEndByteString))] + [InlineData(nameof(CborWriter.WriteEndArray))] + [InlineData(nameof(CborWriter.WriteEndMap))] + public static void WriteTextString_IndefiteLength_ImbalancedWrites_ShouldThrowInvalidOperationException(string opName) + { + using var writer = new CborWriter(); + writer.WriteStartTextString(); + Assert.Throws(() => Helpers.ExecOperation(writer, opName)); + } + + [Theory] + [InlineData(nameof(CborWriter.WriteInt64))] + [InlineData(nameof(CborWriter.WriteTextString))] + [InlineData(nameof(CborWriter.WriteStartTextString))] + [InlineData(nameof(CborWriter.WriteStartByteString))] + [InlineData(nameof(CborWriter.WriteStartArray))] + [InlineData(nameof(CborWriter.WriteStartMap))] + [InlineData(nameof(CborWriter.WriteEndTextString))] + [InlineData(nameof(CborWriter.WriteEndArray))] + [InlineData(nameof(CborWriter.WriteEndMap))] + public static void WriteByteString_IndefiteLength_NestedWrites_ShouldThrowInvalidOperationException(string opName) + { + using var writer = new CborWriter(); + writer.WriteStartByteString(); + Assert.Throws(() => Helpers.ExecOperation(writer, opName)); + } + + [Theory] + [InlineData(nameof(CborWriter.WriteEndTextString))] + [InlineData(nameof(CborWriter.WriteEndArray))] + [InlineData(nameof(CborWriter.WriteEndMap))] + public static void WriteByteString_IndefiteLength_ImbalancedWrites_ShouldThrowInvalidOperationException(string opName) + { + using var writer = new CborWriter(); + writer.WriteStartByteString(); + Assert.Throws(() => Helpers.ExecOperation(writer, opName)); + } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborInitialByte.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborInitialByte.cs index ef82fb4907519..b2dd4621add61 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborInitialByte.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborInitialByte.cs @@ -30,7 +30,9 @@ internal enum CborAdditionalInfo : byte /// Represents the Cbor Data item initial byte structure internal readonly struct CborInitialByte { + public const byte IndefiniteLengthBreakByte = 0xff; private const byte AdditionalInformationMask = 0b000_11111; + public byte InitialByte { get; } internal CborInitialByte(CborMajorType majorType, CborAdditionalInfo additionalInfo) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Array.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Array.cs index 64676a183689c..0131401a29422 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Array.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Array.cs @@ -23,7 +23,22 @@ public void WriteStartArray(int definiteLength) public void WriteEndArray() { + if (!_remainingDataItems.HasValue) + { + // indefinite-length map, add break byte + EnsureWriteCapacity(1); + WriteInitialByte(new CborInitialByte(CborInitialByte.IndefiniteLengthBreakByte)); + } + PopDataItem(CborMajorType.Array); } + + public void WriteStartArray() + { + EnsureWriteCapacity(1); + WriteInitialByte(new CborInitialByte(CborMajorType.Array, CborAdditionalInfo.IndefiniteLength)); + DecrementRemainingItemCount(); + PushDataItem(CborMajorType.Array, expectedNestedItems: null); + } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Integer.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Integer.cs index f79cbd97be20d..75cd10d55a577 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Integer.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Integer.cs @@ -31,43 +31,40 @@ public void WriteInt64(long value) // Unsigned integer encoding https://tools.ietf.org/html/rfc7049#section-2.1 private void WriteUnsignedInteger(CborMajorType type, ulong value) { - EnsureCanWriteNewDataItem(); - if (value < 24) { EnsureWriteCapacity(1); - _buffer[_offset++] = new CborInitialByte(type, (CborAdditionalInfo)value).InitialByte; + WriteInitialByte(new CborInitialByte(type, (CborAdditionalInfo)value)); } else if (value <= byte.MaxValue) { EnsureWriteCapacity(2); - _buffer[_offset] = new CborInitialByte(type, CborAdditionalInfo.Unsigned8BitIntegerEncoding).InitialByte; - _buffer[_offset + 1] = (byte)value; - _offset += 2; + WriteInitialByte(new CborInitialByte(type, CborAdditionalInfo.Unsigned8BitIntegerEncoding)); + _buffer[_offset++] = (byte)value; } else if (value <= ushort.MaxValue) { EnsureWriteCapacity(3); - _buffer[_offset] = new CborInitialByte(type, CborAdditionalInfo.Unsigned16BitIntegerEncoding).InitialByte; - BinaryPrimitives.WriteUInt16BigEndian(_buffer.AsSpan(_offset + 1), (ushort)value); - _offset += 3; + WriteInitialByte(new CborInitialByte(type, CborAdditionalInfo.Unsigned16BitIntegerEncoding)); + BinaryPrimitives.WriteUInt16BigEndian(_buffer.AsSpan(_offset), (ushort)value); + _offset += 2; } else if (value <= uint.MaxValue) { EnsureWriteCapacity(5); - _buffer[_offset] = new CborInitialByte(type, CborAdditionalInfo.Unsigned32BitIntegerEncoding).InitialByte; - BinaryPrimitives.WriteUInt32BigEndian(_buffer.AsSpan(_offset + 1), (uint)value); - _offset += 5; + WriteInitialByte(new CborInitialByte(type, CborAdditionalInfo.Unsigned32BitIntegerEncoding)); + BinaryPrimitives.WriteUInt32BigEndian(_buffer.AsSpan(_offset), (uint)value); + _offset += 4; } else { EnsureWriteCapacity(9); - _buffer[_offset] = new CborInitialByte(type, CborAdditionalInfo.Unsigned64BitIntegerEncoding).InitialByte; - BinaryPrimitives.WriteUInt64BigEndian(_buffer.AsSpan(_offset + 1), value); - _offset += 9; + WriteInitialByte(new CborInitialByte(type, CborAdditionalInfo.Unsigned64BitIntegerEncoding)); + BinaryPrimitives.WriteUInt64BigEndian(_buffer.AsSpan(_offset), value); + _offset += 8; } - _remainingDataItems--; + DecrementRemainingItemCount(); } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs index 6304c7af19c0f..58a1bacf739be 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs @@ -22,7 +22,22 @@ public void WriteStartMap(int definiteLength) public void WriteEndMap() { + if (!_remainingDataItems.HasValue) + { + // indefinite-length map, add break byte + EnsureWriteCapacity(1); + WriteInitialByte(new CborInitialByte(CborInitialByte.IndefiniteLengthBreakByte)); + } + PopDataItem(CborMajorType.Map); } + + public void WriteStartMap() + { + EnsureWriteCapacity(1); + WriteInitialByte(new CborInitialByte(CborMajorType.Map, CborAdditionalInfo.IndefiniteLength)); + DecrementRemainingItemCount(); + PushDataItem(CborMajorType.Map, expectedNestedItems: null); + } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs index 6963176fc2cfa..ffdd808aa0307 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs @@ -30,5 +30,35 @@ public void WriteTextString(ReadOnlySpan value) s_utf8Encoding.GetBytes(value, _buffer.AsSpan(_offset)); _offset += length; } + + public void WriteStartByteString() + { + EnsureWriteCapacity(1); + WriteInitialByte(new CborInitialByte(CborMajorType.ByteString, CborAdditionalInfo.IndefiniteLength)); + DecrementRemainingItemCount(); + PushDataItem(CborMajorType.ByteString, expectedNestedItems: null); + } + + public void WriteEndByteString() + { + EnsureWriteCapacity(1); + WriteInitialByte(new CborInitialByte(CborInitialByte.IndefiniteLengthBreakByte)); + PopDataItem(CborMajorType.ByteString); + } + + public void WriteStartTextString() + { + EnsureWriteCapacity(1); + WriteInitialByte(new CborInitialByte(CborMajorType.TextString, CborAdditionalInfo.IndefiniteLength)); + DecrementRemainingItemCount(); + PushDataItem(CborMajorType.TextString, expectedNestedItems: null); + } + + public void WriteEndTextString() + { + EnsureWriteCapacity(1); + WriteInitialByte(new CborInitialByte(CborInitialByte.IndefiniteLengthBreakByte)); + PopDataItem(CborMajorType.TextString); + } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.cs index 339a8860bd5dc..075819261c916 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.cs @@ -21,7 +21,8 @@ internal partial class CborWriter : IDisposable // with null representing indefinite length data items. // The root context ony permits one data item to be written. private uint? _remainingDataItems = 1; - private Stack<(CborMajorType type, uint? remainingDataItems)>? _nestedDataItemStack; + private bool _isEvenNumberOfDataItemsWritten = true; // required for indefinite-length map writes + private Stack<(CborMajorType type, bool isEventNumberOfDataItemsWritten, uint? remainingDataItems)>? _nestedDataItemStack; public CborWriter() { @@ -58,26 +59,31 @@ private void EnsureWriteCapacity(int pendingCount) } } - private void EnsureCanWriteNewDataItem() - { - if (_remainingDataItems == 0) - { - throw new InvalidOperationException("Adding a CBOR data item to the current context exceeds its definite length."); - } - } - private void PushDataItem(CborMajorType type, uint? expectedNestedItems) { - _nestedDataItemStack ??= new Stack<(CborMajorType, uint?)>(); - _nestedDataItemStack.Push((type, _remainingDataItems)); + _nestedDataItemStack ??= new Stack<(CborMajorType, bool, uint?)>(); + _nestedDataItemStack.Push((type, _isEvenNumberOfDataItemsWritten, _remainingDataItems)); _remainingDataItems = expectedNestedItems; + _isEvenNumberOfDataItemsWritten = true; } private void PopDataItem(CborMajorType expectedType) { - if (_remainingDataItems == null) + if (_nestedDataItemStack is null || _nestedDataItemStack.Count == 0) + { + throw new InvalidOperationException("No active CBOR nested data item to pop"); + } + + (CborMajorType actualType, bool isEvenNumberOfDataItemsWritten, uint? remainingItems) = _nestedDataItemStack.Peek(); + + if (expectedType != actualType) + { + throw new InvalidOperationException("Unexpected major type in nested CBOR data item."); + } + + if (expectedType == CborMajorType.Map && !_isEvenNumberOfDataItemsWritten) { - throw new NotImplementedException("Indefinite-length data items"); + throw new InvalidOperationException("CBOR Map types require an even number of key/value combinations"); } if (_remainingDataItems > 0) @@ -85,20 +91,52 @@ private void PopDataItem(CborMajorType expectedType) throw new InvalidOperationException("Definite-length nested CBOR data item is incomplete."); } - if (_nestedDataItemStack is null || _nestedDataItemStack.Count == 0) + _nestedDataItemStack.Pop(); + _remainingDataItems = remainingItems; + _isEvenNumberOfDataItemsWritten = isEvenNumberOfDataItemsWritten; + } + + private void DecrementRemainingItemCount() + { + _remainingDataItems--; + _isEvenNumberOfDataItemsWritten = !_isEvenNumberOfDataItemsWritten; + } + + private void WriteInitialByte(CborInitialByte initialByte) + { + if (_remainingDataItems == 0) { - throw new InvalidOperationException("No active CBOR nested data item to pop"); + throw new InvalidOperationException("Adding a CBOR data item to the current context exceeds its definite length."); } - (CborMajorType actualType, uint? remainingItems) = _nestedDataItemStack.Peek(); + if (_remainingDataItems.HasValue && initialByte.InitialByte == CborInitialByte.IndefiniteLengthBreakByte) + { + throw new InvalidOperationException("Cannot write CBOR break byte in definite-length contexts"); + } - if (expectedType != actualType) + // TODO check for tag state + + if (_nestedDataItemStack != null && _nestedDataItemStack.Count > 0) { - throw new InvalidOperationException("Unexpected major type in nested CBOR data item."); + CborMajorType parentType = _nestedDataItemStack.Peek().type; + + switch (parentType) + { + // indefinite-length string contexts do not permit nesting + case CborMajorType.ByteString: + case CborMajorType.TextString: + if (initialByte.InitialByte == CborInitialByte.IndefiniteLengthBreakByte || + initialByte.MajorType == parentType && + initialByte.AdditionalInfo != CborAdditionalInfo.IndefiniteLength) + { + break; + } + + throw new InvalidOperationException("Cannot nest data items in indefinite-length CBOR string contexts."); + } } - _nestedDataItemStack.Pop(); - _remainingDataItems = remainingItems; + _buffer[_offset++] = initialByte.InitialByte; } private void CheckDisposed() From c77f4ad155ecddf2069089919b57ba1918f42454 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Sun, 22 Mar 2020 00:48:42 +0000 Subject: [PATCH 02/12] add indefinite-length cbor reader support --- .../tests/Cbor.Tests/CborReaderTests.Array.cs | 49 +++++++++ .../Cbor.Tests/CborReaderTests.Helpers.cs | 61 +++++++++-- .../Cbor.Tests/CborReaderTests.Integer.cs | 4 +- .../tests/Cbor.Tests/CborReaderTests.Map.cs | 66 ++++++++++++ .../Cbor.Tests/CborReaderTests.String.cs | 50 +++++++++ .../Cbor.Tests/CborWriterTests.String.cs | 6 +- .../tests/Cbor/CborReader.Array.cs | 37 +++++-- .../tests/Cbor/CborReader.Integer.cs | 11 +- .../tests/Cbor/CborReader.Map.cs | 47 ++++++-- .../tests/Cbor/CborReader.String.cs | 52 ++++++++- .../tests/Cbor/CborReader.cs | 100 ++++++++++++++---- .../tests/Cbor/CborWriter.Map.cs | 5 + .../tests/Cbor/CborWriter.cs | 7 +- 13 files changed, 428 insertions(+), 67 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Array.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Array.cs index 22a27e4cf9f13..e836967ab8827 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Array.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Array.cs @@ -41,6 +41,21 @@ public static void ReadArray_NestedValues_HappyPath(object[] expectedValues, str Assert.Equal(CborReaderState.Finished, reader.Peek()); } + [Theory] + [InlineData(new object[] { }, "9fff")] + [InlineData(new object[] { 42 }, "9f182aff")] + [InlineData(new object[] { 1, 2, 3 }, "9f010203ff")] + [InlineData(new object[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25 }, "9f0102030405060708090a0b0c0d0e0f101112131415161718181819ff")] + [InlineData(new object[] { 1, -1, "", new byte[] { 7 } }, "9f0120604107ff")] + [InlineData(new object[] { "lorem", "ipsum", "dolor" }, "9f656c6f72656d65697073756d65646f6c6f72ff")] + public static void ReadArray_IndefiniteLength_HappyPath(object[] expectedValues, string hexEncoding) + { + byte[] encoding = hexEncoding.HexToByteArray(); + var reader = new CborReader(encoding); + Helpers.VerifyArray(reader, expectedValues, expectDefiniteLengthCollections: false); + Assert.Equal(CborReaderState.Finished, reader.Peek()); + } + [Theory] [InlineData("80", 0)] [InlineData("8101", 1)] @@ -83,6 +98,40 @@ public static void ReadArray_DefiniteLengthExceeded_WithNestedData_ShouldThrowIn Assert.Throws(() => reader.ReadInt64()); } + [Theory] + [InlineData("9f")] + [InlineData("9f01")] + [InlineData("9f0102")] + public static void ReadArray_IndefiniteLength_MissingBreakByte_ShouldReportEndOfData(string hexEncoding) + { + byte[] encoding = hexEncoding.HexToByteArray(); + var reader = new CborReader(encoding); + reader.ReadStartArray(); + while (reader.Peek() == CborReaderState.UnsignedInteger) + { + reader.ReadInt64(); + } + + Assert.Equal(CborReaderState.EndOfData, reader.Peek()); + } + + [Theory] + [InlineData("9f01ff", 1)] + [InlineData("9f0102ff", 2)] + [InlineData("9f010203ff", 3)] + public static void ReadArray_IndefiniteLength_PrematureEndArrayCall_ShouldThrowInvalidOperationException(string hexEncoding, int length) + { + byte[] encoding = hexEncoding.HexToByteArray(); + var reader = new CborReader(encoding); + reader.ReadStartArray(); + for (int i = 1; i < length; i++) + { + reader.ReadInt64(); + } + + Assert.Throws(() => reader.ReadEndArray()); + } + [Theory] [InlineData("8101", 1)] [InlineData("83010203", 3)] diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs index 6eba42bdb8317..d079d30734cc2 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs @@ -4,6 +4,7 @@ #nullable enable using System.Linq; +using Test.Cryptography; using Xunit; namespace System.Security.Cryptography.Encoding.Tests.Cbor @@ -12,7 +13,7 @@ public partial class CborReaderTests { internal static class Helpers { - public static void VerifyValue(CborReader reader, object expectedValue) + public static void VerifyValue(CborReader reader, object expectedValue, bool expectDefiniteLengthCollections = true) { switch (expectedValue) { @@ -39,13 +40,38 @@ public static void VerifyValue(CborReader reader, object expectedValue) case byte[] expected: Assert.Equal(CborReaderState.ByteString, reader.Peek()); byte[] b = reader.ReadByteString(); - Assert.Equal(expected, b); + Assert.Equal(expected.ByteArrayToHex(), b.ByteArrayToHex()); break; + case string[] expectedChunks: + Assert.Equal(CborReaderState.StartTextString, reader.Peek()); + reader.ReadStartTextString(); + foreach(string expectedChunk in expectedChunks) + { + Assert.Equal(CborReaderState.TextString, reader.Peek()); + string chunk = reader.ReadTextString(); + Assert.Equal(expectedChunk, chunk); + } + Assert.Equal(CborReaderState.EndTextString, reader.Peek()); + reader.ReadEndTextString(); + break; + case byte[][] expectedChunks: + Assert.Equal(CborReaderState.StartByteString, reader.Peek()); + reader.ReadStartByteString(); + foreach (byte[] expectedChunk in expectedChunks) + { + Assert.Equal(CborReaderState.ByteString, reader.Peek()); + byte[] chunk = reader.ReadByteString(); + Assert.Equal(expectedChunk.ByteArrayToHex(), chunk.ByteArrayToHex()); + } + Assert.Equal(CborReaderState.EndByteString, reader.Peek()); + reader.ReadEndByteString(); + break; + case object[] nested when CborWriterTests.Helpers.IsCborMapRepresentation(nested): - VerifyMap(reader, nested); + VerifyMap(reader, nested, expectDefiniteLengthCollections); break; case object[] nested: - VerifyArray(reader, nested); + VerifyArray(reader, nested, expectDefiniteLengthCollections); break; default: throw new ArgumentException($"Unrecognized argument type {expectedValue.GetType()}"); @@ -58,14 +84,21 @@ static void VerifyPeekInteger(CborReader reader, bool isUnsignedInteger) } } - public static void VerifyArray(CborReader reader, params object[] expectedValues) + public static void VerifyArray(CborReader reader, object[] expectedValues, bool expectDefiniteLengthCollections = true) { Assert.Equal(CborReaderState.StartArray, reader.Peek()); ulong? length = reader.ReadStartArray(); - Assert.NotNull(length); - Assert.Equal(expectedValues.Length, (int)length!.Value); + if (expectDefiniteLengthCollections) + { + Assert.NotNull(length); + Assert.Equal(expectedValues.Length, (int)length!.Value); + } + else + { + Assert.Null(length); + } foreach (object value in expectedValues) { @@ -76,7 +109,7 @@ public static void VerifyArray(CborReader reader, params object[] expectedValues reader.ReadEndArray(); } - public static void VerifyMap(CborReader reader, params object[] expectedValues) + public static void VerifyMap(CborReader reader, object[] expectedValues, bool expectDefiniteLengthCollections = true) { if (!CborWriterTests.Helpers.IsCborMapRepresentation(expectedValues)) { @@ -84,10 +117,18 @@ public static void VerifyMap(CborReader reader, params object[] expectedValues) } Assert.Equal(CborReaderState.StartMap, reader.Peek()); + ulong? length = reader.ReadStartMap(); - Assert.NotNull(length); - Assert.Equal((expectedValues.Length - 1) / 2, (int)length!.Value); + if (expectDefiniteLengthCollections) + { + Assert.NotNull(length); + Assert.Equal((expectedValues.Length - 1) / 2, (int)length!.Value); + } + else + { + Assert.Null(length); + } foreach (object value in expectedValues.Skip(1)) { diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Integer.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Integer.cs index 9c42ec6fa1e48..62a04378d245d 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Integer.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Integer.cs @@ -256,12 +256,12 @@ public static void ReadCborNegativeIntegerEncoding_InvalidData_ShouldThrowFormat [Theory] [InlineData("1f")] [InlineData("3f")] - public static void ReadInt64_IndefiniteLengthIntegers_ShouldThrowNotImplementedException(string hexEncoding) + public static void ReadInt64_IndefiniteLengthIntegers_ShouldThrowFormatException(string hexEncoding) { byte[] data = hexEncoding.HexToByteArray(); var reader = new CborReader(data); - Assert.Throws(() => reader.ReadInt64()); + Assert.Throws(() => reader.ReadInt64()); } [Fact] diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Map.cs index c443f354dfdd2..eba4d740ef5fc 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Map.cs @@ -54,6 +54,19 @@ public static void ReadMap_NestedListValues_HappyPath(object expectedValue, stri Assert.Equal(CborReaderState.Finished, reader.Peek()); } + [Theory] + [InlineData(new object[] { Map }, "bfff")] + [InlineData(new object[] { Map, 1, 2, 3, 4 }, "bf01020304ff")] + [InlineData(new object[] { Map, "a", "A", "b", "B", "c", "C", "d", "D", "e", "E" }, "bf6161614161626142616361436164614461656145ff")] + [InlineData(new object[] { Map, "a", "A", -1, 2, new byte[] { }, new byte[] { 1 } }, "bf616161412002404101ff")] + public static void ReadMap_IndefiniteLength_SimpleValues_HappyPath(object[] exoectedValues, string hexEncoding) + { + byte[] encoding = hexEncoding.HexToByteArray(); + var reader = new CborReader(encoding); + Helpers.VerifyMap(reader, exoectedValues, expectDefiniteLengthCollections: false); + Assert.Equal(CborReaderState.Finished, reader.Peek()); + } + [Theory] [InlineData(new object[] { Map, "a", 1, "a", 2 }, "a2616101616102")] @@ -193,6 +206,59 @@ public static void ReadMap_IncorrectDefiniteLength_ShouldThrowFormatException(st Assert.Throws(() => reader.ReadInt64()); } + [Theory] + [InlineData("bf")] + [InlineData("bf0102")] + [InlineData("bf01020304")] + public static void ReadMap_IndefiniteLength_MissingBreakByte_ShouldReportEndOfData(string hexEncoding) + { + byte[] encoding = hexEncoding.HexToByteArray(); + var reader = new CborReader(encoding); + reader.ReadStartMap(); + while (reader.Peek() == CborReaderState.UnsignedInteger) + { + reader.ReadInt64(); + } + + Assert.Equal(CborReaderState.EndOfData, reader.Peek()); + } + + [Theory] + [InlineData("bf0102ff", 1)] + [InlineData("bf01020304ff", 2)] + [InlineData("bf010203040506ff", 3)] + public static void ReadMap_IndefiniteLength_PrematureEndArrayCall_ShouldThrowInvalidOperationException(string hexEncoding, int length) + { + byte[] encoding = hexEncoding.HexToByteArray(); + var reader = new CborReader(encoding); + reader.ReadStartMap(); + for (int i = 1; i < length; i++) + { + reader.ReadInt64(); + } + + Assert.Equal(CborReaderState.UnsignedInteger, reader.Peek()); + Assert.Throws(() => reader.ReadEndMap()); + } + + [Theory] + [InlineData("bf01ff", 1)] + [InlineData("bf010203ff", 3)] + [InlineData("bf0102030405ff", 5)] + public static void ReadMap_IndefiniteLength_OddKeyValuePairs_ShouldThrowFormatException(string hexEncoding, int length) + { + byte[] encoding = hexEncoding.HexToByteArray(); + var reader = new CborReader(encoding); + reader.ReadStartMap(); + for (int i = 0; i < length; i++) + { + reader.ReadInt64(); + } + + Assert.Equal(CborReaderState.EndMap, reader.Peek()); // don't want this to fail + Assert.Throws(() => reader.ReadEndMap()); + } + [Theory] [InlineData("a201811907e4", 2, 1)] [InlineData("a61907e4811907e402811907e4", 6, 2)] diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs index 10c91c674da2f..fbadae36c174b 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs @@ -4,6 +4,7 @@ #nullable enable using System; +using System.Linq; using Test.Cryptography; using Xunit; @@ -83,6 +84,31 @@ public static void TryReadTextString_SingleValue_HappyPath(string expectedValue, Assert.Equal(CborReaderState.Finished, reader.Peek()); } + [Theory] + [InlineData(new string[] { }, "5fff")] + [InlineData(new string[] { "" }, "5f40ff")] + [InlineData(new string[] { "ab", "" }, "5f41ab40ff")] + [InlineData(new string[] { "ab", "bc", "" }, "5f41ab41bc40ff")] + public static void ReadByteString_IndefiteLength_SingleValue_HappyPath(string[] expectedHexValues, string hexEncoding) + { + byte[] data = hexEncoding.HexToByteArray(); + byte[][] expectedValues = expectedHexValues.Select(x => x.HexToByteArray()).ToArray(); + var reader = new CborReader(data); + Helpers.VerifyValue(reader, expectedValues); + } + + [Theory] + [InlineData(new string[] { }, "7fff")] + [InlineData(new string[] { "" }, "7f60ff")] + [InlineData(new string[] { "ab", "" }, "7f62616260ff")] + [InlineData(new string[] { "ab", "bc", "" }, "7f62616262626360ff")] + public static void ReadTextString_IndefiteLength_SingleValue_HappyPath(string[] expectedValues, string hexEncoding) + { + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + Helpers.VerifyValue(reader, expectedValues); + } + [Theory] [InlineData("01020304", "4401020304")] [InlineData("ffffffffffffffffffffffffffff", "4effffffffffffffffffffffffffff")] @@ -343,5 +369,29 @@ public static void ReadByteString_EmptyBuffer_ShouldThrowFormatException() Assert.Throws(() => reader.ReadByteString()); } + + [Fact] + public static void ReadByteString_IndefiteLength_ContainingInvalidMajorTypes_ShouldThrowFormatException() + { + string hexEncoding = "5f4001ff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + reader.ReadStartByteString(); + reader.ReadByteString(); + Assert.Equal(CborReaderState.UnsignedInteger, reader.Peek()); // peek should not fail + Assert.Throws(() => reader.ReadInt64()); // throws FormatException even if it's the right major type we're trying to read + } + + [Fact] + public static void ReadTextString_IndefiteLength_ContainingInvalidMajorTypes_ShouldThrowFormatException() + { + string hexEncoding = "7f6001ff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + reader.ReadStartTextString(); + reader.ReadTextString(); + Assert.Equal(CborReaderState.UnsignedInteger, reader.Peek()); // peek should not fail + Assert.Throws(() => reader.ReadInt64()); // throws FormatException even if it's the right major type we're trying to read + } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs index b406bf7f631ab..58dd2f3ba6a32 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs @@ -63,7 +63,7 @@ public static void WriteTextString_SingleValue_HappyPath(string input, string he [InlineData(new string[] { "" }, "7f60ff")] [InlineData(new string[] { "ab", "" }, "7f62616260ff")] [InlineData(new string[] { "ab", "bc", "" }, "7f62616262626360ff")] - public static void WriteTextString_IndefiteLength_SingleValue_HappyPath(string[] chunkInputs, string hexExpectedEncoding) + public static void WriteTextString_IndefiniteLength_SingleValue_HappyPath(string[] chunkInputs, string hexExpectedEncoding) { byte[] expectedEncoding = hexExpectedEncoding.HexToByteArray(); using var writer = new CborWriter(); @@ -87,7 +87,7 @@ public static void WriteTextString_InvalidUnicodeString_ShouldThrowEncoderFallba [InlineData(nameof(CborWriter.WriteStartByteString))] [InlineData(nameof(CborWriter.WriteStartArray))] [InlineData(nameof(CborWriter.WriteStartMap))] - public static void WriteTextString_IndefiteLength_NestedWrites_ShouldThrowInvalidOperationException(string opName) + public static void WriteTextString_IndefiniteLength_NestedWrites_ShouldThrowInvalidOperationException(string opName) { using var writer = new CborWriter(); writer.WriteStartTextString(); @@ -98,7 +98,7 @@ public static void WriteTextString_IndefiteLength_NestedWrites_ShouldThrowInvali [InlineData(nameof(CborWriter.WriteEndByteString))] [InlineData(nameof(CborWriter.WriteEndArray))] [InlineData(nameof(CborWriter.WriteEndMap))] - public static void WriteTextString_IndefiteLength_ImbalancedWrites_ShouldThrowInvalidOperationException(string opName) + public static void WriteTextString_IndefiniteLength_ImbalancedWrites_ShouldThrowInvalidOperationException(string opName) { using var writer = new CborWriter(); writer.WriteStartTextString(); diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Array.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Array.cs index 34f5c5b18a8dc..30bfd850e215c 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Array.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Array.cs @@ -12,17 +12,42 @@ internal partial class CborReader public ulong? ReadStartArray() { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.Array); - ulong arrayLength = checked((ulong)ReadUnsignedInteger(header, out int additionalBytes)); - AdvanceBuffer(1 + additionalBytes); - _remainingDataItems--; - PushDataItem(CborMajorType.Array, arrayLength); - return arrayLength; + if (header.AdditionalInfo == CborAdditionalInfo.IndefiniteLength) + { + AdvanceBuffer(1); + DecrementRemainingItemCount(); + PushDataItem(CborMajorType.Array, null); + return null; + } + else + { + ulong arrayLength = ReadUnsignedInteger(header, out int additionalBytes); + AdvanceBuffer(1 + additionalBytes); + DecrementRemainingItemCount(); + PushDataItem(CborMajorType.Array, arrayLength); + return arrayLength; + } } public void ReadEndArray() { - PopDataItem(expectedType: CborMajorType.Array); + if (_remainingDataItems == null) + { + CborInitialByte value = PeekInitialByte(); + + if (value.InitialByte != CborInitialByte.IndefiniteLengthBreakByte) + { + throw new InvalidOperationException("Not at end of indefinite-length array."); + } + + PopDataItem(expectedType: CborMajorType.Array); + AdvanceBuffer(1); + } + else + { + PopDataItem(expectedType: CborMajorType.Array); + } } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Integer.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Integer.cs index a3bb7b879fa24..da230f22bdeb4 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Integer.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Integer.cs @@ -18,7 +18,7 @@ public ulong ReadUInt64() case CborMajorType.UnsignedInteger: ulong value = ReadUnsignedInteger(header, out int additionalBytes); AdvanceBuffer(1 + additionalBytes); - _remainingDataItems--; + DecrementRemainingItemCount(); return value; case CborMajorType.NegativeInteger: @@ -42,13 +42,13 @@ public long ReadInt64() case CborMajorType.UnsignedInteger: value = checked((long)ReadUnsignedInteger(header, out additionalBytes)); AdvanceBuffer(1 + additionalBytes); - _remainingDataItems--; + DecrementRemainingItemCount(); return value; case CborMajorType.NegativeInteger: value = checked(-1 - (long)ReadUnsignedInteger(header, out additionalBytes)); AdvanceBuffer(1 + additionalBytes); - _remainingDataItems--; + DecrementRemainingItemCount(); return value; default: @@ -63,7 +63,7 @@ public ulong ReadCborNegativeIntegerEncoding() CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.NegativeInteger); ulong value = ReadUnsignedInteger(header, out int additionalBytes); AdvanceBuffer(1 + additionalBytes); - _remainingDataItems--; + DecrementRemainingItemCount(); return value; } @@ -98,9 +98,6 @@ private ulong ReadUnsignedInteger(CborInitialByte header, out int additionalByte additionalBytes = 8; return BinaryPrimitives.ReadUInt64BigEndian(buffer.Slice(1)); - case CborAdditionalInfo.IndefiniteLength: - throw new NotImplementedException("indefinite length support"); - default: throw new FormatException("initial byte contains invalid integer encoding data"); } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs index aaca54eb8b356..0d9673a404e56 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs @@ -12,22 +12,53 @@ internal partial class CborReader public ulong? ReadStartMap() { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.Map); - ulong arrayLength = checked((ulong)ReadUnsignedInteger(header, out int additionalBytes)); - AdvanceBuffer(1 + additionalBytes); - _remainingDataItems--; - if (arrayLength > long.MaxValue) + if (header.AdditionalInfo == CborAdditionalInfo.IndefiniteLength) { - throw new OverflowException("Read CBOR map field count exceeds supported size."); + AdvanceBuffer(1); + DecrementRemainingItemCount(); + PushDataItem(CborMajorType.Map, null); + return null; } + else + { + ulong mapSize = ReadUnsignedInteger(header, out int additionalBytes); + + if (mapSize > long.MaxValue) + { + throw new OverflowException("Read CBOR map field count exceeds supported size."); + } - PushDataItem(CborMajorType.Map, 2 * arrayLength); - return arrayLength; + AdvanceBuffer(1 + additionalBytes); + DecrementRemainingItemCount(); + PushDataItem(CborMajorType.Map, 2 * mapSize); + return mapSize; + } } public void ReadEndMap() { - PopDataItem(expectedType: CborMajorType.Map); + if (_remainingDataItems == null) + { + CborInitialByte value = PeekInitialByte(); + + if (value.InitialByte != CborInitialByte.IndefiniteLengthBreakByte) + { + throw new InvalidOperationException("Not at end of indefinite-length map."); + } + + if (!_isEvenNumberOfDataItemsWritten) + { + throw new FormatException("CBOR Map types require an even number of key/value combinations"); + } + + PopDataItem(expectedType: CborMajorType.Map); + AdvanceBuffer(1); + } + else + { + PopDataItem(expectedType: CborMajorType.Map); + } } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs index dd16a9a4e09fd..4e47d9e6fe950 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs @@ -19,7 +19,7 @@ public byte[] ReadByteString() byte[] result = new byte[length]; _buffer.Slice(1 + additionalBytes, length).CopyTo(result); AdvanceBuffer(1 + additionalBytes + length); - _remainingDataItems--; + DecrementRemainingItemCount(); return result; } @@ -37,7 +37,7 @@ public bool TryReadByteString(Span destination, out int bytesWritten) _buffer.Span.Slice(1 + additionalBytes, length).CopyTo(destination); AdvanceBuffer(1 + additionalBytes + length); - _remainingDataItems--; + DecrementRemainingItemCount(); bytesWritten = length; return true; @@ -52,7 +52,7 @@ public string ReadTextString() ReadOnlySpan encodedString = _buffer.Span.Slice(1 + additionalBytes, length); string result = s_utf8Encoding.GetString(encodedString); AdvanceBuffer(1 + additionalBytes + length); - _remainingDataItems--; + DecrementRemainingItemCount(); return result; } @@ -72,9 +72,53 @@ public bool TryReadTextString(Span destination, out int charsWritten) s_utf8Encoding.GetChars(encodedSlice, destination); AdvanceBuffer(1 + additionalBytes + byteLength); - _remainingDataItems--; + DecrementRemainingItemCount(); charsWritten = charLength; return true; } + + public void ReadStartTextString() + { + CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.TextString); + + if (header.AdditionalInfo != CborAdditionalInfo.IndefiniteLength) + { + throw new InvalidOperationException("CBOR text string is not of indefinite length."); + } + + DecrementRemainingItemCount(); + AdvanceBuffer(1); + + PushDataItem(CborMajorType.TextString, expectedNestedItems: null); + } + + public void ReadEndTextString() + { + ReadNextIndefiniteLengthBreakByte(); + PopDataItem(CborMajorType.TextString); + AdvanceBuffer(1); + } + + public void ReadStartByteString() + { + CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.ByteString); + + if (header.AdditionalInfo != CborAdditionalInfo.IndefiniteLength) + { + throw new InvalidOperationException("CBOR text string is not of indefinite length."); + } + + DecrementRemainingItemCount(); + AdvanceBuffer(1); + + PushDataItem(CborMajorType.ByteString, expectedNestedItems: null); + } + + public void ReadEndByteString() + { + ReadNextIndefiniteLengthBreakByte(); + PopDataItem(CborMajorType.ByteString); + AdvanceBuffer(1); + } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs index 83673c8124708..e2cb9bf272564 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs @@ -14,8 +14,12 @@ internal enum CborReaderState NegativeInteger, ByteString, TextString, + StartTextString, + StartByteString, StartArray, StartMap, + EndTextString, + EndByteString, EndArray, EndMap, Tag, @@ -33,7 +37,8 @@ internal partial class CborReader // with null representing indefinite length data items. // The root context ony permits one data item to be read. private ulong? _remainingDataItems = 1; - private Stack<(CborMajorType type, ulong? remainingDataItems)>? _nestedDataItemStack; + private bool _isEvenNumberOfDataItemsWritten = true; // required for indefinite-length map writes + private Stack<(CborMajorType type, bool isEvenNumberOfDataItemsWritten, ulong? remainingDataItems)>? _nestedDataItemStack; internal CborReader(ReadOnlyMemory buffer) { @@ -45,11 +50,6 @@ internal CborReader(ReadOnlyMemory buffer) public CborReaderState Peek() { - if (_remainingDataItems is null) - { - throw new NotImplementedException("indefinite length collections"); - } - if (_remainingDataItems == 0) { if (_nestedDataItemStack?.Count > 0) @@ -72,13 +72,34 @@ public CborReaderState Peek() return CborReaderState.EndOfData; } - CborInitialByte initialByte = new CborInitialByte(_buffer.Span[0]); + var initialByte = new CborInitialByte(_buffer.Span[0]); + + if (initialByte.InitialByte == CborInitialByte.IndefiniteLengthBreakByte) + { + if (_nestedDataItemStack?.Count > 0 && _remainingDataItems == null) + { + return _nestedDataItemStack.Peek().type switch + { + CborMajorType.ByteString => CborReaderState.EndByteString, + CborMajorType.TextString => CborReaderState.EndTextString, + CborMajorType.Array => CborReaderState.EndArray, + CborMajorType.Map => CborReaderState.EndMap, + _ => throw new Exception("CborReader internal error. Invalid CBOR major type pushed to stack."), + }; + } + else + { + throw new FormatException("Unexpected CBOR break byte."); + } + } return initialByte.MajorType switch { CborMajorType.UnsignedInteger => CborReaderState.UnsignedInteger, CborMajorType.NegativeInteger => CborReaderState.NegativeInteger, + CborMajorType.ByteString when initialByte.AdditionalInfo == CborAdditionalInfo.IndefiniteLength => CborReaderState.StartByteString, CborMajorType.ByteString => CborReaderState.ByteString, + CborMajorType.TextString when initialByte.AdditionalInfo == CborAdditionalInfo.IndefiniteLength => CborReaderState.StartTextString, CborMajorType.TextString => CborReaderState.TextString, CborMajorType.Array => CborReaderState.StartArray, CborMajorType.Map => CborReaderState.StartMap, @@ -100,7 +121,31 @@ private CborInitialByte PeekInitialByte() throw new FormatException("unexpected end of buffer."); } - return new CborInitialByte(_buffer.Span[0]); + var result = new CborInitialByte(_buffer.Span[0]); + + // TODO check for tag state + + if (_nestedDataItemStack != null && _nestedDataItemStack.Count > 0) + { + CborMajorType parentType = _nestedDataItemStack.Peek().type; + + switch (parentType) + { + // indefinite-length string contexts do not permit nesting + case CborMajorType.ByteString: + case CborMajorType.TextString: + if (result.InitialByte == CborInitialByte.IndefiniteLengthBreakByte || + result.MajorType == parentType && + result.AdditionalInfo != CborAdditionalInfo.IndefiniteLength) + { + break; + } + + throw new FormatException("Indefinite-length CBOR string containing invalid data item."); + } + } + + return result; } private CborInitialByte PeekInitialByte(CborMajorType expectedType) @@ -115,6 +160,16 @@ private CborInitialByte PeekInitialByte(CborMajorType expectedType) return result; } + private void ReadNextIndefiniteLengthBreakByte() + { + CborInitialByte result = PeekInitialByte(); + + if (result.InitialByte != CborInitialByte.IndefiniteLengthBreakByte) + { + throw new InvalidOperationException("Next data item is not indefinite-length break byte."); + } + } + private void PushDataItem(CborMajorType type, ulong? expectedNestedItems) { if (expectedNestedItems > (ulong)_buffer.Length) @@ -122,37 +177,40 @@ private void PushDataItem(CborMajorType type, ulong? expectedNestedItems) throw new FormatException("Insufficient buffer size for declared definite length in CBOR data item."); } - _nestedDataItemStack ??= new Stack<(CborMajorType, ulong?)>(); - _nestedDataItemStack.Push((type, _remainingDataItems)); + _nestedDataItemStack ??= new Stack<(CborMajorType, bool, ulong?)>(); + _nestedDataItemStack.Push((type, _isEvenNumberOfDataItemsWritten, _remainingDataItems)); _remainingDataItems = expectedNestedItems; + _isEvenNumberOfDataItemsWritten = true; } private void PopDataItem(CborMajorType expectedType) { - if (_remainingDataItems == null) - { - throw new NotImplementedException("Indefinite-length data items"); - } - - if (_remainingDataItems > 0) - { - throw new InvalidOperationException("Definite-length nested CBOR data item is incomplete."); - } - if (_nestedDataItemStack is null || _nestedDataItemStack.Count == 0) { throw new InvalidOperationException("No active CBOR nested data item to pop"); } - (CborMajorType actualType, ulong? remainingItems) = _nestedDataItemStack.Peek(); + (CborMajorType actualType, bool isEvenNumberOfDataItemsWritten, ulong? remainingItems) = _nestedDataItemStack.Peek(); if (expectedType != actualType) { throw new InvalidOperationException("Unexpected major type in nested CBOR data item."); } + if (_remainingDataItems > 0) + { + throw new InvalidOperationException("Definite-length nested CBOR data item is incomplete."); + } + _nestedDataItemStack.Pop(); _remainingDataItems = remainingItems; + _isEvenNumberOfDataItemsWritten = isEvenNumberOfDataItemsWritten; + } + + private void DecrementRemainingItemCount() + { + _remainingDataItems--; + _isEvenNumberOfDataItemsWritten = !_isEvenNumberOfDataItemsWritten; } private void AdvanceBuffer(int length) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs index 58a1bacf739be..f4eee312144c3 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs @@ -22,6 +22,11 @@ public void WriteStartMap(int definiteLength) public void WriteEndMap() { + if (!_isEvenNumberOfDataItemsWritten) + { + throw new InvalidOperationException("CBOR Map types require an even number of key/value combinations"); + } + if (!_remainingDataItems.HasValue) { // indefinite-length map, add break byte diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.cs index 075819261c916..316c10b92399a 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.cs @@ -22,7 +22,7 @@ internal partial class CborWriter : IDisposable // The root context ony permits one data item to be written. private uint? _remainingDataItems = 1; private bool _isEvenNumberOfDataItemsWritten = true; // required for indefinite-length map writes - private Stack<(CborMajorType type, bool isEventNumberOfDataItemsWritten, uint? remainingDataItems)>? _nestedDataItemStack; + private Stack<(CborMajorType type, bool isEvenNumberOfDataItemsWritten, uint? remainingDataItems)>? _nestedDataItemStack; public CborWriter() { @@ -81,11 +81,6 @@ private void PopDataItem(CborMajorType expectedType) throw new InvalidOperationException("Unexpected major type in nested CBOR data item."); } - if (expectedType == CborMajorType.Map && !_isEvenNumberOfDataItemsWritten) - { - throw new InvalidOperationException("CBOR Map types require an even number of key/value combinations"); - } - if (_remainingDataItems > 0) { throw new InvalidOperationException("Definite-length nested CBOR data item is incomplete."); From 77717cc4e5a3b55ba155c6060f9d06abd9920565 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 23 Mar 2020 16:17:54 +0000 Subject: [PATCH 03/12] Use CborReaderState.FormatError in Peek() instead of throwing exceptions. --- .../tests/Cbor.Tests/CborReaderTests.Map.cs | 2 +- .../tests/Cbor/CborReader.cs | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Map.cs index eba4d740ef5fc..8c90149c5c91f 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Map.cs @@ -255,7 +255,7 @@ public static void ReadMap_IndefiniteLength_OddKeyValuePairs_ShouldThrowFormatEx reader.ReadInt64(); } - Assert.Equal(CborReaderState.EndMap, reader.Peek()); // don't want this to fail + Assert.Equal(CborReaderState.FormatError, reader.Peek()); // don't want this to fail Assert.Throws(() => reader.ReadEndMap()); } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs index e2cb9bf272564..619ad7316248e 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs @@ -25,6 +25,7 @@ internal enum CborReaderState Tag, Special, Finished, + FormatError, EndOfData, } @@ -57,6 +58,7 @@ public CborReaderState Peek() return _nestedDataItemStack.Peek().type switch { CborMajorType.Array => CborReaderState.EndArray, + CborMajorType.Map when !_isEvenNumberOfDataItemsWritten => CborReaderState.FormatError, CborMajorType.Map => CborReaderState.EndMap, _ => throw new Exception("CborReader internal error. Invalid CBOR major type pushed to stack."), }; @@ -83,13 +85,14 @@ public CborReaderState Peek() CborMajorType.ByteString => CborReaderState.EndByteString, CborMajorType.TextString => CborReaderState.EndTextString, CborMajorType.Array => CborReaderState.EndArray, + CborMajorType.Map when !_isEvenNumberOfDataItemsWritten => CborReaderState.FormatError, CborMajorType.Map => CborReaderState.EndMap, _ => throw new Exception("CborReader internal error. Invalid CBOR major type pushed to stack."), }; } else { - throw new FormatException("Unexpected CBOR break byte."); + return CborReaderState.FormatError; } } @@ -105,7 +108,7 @@ public CborReaderState Peek() CborMajorType.Map => CborReaderState.StartMap, CborMajorType.Tag => CborReaderState.Tag, CborMajorType.Special => CborReaderState.Special, - _ => throw new FormatException("Invalid CBOR major type"), + _ => CborReaderState.FormatError, }; } From 3e9314932d4d61d079211b0fbe5f8a4bb72cf73b Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 23 Mar 2020 16:36:33 +0000 Subject: [PATCH 04/12] use verbose naming for indefinite-length write methods --- .../tests/Cbor.Tests/CborWriterTests.Helpers.cs | 16 ++++++++-------- .../tests/Cbor.Tests/CborWriterTests.Map.cs | 2 +- .../tests/Cbor.Tests/CborWriterTests.String.cs | 16 ++++++++-------- .../tests/Cbor/CborWriter.Array.cs | 2 +- .../tests/Cbor/CborWriter.Map.cs | 2 +- .../tests/Cbor/CborWriter.String.cs | 4 ++-- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs index 8ba20aef41e7e..36bb3370f44bf 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs @@ -47,7 +47,7 @@ public static void WriteArray(CborWriter writer, object[] values, bool useDefini } else { - writer.WriteStartArray(); + writer.WriteStartArrayIndefiniteLength(); } foreach (object value in values) @@ -71,7 +71,7 @@ public static void WriteMap(CborWriter writer, object[] keyValuePairs, bool useD } else { - writer.WriteStartMap(); + writer.WriteStartMapIndefiniteLength(); } foreach (object value in keyValuePairs.Skip(1)) @@ -84,7 +84,7 @@ public static void WriteMap(CborWriter writer, object[] keyValuePairs, bool useD public static void WriteChunkedByteString(CborWriter writer, byte[][] chunks) { - writer.WriteStartByteString(); + writer.WriteStartByteStringIndefiniteLength(); foreach (byte[] chunk in chunks) { writer.WriteByteString(chunk); @@ -94,7 +94,7 @@ public static void WriteChunkedByteString(CborWriter writer, byte[][] chunks) public static void WriteChunkedTextString(CborWriter writer, string[] chunks) { - writer.WriteStartTextString(); + writer.WriteStartTextStringIndefiniteLength(); foreach (string chunk in chunks) { writer.WriteTextString(chunk); @@ -109,10 +109,10 @@ public static void ExecOperation(CborWriter writer, string op) case nameof(writer.WriteInt64): writer.WriteInt64(42); break; case nameof(writer.WriteByteString): writer.WriteByteString(Array.Empty()); break; case nameof(writer.WriteTextString): writer.WriteTextString(""); break; - case nameof(writer.WriteStartTextString): writer.WriteStartTextString(); break; - case nameof(writer.WriteStartByteString): writer.WriteStartByteString(); break; - case nameof(writer.WriteStartArray): writer.WriteStartArray(); break; - case nameof(writer.WriteStartMap): writer.WriteStartMap(); break; + case nameof(writer.WriteStartTextStringIndefiniteLength): writer.WriteStartTextStringIndefiniteLength(); break; + case nameof(writer.WriteStartByteStringIndefiniteLength): writer.WriteStartByteStringIndefiniteLength(); break; + case nameof(writer.WriteStartArray): writer.WriteStartArrayIndefiniteLength(); break; + case nameof(writer.WriteStartMap): writer.WriteStartMapIndefiniteLength(); break; case nameof(writer.WriteEndByteString): writer.WriteEndByteString(); break; case nameof(writer.WriteEndTextString): writer.WriteEndTextString(); break; case nameof(writer.WriteEndArray): writer.WriteEndArray(); break; diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Map.cs index 18514ec646820..a23ddcb68984a 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Map.cs @@ -179,7 +179,7 @@ public static void EndWriteMap_DefiniteLengthNotMet_WithNestedData_ShouldThrowIn public static void EndWriteMap_IndefiniteLength_EvenItems_ShouldThrowInvalidOperationException(int length) { using var writer = new CborWriter(); - writer.WriteStartMap(); + writer.WriteStartMapIndefiniteLength(); for (int i = 1; i < length; i++) { diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs index 58dd2f3ba6a32..c36fe9e716d4c 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs @@ -83,14 +83,14 @@ public static void WriteTextString_InvalidUnicodeString_ShouldThrowEncoderFallba [Theory] [InlineData(nameof(CborWriter.WriteInt64))] [InlineData(nameof(CborWriter.WriteByteString))] - [InlineData(nameof(CborWriter.WriteStartTextString))] - [InlineData(nameof(CborWriter.WriteStartByteString))] + [InlineData(nameof(CborWriter.WriteStartTextStringIndefiniteLength))] + [InlineData(nameof(CborWriter.WriteStartByteStringIndefiniteLength))] [InlineData(nameof(CborWriter.WriteStartArray))] [InlineData(nameof(CborWriter.WriteStartMap))] public static void WriteTextString_IndefiniteLength_NestedWrites_ShouldThrowInvalidOperationException(string opName) { using var writer = new CborWriter(); - writer.WriteStartTextString(); + writer.WriteStartTextStringIndefiniteLength(); Assert.Throws(() => Helpers.ExecOperation(writer, opName)); } @@ -101,15 +101,15 @@ public static void WriteTextString_IndefiniteLength_NestedWrites_ShouldThrowInva public static void WriteTextString_IndefiniteLength_ImbalancedWrites_ShouldThrowInvalidOperationException(string opName) { using var writer = new CborWriter(); - writer.WriteStartTextString(); + writer.WriteStartTextStringIndefiniteLength(); Assert.Throws(() => Helpers.ExecOperation(writer, opName)); } [Theory] [InlineData(nameof(CborWriter.WriteInt64))] [InlineData(nameof(CborWriter.WriteTextString))] - [InlineData(nameof(CborWriter.WriteStartTextString))] - [InlineData(nameof(CborWriter.WriteStartByteString))] + [InlineData(nameof(CborWriter.WriteStartTextStringIndefiniteLength))] + [InlineData(nameof(CborWriter.WriteStartByteStringIndefiniteLength))] [InlineData(nameof(CborWriter.WriteStartArray))] [InlineData(nameof(CborWriter.WriteStartMap))] [InlineData(nameof(CborWriter.WriteEndTextString))] @@ -118,7 +118,7 @@ public static void WriteTextString_IndefiniteLength_ImbalancedWrites_ShouldThrow public static void WriteByteString_IndefiteLength_NestedWrites_ShouldThrowInvalidOperationException(string opName) { using var writer = new CborWriter(); - writer.WriteStartByteString(); + writer.WriteStartByteStringIndefiniteLength(); Assert.Throws(() => Helpers.ExecOperation(writer, opName)); } @@ -129,7 +129,7 @@ public static void WriteByteString_IndefiteLength_NestedWrites_ShouldThrowInvali public static void WriteByteString_IndefiteLength_ImbalancedWrites_ShouldThrowInvalidOperationException(string opName) { using var writer = new CborWriter(); - writer.WriteStartByteString(); + writer.WriteStartByteStringIndefiniteLength(); Assert.Throws(() => Helpers.ExecOperation(writer, opName)); } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Array.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Array.cs index 0131401a29422..97e7f4ff3a609 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Array.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Array.cs @@ -33,7 +33,7 @@ public void WriteEndArray() PopDataItem(CborMajorType.Array); } - public void WriteStartArray() + public void WriteStartArrayIndefiniteLength() { EnsureWriteCapacity(1); WriteInitialByte(new CborInitialByte(CborMajorType.Array, CborAdditionalInfo.IndefiniteLength)); diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs index f4eee312144c3..98147689d3546 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.Map.cs @@ -37,7 +37,7 @@ public void WriteEndMap() PopDataItem(CborMajorType.Map); } - public void WriteStartMap() + public void WriteStartMapIndefiniteLength() { EnsureWriteCapacity(1); WriteInitialByte(new CborInitialByte(CborMajorType.Map, CborAdditionalInfo.IndefiniteLength)); diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs index ffdd808aa0307..cb940705c9f2e 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs @@ -31,7 +31,7 @@ public void WriteTextString(ReadOnlySpan value) _offset += length; } - public void WriteStartByteString() + public void WriteStartByteStringIndefiniteLength() { EnsureWriteCapacity(1); WriteInitialByte(new CborInitialByte(CborMajorType.ByteString, CborAdditionalInfo.IndefiniteLength)); @@ -46,7 +46,7 @@ public void WriteEndByteString() PopDataItem(CborMajorType.ByteString); } - public void WriteStartTextString() + public void WriteStartTextStringIndefiniteLength() { EnsureWriteCapacity(1); WriteInitialByte(new CborInitialByte(CborMajorType.TextString, CborAdditionalInfo.IndefiniteLength)); From 993ab596c11e5be772f801b2836a2e7896967eb5 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 23 Mar 2020 16:37:50 +0000 Subject: [PATCH 05/12] address feedback --- .../tests/Cbor/CborReader.Map.cs | 2 +- .../tests/Cbor/CborReader.cs | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs index 0d9673a404e56..c80f41759e158 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs @@ -47,7 +47,7 @@ public void ReadEndMap() throw new InvalidOperationException("Not at end of indefinite-length map."); } - if (!_isEvenNumberOfDataItemsWritten) + if (!_isEvenNumberOfDataItemsRead) { throw new FormatException("CBOR Map types require an even number of key/value combinations"); } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs index 619ad7316248e..56c8c26d15582 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs @@ -38,7 +38,7 @@ internal partial class CborReader // with null representing indefinite length data items. // The root context ony permits one data item to be read. private ulong? _remainingDataItems = 1; - private bool _isEvenNumberOfDataItemsWritten = true; // required for indefinite-length map writes + private bool _isEvenNumberOfDataItemsRead = true; // required for indefinite-length map writes private Stack<(CborMajorType type, bool isEvenNumberOfDataItemsWritten, ulong? remainingDataItems)>? _nestedDataItemStack; internal CborReader(ReadOnlyMemory buffer) @@ -58,7 +58,6 @@ public CborReaderState Peek() return _nestedDataItemStack.Peek().type switch { CborMajorType.Array => CborReaderState.EndArray, - CborMajorType.Map when !_isEvenNumberOfDataItemsWritten => CborReaderState.FormatError, CborMajorType.Map => CborReaderState.EndMap, _ => throw new Exception("CborReader internal error. Invalid CBOR major type pushed to stack."), }; @@ -85,7 +84,7 @@ public CborReaderState Peek() CborMajorType.ByteString => CborReaderState.EndByteString, CborMajorType.TextString => CborReaderState.EndTextString, CborMajorType.Array => CborReaderState.EndArray, - CborMajorType.Map when !_isEvenNumberOfDataItemsWritten => CborReaderState.FormatError, + CborMajorType.Map when !_isEvenNumberOfDataItemsRead => CborReaderState.FormatError, CborMajorType.Map => CborReaderState.EndMap, _ => throw new Exception("CborReader internal error. Invalid CBOR major type pushed to stack."), }; @@ -181,9 +180,9 @@ private void PushDataItem(CborMajorType type, ulong? expectedNestedItems) } _nestedDataItemStack ??= new Stack<(CborMajorType, bool, ulong?)>(); - _nestedDataItemStack.Push((type, _isEvenNumberOfDataItemsWritten, _remainingDataItems)); + _nestedDataItemStack.Push((type, _isEvenNumberOfDataItemsRead, _remainingDataItems)); _remainingDataItems = expectedNestedItems; - _isEvenNumberOfDataItemsWritten = true; + _isEvenNumberOfDataItemsRead = true; } private void PopDataItem(CborMajorType expectedType) @@ -207,13 +206,13 @@ private void PopDataItem(CborMajorType expectedType) _nestedDataItemStack.Pop(); _remainingDataItems = remainingItems; - _isEvenNumberOfDataItemsWritten = isEvenNumberOfDataItemsWritten; + _isEvenNumberOfDataItemsRead = isEvenNumberOfDataItemsWritten; } private void DecrementRemainingItemCount() { _remainingDataItems--; - _isEvenNumberOfDataItemsWritten = !_isEvenNumberOfDataItemsWritten; + _isEvenNumberOfDataItemsRead = !_isEvenNumberOfDataItemsRead; } private void AdvanceBuffer(int length) From 4cd3e44b0b23230d3d42e8f313664d6c06a09894 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 24 Mar 2020 01:20:49 +0000 Subject: [PATCH 06/12] implement concatenation logic for indefinite-length string readers --- .../Cbor.Tests/CborReaderTests.String.cs | 153 ++++++++++++++- .../tests/Cbor/CborReader.Array.cs | 2 +- .../tests/Cbor/CborReader.Integer.cs | 20 +- .../tests/Cbor/CborReader.Map.cs | 2 +- .../tests/Cbor/CborReader.String.cs | 176 +++++++++++++++++- .../tests/Cbor/CborReader.cs | 34 +++- 6 files changed, 361 insertions(+), 26 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs index fbadae36c174b..060aa6159714e 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs @@ -5,6 +5,7 @@ #nullable enable using System; using System.Linq; +using System.Text; using Test.Cryptography; using Xunit; @@ -89,7 +90,7 @@ public static void TryReadTextString_SingleValue_HappyPath(string expectedValue, [InlineData(new string[] { "" }, "5f40ff")] [InlineData(new string[] { "ab", "" }, "5f41ab40ff")] [InlineData(new string[] { "ab", "bc", "" }, "5f41ab41bc40ff")] - public static void ReadByteString_IndefiteLength_SingleValue_HappyPath(string[] expectedHexValues, string hexEncoding) + public static void ReadByteString_IndefiniteLength_SingleValue_HappyPath(string[] expectedHexValues, string hexEncoding) { byte[] data = hexEncoding.HexToByteArray(); byte[][] expectedValues = expectedHexValues.Select(x => x.HexToByteArray()).ToArray(); @@ -97,18 +98,88 @@ public static void ReadByteString_IndefiteLength_SingleValue_HappyPath(string[] Helpers.VerifyValue(reader, expectedValues); } + [Theory] + [InlineData("", "5fff")] + [InlineData("", "5f40ff")] + [InlineData("ab", "5f41ab40ff")] + [InlineData("abbc", "5f41ab41bc40ff")] + public static void ReadByteString_IndefiniteLengthConcatenated_SingleValue_HappyPath(string expectedHexValue, string hexEncoding) + { + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + Assert.Equal(CborReaderState.StartByteString, reader.Peek()); + byte[] actualValue = reader.ReadByteString(); + Assert.Equal(expectedHexValue.ToUpper(), actualValue.ByteArrayToHex()); + Assert.Equal(CborReaderState.Finished, reader.Peek()); + } + + [Theory] + [InlineData("", "5fff")] + [InlineData("", "5f40ff")] + [InlineData("ab", "5f41ab40ff")] + [InlineData("abbc", "5f41ab41bc40ff")] + public static void TryReadByteString_IndefiniteLengthConcatenated_SingleValue_HappyPath(string expectedHexValue, string hexEncoding) + { + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + Assert.Equal(CborReaderState.StartByteString, reader.Peek()); + + Span buffer = new byte[32]; + bool result = reader.TryReadByteString(buffer, out int bytesWritten); + + Assert.True(result); + Assert.Equal(expectedHexValue.Length / 2, bytesWritten); + Assert.Equal(expectedHexValue.ToUpper(), buffer.Slice(0, bytesWritten).ByteArrayToHex()); + Assert.Equal(CborReaderState.Finished, reader.Peek()); + } + [Theory] [InlineData(new string[] { }, "7fff")] [InlineData(new string[] { "" }, "7f60ff")] [InlineData(new string[] { "ab", "" }, "7f62616260ff")] [InlineData(new string[] { "ab", "bc", "" }, "7f62616262626360ff")] - public static void ReadTextString_IndefiteLength_SingleValue_HappyPath(string[] expectedValues, string hexEncoding) + public static void ReadTextString_IndefiniteLength_SingleValue_HappyPath(string[] expectedValues, string hexEncoding) { byte[] data = hexEncoding.HexToByteArray(); var reader = new CborReader(data); Helpers.VerifyValue(reader, expectedValues); } + [Theory] + [InlineData("", "7fff")] + [InlineData("", "7f60ff")] + [InlineData("ab", "7f62616260ff")] + [InlineData("abbc", "7f62616262626360ff")] + public static void ReadTextString_IndefiniteLengthConcatenated_SingleValue_HappyPath(string expectedValue, string hexEncoding) + { + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + Assert.Equal(CborReaderState.StartTextString, reader.Peek()); + string actualValue = reader.ReadTextString(); + Assert.Equal(expectedValue, actualValue); + Assert.Equal(CborReaderState.Finished, reader.Peek()); + } + + [Theory] + [InlineData("", "7fff")] + [InlineData("", "7f60ff")] + [InlineData("ab", "7f62616260ff")] + [InlineData("abbc", "7f62616262626360ff")] + public static void TryReadTextString_IndefiniteLengthConcatenated_SingleValue__HappyPath(string expectedValue, string hexEncoding) + { + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + Assert.Equal(CborReaderState.StartTextString, reader.Peek()); + + Span buffer = new char[32]; + bool result = reader.TryReadTextString(buffer, out int charsWritten); + + Assert.True(result); + Assert.Equal(expectedValue.Length, charsWritten); + Assert.Equal(expectedValue, new string(buffer.Slice(0, charsWritten))); + Assert.Equal(CborReaderState.Finished, reader.Peek()); + } + [Theory] [InlineData("01020304", "4401020304")] [InlineData("ffffffffffffffffffffffffffff", "4effffffffffffffffffffffffffff")] @@ -142,6 +213,38 @@ public static void TryReadTextString_BufferTooSmall_ShouldReturnFalse(string act Assert.All(buffer, (b => Assert.Equal(0, '\0'))); } + [Theory] + [InlineData("ab", "5f41ab40ff")] + [InlineData("abbc", "5f41ab41bc40ff")] + public static void TryReadByteString_IndefiniteLengthConcatenated_BufferTooSmall_ShouldReturnFalse(string expectedHexValue, string hexEncoding) + { + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + + byte[] buffer = new byte[expectedHexValue.Length / 2 - 1]; + bool result = reader.TryReadByteString(buffer, out int bytesWritten); + + Assert.False(result); + Assert.Equal(0, bytesWritten); + Assert.All(buffer, (b => Assert.Equal(0, b))); + } + + [Theory] + [InlineData("ab", "7f62616260ff")] + [InlineData("abbc", "7f62616262626360ff")] + public static void TryReadTextString_IndefiniteLengthConcatenated_BufferTooSmall_ShouldReturnFalse(string expectedValue, string hexEncoding) + { + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + + char[] buffer = new char[expectedValue.Length - 1]; + bool result = reader.TryReadTextString(buffer, out int charsWritten); + + Assert.False(result); + Assert.Equal(0, charsWritten); + Assert.All(buffer, (b => Assert.Equal(0, '\0'))); + } + [Theory] [InlineData("00")] // 0 [InlineData("20")] // -1 @@ -371,27 +474,61 @@ public static void ReadByteString_EmptyBuffer_ShouldThrowFormatException() } [Fact] - public static void ReadByteString_IndefiteLength_ContainingInvalidMajorTypes_ShouldThrowFormatException() + public static void ReadByteString_IndefiniteLength_ContainingInvalidMajorTypes_ShouldThrowFormatException() { string hexEncoding = "5f4001ff"; byte[] data = hexEncoding.HexToByteArray(); var reader = new CborReader(data); reader.ReadStartByteString(); reader.ReadByteString(); - Assert.Equal(CborReaderState.UnsignedInteger, reader.Peek()); // peek should not fail - Assert.Throws(() => reader.ReadInt64()); // throws FormatException even if it's the right major type we're trying to read + + Assert.Equal(CborReaderState.FormatError, reader.Peek()); + // throws FormatException even if it's the right major type we're trying to read + Assert.Throws(() => reader.ReadInt64()); } [Fact] - public static void ReadTextString_IndefiteLength_ContainingInvalidMajorTypes_ShouldThrowFormatException() + public static void ReadTextString_IndefiniteLength_ContainingInvalidMajorTypes_ShouldThrowFormatException() { string hexEncoding = "7f6001ff"; byte[] data = hexEncoding.HexToByteArray(); var reader = new CborReader(data); reader.ReadStartTextString(); reader.ReadTextString(); - Assert.Equal(CborReaderState.UnsignedInteger, reader.Peek()); // peek should not fail - Assert.Throws(() => reader.ReadInt64()); // throws FormatException even if it's the right major type we're trying to read + + Assert.Equal(CborReaderState.FormatError, reader.Peek()); + // throws FormatException even if it's the right major type we're trying to read + Assert.Throws(() => reader.ReadInt64()); + } + + [Fact] + public static void ReadByteString_IndefiniteLengthConcatenated_ContainingInvalidMajorTypes_ShouldThrowFormatException() + { + string hexEncoding = "5f4001ff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + Assert.Throws(() => reader.ReadByteString()); + } + + [Fact] + public static void ReadTextString_IndefiniteLengthConcatenated_ContainingInvalidMajorTypes_ShouldThrowFormatException() + { + string hexEncoding = "7f6001ff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + Assert.Throws(() => reader.ReadTextString()); + } + + [Fact] + public static void ReadTextString_IndefiniteLengthConcatenated_InvalidUtf8Chunks_ShouldThrowDecoderFallbackException() + { + // while the concatenated string is valid utf8, the individual chunks are not, + // which is in violation of the CBOR format. + + string hexEncoding = "7f62f090628591ff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + Assert.Throws(() => reader.ReadTextString()); } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Array.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Array.cs index 30bfd850e215c..7ad19430a0388 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Array.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Array.cs @@ -22,7 +22,7 @@ internal partial class CborReader } else { - ulong arrayLength = ReadUnsignedInteger(header, out int additionalBytes); + ulong arrayLength = ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes); AdvanceBuffer(1 + additionalBytes); DecrementRemainingItemCount(); PushDataItem(CborMajorType.Array, arrayLength); diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Integer.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Integer.cs index da230f22bdeb4..ef9ebb185d5be 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Integer.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Integer.cs @@ -16,7 +16,7 @@ public ulong ReadUInt64() switch (header.MajorType) { case CborMajorType.UnsignedInteger: - ulong value = ReadUnsignedInteger(header, out int additionalBytes); + ulong value = ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes); AdvanceBuffer(1 + additionalBytes); DecrementRemainingItemCount(); return value; @@ -40,13 +40,13 @@ public long ReadInt64() switch (header.MajorType) { case CborMajorType.UnsignedInteger: - value = checked((long)ReadUnsignedInteger(header, out additionalBytes)); + value = checked((long)ReadUnsignedInteger(_buffer.Span, header, out additionalBytes)); AdvanceBuffer(1 + additionalBytes); DecrementRemainingItemCount(); return value; case CborMajorType.NegativeInteger: - value = checked(-1 - (long)ReadUnsignedInteger(header, out additionalBytes)); + value = checked(-1 - (long)ReadUnsignedInteger(_buffer.Span, header, out additionalBytes)); AdvanceBuffer(1 + additionalBytes); DecrementRemainingItemCount(); return value; @@ -61,17 +61,15 @@ public long ReadInt64() public ulong ReadCborNegativeIntegerEncoding() { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.NegativeInteger); - ulong value = ReadUnsignedInteger(header, out int additionalBytes); + ulong value = ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes); AdvanceBuffer(1 + additionalBytes); DecrementRemainingItemCount(); return value; } // Unsigned integer decoding https://tools.ietf.org/html/rfc7049#section-2.1 - private ulong ReadUnsignedInteger(CborInitialByte header, out int additionalBytes) + private static ulong ReadUnsignedInteger(ReadOnlySpan buffer, CborInitialByte header, out int additionalBytes) { - ReadOnlySpan buffer = _buffer.Span; - switch (header.AdditionalInfo) { case CborAdditionalInfo x when (x < CborAdditionalInfo.Unsigned8BitIntegerEncoding): @@ -79,22 +77,22 @@ private ulong ReadUnsignedInteger(CborInitialByte header, out int additionalByte return (ulong)x; case CborAdditionalInfo.Unsigned8BitIntegerEncoding: - EnsureBuffer(2); + EnsureBuffer(buffer, 2); additionalBytes = 1; return buffer[1]; case CborAdditionalInfo.Unsigned16BitIntegerEncoding: - EnsureBuffer(3); + EnsureBuffer(buffer, 3); additionalBytes = 2; return BinaryPrimitives.ReadUInt16BigEndian(buffer.Slice(1)); case CborAdditionalInfo.Unsigned32BitIntegerEncoding: - EnsureBuffer(5); + EnsureBuffer(buffer, 5); additionalBytes = 4; return BinaryPrimitives.ReadUInt32BigEndian(buffer.Slice(1)); case CborAdditionalInfo.Unsigned64BitIntegerEncoding: - EnsureBuffer(9); + EnsureBuffer(buffer, 9); additionalBytes = 8; return BinaryPrimitives.ReadUInt64BigEndian(buffer.Slice(1)); diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs index c80f41759e158..4c800055f8e88 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.Map.cs @@ -22,7 +22,7 @@ internal partial class CborReader } else { - ulong mapSize = ReadUnsignedInteger(header, out int additionalBytes); + ulong mapSize = ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes); if (mapSize > long.MaxValue) { diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs index 4e47d9e6fe950..d61a7501b4364 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs @@ -2,7 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable using System.Buffers.Binary; +using System.Collections.Generic; +using System.Diagnostics; +using System.Text; namespace System.Security.Cryptography.Encoding.Tests.Cbor { @@ -14,7 +18,13 @@ internal partial class CborReader public byte[] ReadByteString() { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.ByteString); - int length = checked((int)ReadUnsignedInteger(header, out int additionalBytes)); + + if (header.AdditionalInfo == CborAdditionalInfo.IndefiniteLength) + { + return ReadChunkedByteStringConcatenated(); + } + + int length = checked((int)ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes)); EnsureBuffer(1 + additionalBytes + length); byte[] result = new byte[length]; _buffer.Slice(1 + additionalBytes, length).CopyTo(result); @@ -26,7 +36,13 @@ public byte[] ReadByteString() public bool TryReadByteString(Span destination, out int bytesWritten) { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.ByteString); - int length = checked((int)ReadUnsignedInteger(header, out int additionalBytes)); + + if (header.AdditionalInfo == CborAdditionalInfo.IndefiniteLength) + { + return TryReadChunkedByteStringConcatenated(destination, out bytesWritten); + } + + int length = checked((int)ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes)); EnsureBuffer(1 + additionalBytes + length); if (length > destination.Length) @@ -47,7 +63,13 @@ public bool TryReadByteString(Span destination, out int bytesWritten) public string ReadTextString() { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.TextString); - int length = checked((int)ReadUnsignedInteger(header, out int additionalBytes)); + + if (header.AdditionalInfo == CborAdditionalInfo.IndefiniteLength) + { + return ReadChunkedTextStringConcatenated(); + } + + int length = checked((int)ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes)); EnsureBuffer(1 + additionalBytes + length); ReadOnlySpan encodedString = _buffer.Span.Slice(1 + additionalBytes, length); string result = s_utf8Encoding.GetString(encodedString); @@ -59,7 +81,13 @@ public string ReadTextString() public bool TryReadTextString(Span destination, out int charsWritten) { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.TextString); - int byteLength = checked((int)ReadUnsignedInteger(header, out int additionalBytes)); + + if (header.AdditionalInfo == CborAdditionalInfo.IndefiniteLength) + { + return TryReadChunkedTextStringConcatenated(destination, out charsWritten); + } + + int byteLength = checked((int)ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes)); EnsureBuffer(1 + additionalBytes + byteLength); ReadOnlySpan encodedSlice = _buffer.Span.Slice(1 + additionalBytes, byteLength); @@ -120,5 +148,145 @@ public void ReadEndByteString() PopDataItem(CborMajorType.ByteString); AdvanceBuffer(1); } + + private bool TryReadChunkedByteStringConcatenated(Span destination, out int bytesWritten) + { + List<(int offset, int length)> offsets = ReadChunkedStringRanges(CborMajorType.ByteString, out int offset, out int concatenatedBufferSize); + + if (concatenatedBufferSize > destination.Length) + { + bytesWritten = 0; + return false; + } + + ReadOnlySpan source = _buffer.Span; + + foreach ((int o, int l) in offsets) + { + source.Slice(o, l).CopyTo(destination); + destination = destination.Slice(l); + } + + bytesWritten = concatenatedBufferSize; + AdvanceBuffer(offset); + DecrementRemainingItemCount(); + return true; + } + + private bool TryReadChunkedTextStringConcatenated(Span destination, out int charsWritten) + { + List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.TextString, out int offset, out int _); + ReadOnlySpan buffer = _buffer.Span; + + int concatenatedStringSize = 0; + foreach ((int o, int l) in ranges) + { + concatenatedStringSize += s_utf8Encoding.GetCharCount(buffer.Slice(o, l)); + } + + if (concatenatedStringSize > destination.Length) + { + charsWritten = 0; + return false; + } + + foreach ((int o, int l) in ranges) + { + s_utf8Encoding.GetChars(buffer.Slice(o, l), destination); + destination = destination.Slice(l); + } + + charsWritten = concatenatedStringSize; + AdvanceBuffer(offset); + DecrementRemainingItemCount(); + return true; + } + + private byte[] ReadChunkedByteStringConcatenated() + { + List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.ByteString, out int offset, out int concatenatedBufferSize); + var output = new byte[concatenatedBufferSize]; + + ReadOnlySpan source = _buffer.Span; + Span target = output; + + foreach ((int o, int l) in ranges) + { + source.Slice(o, l).CopyTo(target); + target = target.Slice(l); + } + + Debug.Assert(target.IsEmpty); + AdvanceBuffer(offset); + DecrementRemainingItemCount(); + return output; + } + + private string ReadChunkedTextStringConcatenated() + { + List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.TextString, out int offset, out int concatenatedBufferSize); + ReadOnlySpan buffer = _buffer.Span; + int concatenatedStringSize = 0; + + foreach ((int o, int l) in ranges) + { + concatenatedStringSize += s_utf8Encoding.GetCharCount(buffer.Slice(o, l)); + } + + var output = new char[concatenatedStringSize]; + Span target = output; + + foreach ((int o, int l) in ranges) + { + s_utf8Encoding.GetChars(buffer.Slice(o, l), target); + target = target.Slice(l); + } + + Debug.Assert(target.IsEmpty); + AdvanceBuffer(offset); + DecrementRemainingItemCount(); + return new string(output); + } + + // reads a buffer starting with an indefinite-length string, + // performing validation and returning a list of ranges containing the individual chunk payloads + private List<(int offset, int length)> ReadChunkedStringRanges(CborMajorType type, out int encodingLength, out int concatenatedBufferSize) + { + var ranges = new List<(int offset, int length)>(); + ReadOnlySpan buffer = _buffer.Span; + concatenatedBufferSize = 0; + + int i = 1; // skip the indefinite-length initial byte + CborInitialByte nextInitialByte = ReadNextInitialByte(buffer.Slice(i), type); + + while (nextInitialByte.InitialByte != CborInitialByte.IndefiniteLengthBreakByte) + { + checked + { + int chunkLength = (int)ReadUnsignedInteger(buffer.Slice(i), nextInitialByte, out int additionalBytes); + ranges.Add((i + 1 + additionalBytes, chunkLength)); + i += 1 + additionalBytes + chunkLength; + concatenatedBufferSize += chunkLength; + } + + nextInitialByte = ReadNextInitialByte(buffer.Slice(i), type); + } + + encodingLength = i + 1; // include the break byte + return ranges; + + static CborInitialByte ReadNextInitialByte(ReadOnlySpan buffer, CborMajorType expectedType) + { + EnsureBuffer(buffer, 1); + var cib = new CborInitialByte(buffer[0]); + + if (cib.InitialByte != CborInitialByte.IndefiniteLengthBreakByte && cib.MajorType != expectedType) + { + throw new FormatException("Indefinite-length CBOR string containing invalid data item."); + } + + return cib; + } + } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs index 56c8c26d15582..a169500aeb0ab 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs @@ -4,6 +4,7 @@ #nullable enable using System.Collections.Generic; +using System.Diagnostics; namespace System.Security.Cryptography.Encoding.Tests.Cbor { @@ -77,8 +78,11 @@ public CborReaderState Peek() if (initialByte.InitialByte == CborInitialByte.IndefiniteLengthBreakByte) { - if (_nestedDataItemStack?.Count > 0 && _remainingDataItems == null) + if (_remainingDataItems == null) { + // stack guaranteed to be populated since root context cannot be indefinite-length + Debug.Assert(_nestedDataItemStack != null && _nestedDataItemStack.Count > 0); + return _nestedDataItemStack.Peek().type switch { CborMajorType.ByteString => CborReaderState.EndByteString, @@ -95,6 +99,27 @@ public CborReaderState Peek() } } + if (_remainingDataItems == null) + { + // stack guaranteed to be populated since root context cannot be indefinite-length + Debug.Assert(_nestedDataItemStack != null && _nestedDataItemStack.Count > 0); + + CborMajorType parentType = _nestedDataItemStack.Peek().type; + + switch (parentType) + { + case CborMajorType.ByteString: + case CborMajorType.TextString: + // indefinite length string contexts can only contain data items of same major type + if (initialByte.MajorType != parentType) + { + return CborReaderState.FormatError; + } + + break; + } + } + return initialByte.MajorType switch { CborMajorType.UnsignedInteger => CborReaderState.UnsignedInteger, @@ -228,5 +253,12 @@ private void EnsureBuffer(int length) throw new FormatException("Unexpected end of buffer."); } } + private static void EnsureBuffer(ReadOnlySpan buffer, int requiredLength) + { + if (buffer.Length < requiredLength) + { + throw new FormatException("Unexpected end of buffer."); + } + } } } From baafb4c1e049e99c05eac5c9916d793135aa3f0d Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 24 Mar 2020 01:47:15 +0000 Subject: [PATCH 07/12] add tests for nested indefinite-length strings --- .../Cbor.Tests/CborReaderTests.Helpers.cs | 8 ++-- .../Cbor.Tests/CborReaderTests.String.cs | 48 ++++++++++++++++++- .../Cbor.Tests/CborWriterTests.Helpers.cs | 8 ++-- .../Cbor.Tests/CborWriterTests.String.cs | 6 +-- .../tests/Cbor/CborReader.String.cs | 8 ++-- .../tests/Cbor/CborWriter.String.cs | 4 +- 6 files changed, 63 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs index d079d30734cc2..16f444b6946a2 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs @@ -44,7 +44,7 @@ public static void VerifyValue(CborReader reader, object expectedValue, bool exp break; case string[] expectedChunks: Assert.Equal(CborReaderState.StartTextString, reader.Peek()); - reader.ReadStartTextString(); + reader.ReadStartTextStringIndefiniteLength(); foreach(string expectedChunk in expectedChunks) { Assert.Equal(CborReaderState.TextString, reader.Peek()); @@ -52,11 +52,11 @@ public static void VerifyValue(CborReader reader, object expectedValue, bool exp Assert.Equal(expectedChunk, chunk); } Assert.Equal(CborReaderState.EndTextString, reader.Peek()); - reader.ReadEndTextString(); + reader.ReadEndTextStringIndefiniteLength(); break; case byte[][] expectedChunks: Assert.Equal(CborReaderState.StartByteString, reader.Peek()); - reader.ReadStartByteString(); + reader.ReadStartByteStringIndefiniteLength(); foreach (byte[] expectedChunk in expectedChunks) { Assert.Equal(CborReaderState.ByteString, reader.Peek()); @@ -64,7 +64,7 @@ public static void VerifyValue(CborReader reader, object expectedValue, bool exp Assert.Equal(expectedChunk.ByteArrayToHex(), chunk.ByteArrayToHex()); } Assert.Equal(CborReaderState.EndByteString, reader.Peek()); - reader.ReadEndByteString(); + reader.ReadEndByteStringIndefiniteLength(); break; case object[] nested when CborWriterTests.Helpers.IsCborMapRepresentation(nested): diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs index 060aa6159714e..d6fdd254cd24b 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs @@ -479,7 +479,7 @@ public static void ReadByteString_IndefiniteLength_ContainingInvalidMajorTypes_S string hexEncoding = "5f4001ff"; byte[] data = hexEncoding.HexToByteArray(); var reader = new CborReader(data); - reader.ReadStartByteString(); + reader.ReadStartByteStringIndefiniteLength(); reader.ReadByteString(); Assert.Equal(CborReaderState.FormatError, reader.Peek()); @@ -493,7 +493,7 @@ public static void ReadTextString_IndefiniteLength_ContainingInvalidMajorTypes_S string hexEncoding = "7f6001ff"; byte[] data = hexEncoding.HexToByteArray(); var reader = new CborReader(data); - reader.ReadStartTextString(); + reader.ReadStartTextStringIndefiniteLength(); reader.ReadTextString(); Assert.Equal(CborReaderState.FormatError, reader.Peek()); @@ -501,6 +501,50 @@ public static void ReadTextString_IndefiniteLength_ContainingInvalidMajorTypes_S Assert.Throws(() => reader.ReadInt64()); } + [Fact] + public static void ReadByteString_IndefiniteLength_ContainingNestedIndefiniteLengthStrings_ShouldThrowFormatException() + { + string hexEncoding = "5f5fffff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + + reader.ReadStartByteStringIndefiniteLength(); + + Assert.Throws(() => reader.ReadStartByteStringIndefiniteLength()); + } + + [Fact] + public static void ReadByteString_IndefiniteLengthConcatenated_ContainingNestedIndefiniteLengthStrings_ShouldThrowFormatException() + { + string hexEncoding = "5f5fffff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + + Assert.Throws(() => reader.ReadByteString()); + } + + [Fact] + public static void ReadTextString_IndefiniteLength_ContainingNestedIndefiniteLengthStrings_ShouldThrowFormatException() + { + string hexEncoding = "7f7fffff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + + reader.ReadStartTextStringIndefiniteLength(); + + Assert.Throws(() => reader.ReadStartTextStringIndefiniteLength()); + } + + [Fact] + public static void ReadTextString_IndefiniteLengthConcatenated_ContainingNestedIndefiniteLengthStrings_ShouldThrowFormatException() + { + string hexEncoding = "7f7fffff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + + Assert.Throws(() => reader.ReadTextString()); + } + [Fact] public static void ReadByteString_IndefiniteLengthConcatenated_ContainingInvalidMajorTypes_ShouldThrowFormatException() { diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs index 36bb3370f44bf..1aa5e94d217c0 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.Helpers.cs @@ -89,7 +89,7 @@ public static void WriteChunkedByteString(CborWriter writer, byte[][] chunks) { writer.WriteByteString(chunk); } - writer.WriteEndByteString(); + writer.WriteEndByteStringIndefiniteLength(); } public static void WriteChunkedTextString(CborWriter writer, string[] chunks) @@ -99,7 +99,7 @@ public static void WriteChunkedTextString(CborWriter writer, string[] chunks) { writer.WriteTextString(chunk); } - writer.WriteEndTextString(); + writer.WriteEndTextStringIndefiniteLength(); } public static void ExecOperation(CborWriter writer, string op) @@ -113,8 +113,8 @@ public static void ExecOperation(CborWriter writer, string op) case nameof(writer.WriteStartByteStringIndefiniteLength): writer.WriteStartByteStringIndefiniteLength(); break; case nameof(writer.WriteStartArray): writer.WriteStartArrayIndefiniteLength(); break; case nameof(writer.WriteStartMap): writer.WriteStartMapIndefiniteLength(); break; - case nameof(writer.WriteEndByteString): writer.WriteEndByteString(); break; - case nameof(writer.WriteEndTextString): writer.WriteEndTextString(); break; + case nameof(writer.WriteEndByteStringIndefiniteLength): writer.WriteEndByteStringIndefiniteLength(); break; + case nameof(writer.WriteEndTextStringIndefiniteLength): writer.WriteEndTextStringIndefiniteLength(); break; case nameof(writer.WriteEndArray): writer.WriteEndArray(); break; case nameof(writer.WriteEndMap): writer.WriteEndMap(); break; default: throw new Exception($"Unrecognized CborWriter operation name {op}"); diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs index c36fe9e716d4c..9705af552b677 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs @@ -95,7 +95,7 @@ public static void WriteTextString_IndefiniteLength_NestedWrites_ShouldThrowInva } [Theory] - [InlineData(nameof(CborWriter.WriteEndByteString))] + [InlineData(nameof(CborWriter.WriteEndByteStringIndefiniteLength))] [InlineData(nameof(CborWriter.WriteEndArray))] [InlineData(nameof(CborWriter.WriteEndMap))] public static void WriteTextString_IndefiniteLength_ImbalancedWrites_ShouldThrowInvalidOperationException(string opName) @@ -112,7 +112,7 @@ public static void WriteTextString_IndefiniteLength_ImbalancedWrites_ShouldThrow [InlineData(nameof(CborWriter.WriteStartByteStringIndefiniteLength))] [InlineData(nameof(CborWriter.WriteStartArray))] [InlineData(nameof(CborWriter.WriteStartMap))] - [InlineData(nameof(CborWriter.WriteEndTextString))] + [InlineData(nameof(CborWriter.WriteEndTextStringIndefiniteLength))] [InlineData(nameof(CborWriter.WriteEndArray))] [InlineData(nameof(CborWriter.WriteEndMap))] public static void WriteByteString_IndefiteLength_NestedWrites_ShouldThrowInvalidOperationException(string opName) @@ -123,7 +123,7 @@ public static void WriteByteString_IndefiteLength_NestedWrites_ShouldThrowInvali } [Theory] - [InlineData(nameof(CborWriter.WriteEndTextString))] + [InlineData(nameof(CborWriter.WriteEndTextStringIndefiniteLength))] [InlineData(nameof(CborWriter.WriteEndArray))] [InlineData(nameof(CborWriter.WriteEndMap))] public static void WriteByteString_IndefiteLength_ImbalancedWrites_ShouldThrowInvalidOperationException(string opName) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs index d61a7501b4364..fe4552a0385ba 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs @@ -105,7 +105,7 @@ public bool TryReadTextString(Span destination, out int charsWritten) return true; } - public void ReadStartTextString() + public void ReadStartTextStringIndefiniteLength() { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.TextString); @@ -120,14 +120,14 @@ public void ReadStartTextString() PushDataItem(CborMajorType.TextString, expectedNestedItems: null); } - public void ReadEndTextString() + public void ReadEndTextStringIndefiniteLength() { ReadNextIndefiniteLengthBreakByte(); PopDataItem(CborMajorType.TextString); AdvanceBuffer(1); } - public void ReadStartByteString() + public void ReadStartByteStringIndefiniteLength() { CborInitialByte header = PeekInitialByte(expectedType: CborMajorType.ByteString); @@ -142,7 +142,7 @@ public void ReadStartByteString() PushDataItem(CborMajorType.ByteString, expectedNestedItems: null); } - public void ReadEndByteString() + public void ReadEndByteStringIndefiniteLength() { ReadNextIndefiniteLengthBreakByte(); PopDataItem(CborMajorType.ByteString); diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs index cb940705c9f2e..bd9517718971a 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborWriter.String.cs @@ -39,7 +39,7 @@ public void WriteStartByteStringIndefiniteLength() PushDataItem(CborMajorType.ByteString, expectedNestedItems: null); } - public void WriteEndByteString() + public void WriteEndByteStringIndefiniteLength() { EnsureWriteCapacity(1); WriteInitialByte(new CborInitialByte(CborInitialByte.IndefiniteLengthBreakByte)); @@ -54,7 +54,7 @@ public void WriteStartTextStringIndefiniteLength() PushDataItem(CborMajorType.TextString, expectedNestedItems: null); } - public void WriteEndTextString() + public void WriteEndTextStringIndefiniteLength() { EnsureWriteCapacity(1); WriteInitialByte(new CborInitialByte(CborInitialByte.IndefiniteLengthBreakByte)); From c6a83bb303685bb991221e57b0fbb76e31255648 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 24 Mar 2020 18:13:50 +0000 Subject: [PATCH 08/12] fix naming issues --- .../tests/Cbor/CborReader.String.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs index fe4552a0385ba..fa2bdfea0895d 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs @@ -151,7 +151,7 @@ public void ReadEndByteStringIndefiniteLength() private bool TryReadChunkedByteStringConcatenated(Span destination, out int bytesWritten) { - List<(int offset, int length)> offsets = ReadChunkedStringRanges(CborMajorType.ByteString, out int offset, out int concatenatedBufferSize); + List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.ByteString, out int encodingLength, out int concatenatedBufferSize); if (concatenatedBufferSize > destination.Length) { @@ -161,21 +161,21 @@ private bool TryReadChunkedByteStringConcatenated(Span destination, out in ReadOnlySpan source = _buffer.Span; - foreach ((int o, int l) in offsets) + foreach ((int o, int l) in ranges) { source.Slice(o, l).CopyTo(destination); destination = destination.Slice(l); } bytesWritten = concatenatedBufferSize; - AdvanceBuffer(offset); + AdvanceBuffer(encodingLength); DecrementRemainingItemCount(); return true; } private bool TryReadChunkedTextStringConcatenated(Span destination, out int charsWritten) { - List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.TextString, out int offset, out int _); + List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.TextString, out int encodingLength, out int _); ReadOnlySpan buffer = _buffer.Span; int concatenatedStringSize = 0; @@ -197,14 +197,14 @@ private bool TryReadChunkedTextStringConcatenated(Span destination, out in } charsWritten = concatenatedStringSize; - AdvanceBuffer(offset); + AdvanceBuffer(encodingLength); DecrementRemainingItemCount(); return true; } private byte[] ReadChunkedByteStringConcatenated() { - List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.ByteString, out int offset, out int concatenatedBufferSize); + List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.ByteString, out int encodingLength, out int concatenatedBufferSize); var output = new byte[concatenatedBufferSize]; ReadOnlySpan source = _buffer.Span; @@ -217,14 +217,14 @@ private byte[] ReadChunkedByteStringConcatenated() } Debug.Assert(target.IsEmpty); - AdvanceBuffer(offset); + AdvanceBuffer(encodingLength); DecrementRemainingItemCount(); return output; } private string ReadChunkedTextStringConcatenated() { - List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.TextString, out int offset, out int concatenatedBufferSize); + List<(int offset, int length)> ranges = ReadChunkedStringRanges(CborMajorType.TextString, out int encodingLength, out int concatenatedBufferSize); ReadOnlySpan buffer = _buffer.Span; int concatenatedStringSize = 0; @@ -243,7 +243,7 @@ private string ReadChunkedTextStringConcatenated() } Debug.Assert(target.IsEmpty); - AdvanceBuffer(offset); + AdvanceBuffer(encodingLength); DecrementRemainingItemCount(); return new string(output); } From dc30f7909ea94daf38b48a35a29f61c72a309e94 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 24 Mar 2020 19:37:54 +0000 Subject: [PATCH 09/12] check that TryReadString() methods are idempotent on failed reads. --- .../Cbor.Tests/CborReaderTests.String.cs | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs index d6fdd254cd24b..bc5a55d71cad6 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs @@ -185,13 +185,19 @@ public static void TryReadTextString_IndefiniteLengthConcatenated_SingleValue__H [InlineData("ffffffffffffffffffffffffffff", "4effffffffffffffffffffffffffff")] public static void TryReadByteString_BufferTooSmall_ShouldReturnFalse(string actualValue, string hexEncoding) { - byte[] buffer = new byte[actualValue.Length / 2 - 1]; + byte[] buffer = new byte[actualValue.Length / 2]; byte[] encoding = hexEncoding.HexToByteArray(); var reader = new CborReader(encoding); - bool result = reader.TryReadByteString(buffer, out int bytesWritten); + bool result = reader.TryReadByteString(buffer.AsSpan(1), out int bytesWritten); Assert.False(result); Assert.Equal(0, bytesWritten); Assert.All(buffer, (b => Assert.Equal(0, b))); + + // ensure that reader is still able to complete the read operation if a large enough buffer is supplied subsequently + result = reader.TryReadByteString(buffer, out bytesWritten); + Assert.True(result); + Assert.Equal(buffer.Length, bytesWritten); + Assert.Equal(actualValue.ToUpper(), buffer.ByteArrayToHex()); } [Theory] @@ -204,13 +210,19 @@ public static void TryReadByteString_BufferTooSmall_ShouldReturnFalse(string act [InlineData("\ud800\udd51", "64f0908591")] public static void TryReadTextString_BufferTooSmall_ShouldReturnFalse(string actualValue, string hexEncoding) { - char[] buffer = new char[actualValue.Length - 1]; + char[] buffer = new char[actualValue.Length]; byte[] encoding = hexEncoding.HexToByteArray(); var reader = new CborReader(encoding); - bool result = reader.TryReadTextString(buffer, out int charsWritten); + bool result = reader.TryReadTextString(buffer.AsSpan(1), out int charsWritten); Assert.False(result); Assert.Equal(0, charsWritten); Assert.All(buffer, (b => Assert.Equal(0, '\0'))); + + // ensure that reader is still able to complete the read operation if a large enough buffer is supplied subsequently + result = reader.TryReadTextString(buffer, out charsWritten); + Assert.True(result); + Assert.Equal(actualValue.Length, charsWritten); + Assert.Equal(actualValue, new string(buffer.AsSpan(0, charsWritten))); } [Theory] @@ -221,12 +233,18 @@ public static void TryReadByteString_IndefiniteLengthConcatenated_BufferTooSmall byte[] data = hexEncoding.HexToByteArray(); var reader = new CborReader(data); - byte[] buffer = new byte[expectedHexValue.Length / 2 - 1]; - bool result = reader.TryReadByteString(buffer, out int bytesWritten); + byte[] buffer = new byte[expectedHexValue.Length / 2]; + bool result = reader.TryReadByteString(buffer.AsSpan(1), out int bytesWritten); Assert.False(result); Assert.Equal(0, bytesWritten); Assert.All(buffer, (b => Assert.Equal(0, b))); + + // ensure that reader is still able to complete the read operation if a large enough buffer is supplied subsequently + result = reader.TryReadByteString(buffer, out bytesWritten); + Assert.True(result); + Assert.Equal(buffer.Length, bytesWritten); + Assert.Equal(expectedHexValue.ToUpper(), buffer.ByteArrayToHex()); } [Theory] @@ -237,12 +255,18 @@ public static void TryReadTextString_IndefiniteLengthConcatenated_BufferTooSmall byte[] data = hexEncoding.HexToByteArray(); var reader = new CborReader(data); - char[] buffer = new char[expectedValue.Length - 1]; - bool result = reader.TryReadTextString(buffer, out int charsWritten); + char[] buffer = new char[expectedValue.Length]; + bool result = reader.TryReadTextString(buffer.AsSpan(1), out int charsWritten); Assert.False(result); Assert.Equal(0, charsWritten); Assert.All(buffer, (b => Assert.Equal(0, '\0'))); + + // ensure that reader is still able to perform the read operation if a large enough buffer is supplied subsequently + result = reader.TryReadTextString(buffer, out charsWritten); + Assert.True(result); + Assert.Equal(expectedValue.Length, charsWritten); + Assert.Equal(expectedValue, new string(buffer.AsSpan(0, charsWritten))); } [Theory] From 19048dac8e06a4c3c5bd7f31ba39c4478df503d8 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 24 Mar 2020 21:40:24 +0000 Subject: [PATCH 10/12] use string.Create instead of char buffer; share single range list allocation --- .../Cbor.Tests/CborReaderTests.String.cs | 26 +++++++++++ .../tests/Cbor/CborReader.String.cs | 46 ++++++++++++++----- .../tests/Cbor/CborReader.cs | 1 + 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs index bc5a55d71cad6..ead9666d5199c 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs @@ -133,6 +133,19 @@ public static void TryReadByteString_IndefiniteLengthConcatenated_SingleValue_Ha Assert.Equal(CborReaderState.Finished, reader.Peek()); } + [Fact] + public static void ReadByteString_IndefiniteLengthConcatenated_NestedValues_HappyPath() + { + string hexEncoding = "825f41ab40ff5f41ab40ff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + reader.ReadStartArray(); + Assert.Equal("AB", reader.ReadByteString().ByteArrayToHex()); + Assert.Equal("AB", reader.ReadByteString().ByteArrayToHex()); + reader.ReadEndArray(); + Assert.Equal(CborReaderState.Finished, reader.Peek()); + } + [Theory] [InlineData(new string[] { }, "7fff")] [InlineData(new string[] { "" }, "7f60ff")] @@ -160,6 +173,19 @@ public static void ReadTextString_IndefiniteLengthConcatenated_SingleValue_Happy Assert.Equal(CborReaderState.Finished, reader.Peek()); } + [Fact] + public static void ReadTextString_IndefiniteLengthConcatenated_NestedValues_HappyPath() + { + string hexEncoding = "827f62616260ff7f62616260ff"; + byte[] data = hexEncoding.HexToByteArray(); + var reader = new CborReader(data); + reader.ReadStartArray(); + Assert.Equal("ab", reader.ReadTextString()); + Assert.Equal("ab", reader.ReadTextString()); + reader.ReadEndArray(); + Assert.Equal(CborReaderState.Finished, reader.Peek()); + } + [Theory] [InlineData("", "7fff")] [InlineData("", "7f60ff")] diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs index fa2bdfea0895d..fc6636dddafe9 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Text; +using System.Threading; namespace System.Security.Cryptography.Encoding.Tests.Cbor { @@ -170,6 +171,7 @@ private bool TryReadChunkedByteStringConcatenated(Span destination, out in bytesWritten = concatenatedBufferSize; AdvanceBuffer(encodingLength); DecrementRemainingItemCount(); + ReturnRangeList(ranges); return true; } @@ -199,6 +201,7 @@ private bool TryReadChunkedTextStringConcatenated(Span destination, out in charsWritten = concatenatedStringSize; AdvanceBuffer(encodingLength); DecrementRemainingItemCount(); + ReturnRangeList(ranges); return true; } @@ -219,6 +222,7 @@ private byte[] ReadChunkedByteStringConcatenated() Debug.Assert(target.IsEmpty); AdvanceBuffer(encodingLength); DecrementRemainingItemCount(); + ReturnRangeList(ranges); return output; } @@ -233,26 +237,32 @@ private string ReadChunkedTextStringConcatenated() concatenatedStringSize += s_utf8Encoding.GetCharCount(buffer.Slice(o, l)); } - var output = new char[concatenatedStringSize]; - Span target = output; + string output = string.Create(concatenatedStringSize, (ranges, _buffer), BuildString); - foreach ((int o, int l) in ranges) - { - s_utf8Encoding.GetChars(buffer.Slice(o, l), target); - target = target.Slice(l); - } - - Debug.Assert(target.IsEmpty); AdvanceBuffer(encodingLength); DecrementRemainingItemCount(); - return new string(output); + ReturnRangeList(ranges); + return output; + + static void BuildString(Span target, (List<(int offset, int length)> ranges, ReadOnlyMemory source) input) + { + ReadOnlySpan source = input.source.Span; + + foreach ((int o, int l) in input.ranges) + { + s_utf8Encoding.GetChars(source.Slice(o, l), target); + target = target.Slice(l); + } + + Debug.Assert(target.IsEmpty); + } } // reads a buffer starting with an indefinite-length string, // performing validation and returning a list of ranges containing the individual chunk payloads private List<(int offset, int length)> ReadChunkedStringRanges(CborMajorType type, out int encodingLength, out int concatenatedBufferSize) { - var ranges = new List<(int offset, int length)>(); + var ranges = GetRangeList(); ReadOnlySpan buffer = _buffer.Span; concatenatedBufferSize = 0; @@ -288,5 +298,19 @@ static CborInitialByte ReadNextInitialByte(ReadOnlySpan buffer, CborMajorT return cib; } } + + private List<(int offset, int length)>? _rangeListAllocation = null; + private List<(int offset, int length)> GetRangeList() + { + List<(int offset, int length)>? ranges = Interlocked.Exchange(ref _rangeListAllocation, null); + ranges ??= new List<(int, int)>(); + return ranges; + } + + private void ReturnRangeList(List<(int offset, int length)> ranges) + { + ranges.Clear(); + _rangeListAllocation = ranges; + } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs index a169500aeb0ab..fc5f78f9d12f1 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs @@ -253,6 +253,7 @@ private void EnsureBuffer(int length) throw new FormatException("Unexpected end of buffer."); } } + private static void EnsureBuffer(ReadOnlySpan buffer, int requiredLength) { if (buffer.Length < requiredLength) From 08f5538943a0fb59a77e082a345d65b5b1ca9000 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 24 Mar 2020 21:49:16 +0000 Subject: [PATCH 11/12] only clear List if it is guaranteed to be reused --- .../tests/Cbor/CborReader.String.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs index fc6636dddafe9..a08b73d137267 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs @@ -262,7 +262,7 @@ static void BuildString(Span target, (List<(int offset, int length)> range // performing validation and returning a list of ranges containing the individual chunk payloads private List<(int offset, int length)> ReadChunkedStringRanges(CborMajorType type, out int encodingLength, out int concatenatedBufferSize) { - var ranges = GetRangeList(); + var ranges = AcquireRangeList(); ReadOnlySpan buffer = _buffer.Span; concatenatedBufferSize = 0; @@ -300,16 +300,21 @@ static CborInitialByte ReadNextInitialByte(ReadOnlySpan buffer, CborMajorT } private List<(int offset, int length)>? _rangeListAllocation = null; - private List<(int offset, int length)> GetRangeList() + private List<(int offset, int length)> AcquireRangeList() { List<(int offset, int length)>? ranges = Interlocked.Exchange(ref _rangeListAllocation, null); - ranges ??= new List<(int, int)>(); - return ranges; + + if (ranges != null) + { + ranges.Clear(); + return ranges; + } + + return new List<(int, int)>(); } private void ReturnRangeList(List<(int offset, int length)> ranges) { - ranges.Clear(); _rangeListAllocation = ranges; } } From 6912133ea64491e661bb117b111414742d68a59f Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 24 Mar 2020 21:59:11 +0000 Subject: [PATCH 12/12] move field to top of main CborReader source file. --- .../tests/Cbor/CborReader.String.cs | 22 ------------------- .../tests/Cbor/CborReader.cs | 22 +++++++++++++++++++ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs index a08b73d137267..0093a8247e30f 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.String.cs @@ -3,11 +3,8 @@ // See the LICENSE file in the project root for more information. #nullable enable -using System.Buffers.Binary; using System.Collections.Generic; using System.Diagnostics; -using System.Text; -using System.Threading; namespace System.Security.Cryptography.Encoding.Tests.Cbor { @@ -298,24 +295,5 @@ static CborInitialByte ReadNextInitialByte(ReadOnlySpan buffer, CborMajorT return cib; } } - - private List<(int offset, int length)>? _rangeListAllocation = null; - private List<(int offset, int length)> AcquireRangeList() - { - List<(int offset, int length)>? ranges = Interlocked.Exchange(ref _rangeListAllocation, null); - - if (ranges != null) - { - ranges.Clear(); - return ranges; - } - - return new List<(int, int)>(); - } - - private void ReturnRangeList(List<(int offset, int length)> ranges) - { - _rangeListAllocation = ranges; - } } } diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs index fc5f78f9d12f1..fb61dda27af97 100644 --- a/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs +++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Cbor/CborReader.cs @@ -5,6 +5,7 @@ #nullable enable using System.Collections.Generic; using System.Diagnostics; +using System.Threading; namespace System.Security.Cryptography.Encoding.Tests.Cbor { @@ -42,6 +43,9 @@ internal partial class CborReader private bool _isEvenNumberOfDataItemsRead = true; // required for indefinite-length map writes private Stack<(CborMajorType type, bool isEvenNumberOfDataItemsWritten, ulong? remainingDataItems)>? _nestedDataItemStack; + // stores a reusable List allocation for keeping ranges in the buffer + private List<(int offset, int length)>? _rangeListAllocation = null; + internal CborReader(ReadOnlyMemory buffer) { _buffer = buffer; @@ -261,5 +265,23 @@ private static void EnsureBuffer(ReadOnlySpan buffer, int requiredLength) throw new FormatException("Unexpected end of buffer."); } } + + private List<(int offset, int length)> AcquireRangeList() + { + List<(int offset, int length)>? ranges = Interlocked.Exchange(ref _rangeListAllocation, null); + + if (ranges != null) + { + ranges.Clear(); + return ranges; + } + + return new List<(int, int)>(); + } + + private void ReturnRangeList(List<(int offset, int length)> ranges) + { + _rangeListAllocation = ranges; + } } }