Wrap utf8 encoding and decoding exceptions in ArgumentException and F…

…ormatException respectively
dotnet · Apr 8, 2020 · ccffb90 · ccffb90
1 parent 5815768
commit ccffb90
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 25 deletions.
diff --git a/...braries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs b/...braries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.Helpers.cs
@@ -217,6 +217,9 @@ public static void VerifyMap(CborReader reader, object[] expectedValues, bool ex
                 "4201",
                 "61",
                 "6261",
+                // invalid utf8 strings
+                "61ff",
+                "62f090",
                 // indefinite-length strings with missing break byte
                 "5f41ab40",
                 "7f62616260",

diff --git a/...aries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.SkipValue.cs b/...aries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.SkipValue.cs
@@ -72,17 +72,6 @@ public static void SkipValue_InvalidFormat_ShouldThrowFormatException(string hex
             Assert.Throws<FormatException>(() => reader.SkipValue());
         }
 
-        [Theory]
-        [InlineData("61ff")]
-        [InlineData("62f090")]
-        public static void SkipValue_InvalidUtf8_ShouldDecoderFallbackException(string hexEncoding)
-        {
-            byte[] encoding = hexEncoding.HexToByteArray();
-            var reader = new CborReader(encoding);
-
-            Assert.Throws<DecoderFallbackException>(() => reader.SkipValue());
-        }
-
         [Theory]
         [InlineData(50_000)]
         public static void SkipValue_ExtremelyNestedValues_ShouldNotStackOverflow(int depth)

diff --git a/...ibraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs b/...ibraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborReaderTests.String.cs
@@ -486,23 +486,23 @@ public static void ReadTextString_StringLengthTooLarge_ShouldThrowOverflowExcept
         [Theory]
         [InlineData("61ff")]
         [InlineData("62f090")]
-        public static void ReadTextString_InvalidUnicode_ShouldThrowDecoderFallbackException(string hexEncoding)
+        public static void ReadTextString_InvalidUnicode_ShouldThrowFormatException(string hexEncoding)
         {
             byte[] data = hexEncoding.HexToByteArray();
             var reader = new CborReader(data);
-            Assert.Throws<System.Text.DecoderFallbackException>(() => reader.ReadTextString());
+            Assert.Throws<FormatException>(() => reader.ReadTextString());
         }
 
         [Theory]
         [InlineData("61ff")]
         [InlineData("62f090")]
-        public static void TryReadTextString_InvalidUnicode_ShouldThrowDecoderFallbackException(string hexEncoding)
+        public static void TryReadTextString_InvalidUnicode_ShouldThrowFormatException(string hexEncoding)
         {
             byte[] data = hexEncoding.HexToByteArray();
             char[] buffer = new char[32];
             var reader = new CborReader(data);
 
-            Assert.Throws<System.Text.DecoderFallbackException>(() => reader.TryReadTextString(buffer, out int _));
+            Assert.Throws<FormatException>(() => reader.TryReadTextString(buffer, out int _));
         }
 
         [Fact]
@@ -614,15 +614,15 @@ public static void ReadTextString_IndefiniteLengthConcatenated_ContainingInvalid
         }
 
         [Fact]
-        public static void ReadTextString_IndefiniteLengthConcatenated_InvalidUtf8Chunks_ShouldThrowDecoderFallbackException()
+        public static void ReadTextString_IndefiniteLengthConcatenated_InvalidUtf8Chunks_ShouldThrowFormatException()
         {
             // 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<DecoderFallbackException>(() => reader.ReadTextString());
+            Assert.Throws<FormatException>(() => reader.ReadTextString());
         }
     }
 }
diff --git a/...ibraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs b/...ibraries/System.Security.Cryptography.Encoding/tests/Cbor.Tests/CborWriterTests.String.cs
@@ -72,12 +72,12 @@ public static void WriteTextString_IndefiniteLength_SingleValue_HappyPath(string
         }
 
         [Fact]
-        public static void WriteTextString_InvalidUnicodeString_ShouldThrowEncoderFallbackException()
+        public static void WriteTextString_InvalidUnicodeString_ShouldThrowArgumentException()
         {
             // NB Xunit's InlineDataAttribute will corrupt string literals containing invalid unicode
             string invalidUnicodeString = "\ud800";
             using var writer = new CborWriter();
-            Assert.Throws<System.Text.EncoderFallbackException>(() => writer.WriteTextString(invalidUnicodeString));
+            Assert.Throws<ArgumentException>(() => writer.WriteTextString(invalidUnicodeString));
         }
 
         [Theory]

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
@@ -5,6 +5,7 @@
 #nullable enable
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Text;
 
 namespace System.Security.Cryptography.Encoding.Tests.Cbor
 {
@@ -70,7 +71,17 @@ public string ReadTextString()
             int length = checked((int)ReadUnsignedInteger(_buffer.Span, header, out int additionalBytes));
             EnsureBuffer(1 + additionalBytes + length);
             ReadOnlySpan<byte> encodedString = _buffer.Span.Slice(1 + additionalBytes, length);
-            string result = s_utf8Encoding.GetString(encodedString);
+
+            string result;
+            try
+            {
+                result = s_utf8Encoding.GetString(encodedString);
+            }
+            catch (DecoderFallbackException e)
+            {
+                throw new FormatException("Text string payload is not a valid UTF8 string.", e);
+            }
+
             AdvanceBuffer(1 + additionalBytes + length);
             AdvanceDataItemCounters();
             return result;
@@ -89,7 +100,9 @@ public bool TryReadTextString(Span<char> destination, out int charsWritten)
             EnsureBuffer(1 + additionalBytes + byteLength);
 
             ReadOnlySpan<byte> encodedSlice = _buffer.Span.Slice(1 + additionalBytes, byteLength);
-            int charLength = s_utf8Encoding.GetCharCount(encodedSlice);
+
+            int charLength = ValidateUtf8AndGetCharCount(encodedSlice);
+
             if (charLength > destination.Length)
             {
                 charsWritten = 0;
@@ -180,7 +193,7 @@ private bool TryReadChunkedTextStringConcatenated(Span<char> destination, out in
             int concatenatedStringSize = 0;
             foreach ((int o, int l) in ranges)
             {
-                concatenatedStringSize += s_utf8Encoding.GetCharCount(buffer.Slice(o, l));
+                concatenatedStringSize += ValidateUtf8AndGetCharCount(buffer.Slice(o, l));
             }
 
             if (concatenatedStringSize > destination.Length)
@@ -231,7 +244,7 @@ private string ReadChunkedTextStringConcatenated()
 
             foreach ((int o, int l) in ranges)
             {
-                concatenatedStringSize += s_utf8Encoding.GetCharCount(buffer.Slice(o, l));
+                concatenatedStringSize += ValidateUtf8AndGetCharCount(buffer.Slice(o, l));
             }
 
             string output = string.Create(concatenatedStringSize, (ranges, _buffer), BuildString);
@@ -310,11 +323,23 @@ private void SkipString(CborMajorType type)
             if (type == CborMajorType.TextString)
             {
                 ReadOnlySpan<byte> encodedSlice = buffer.Slice(1 + additionalBytes, byteLength);
-                s_utf8Encoding.GetCharCount(encodedSlice);
+                ValidateUtf8AndGetCharCount(encodedSlice);
             }
 
             AdvanceBuffer(1 + additionalBytes + byteLength);
             AdvanceDataItemCounters();
         }
+
+        private int ValidateUtf8AndGetCharCount(ReadOnlySpan<byte> buffer)
+        {
+            try
+            {
+                return s_utf8Encoding.GetCharCount(buffer);
+            }
+            catch (DecoderFallbackException e)
+            {
+                throw new FormatException("Text string payload is not a valid UTF8 string.", e);
+            }
+        }
     }
 }
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
@@ -25,7 +25,16 @@ public void WriteByteString(ReadOnlySpan<byte> value)
         // Implements major type 3 encoding per https://tools.ietf.org/html/rfc7049#section-2.1
         public void WriteTextString(ReadOnlySpan<char> value)
         {
-            int length = s_utf8Encoding.GetByteCount(value);
+            int length;
+            try
+            {
+                length = s_utf8Encoding.GetByteCount(value);
+            }
+            catch (EncoderFallbackException e)
+            {
+                throw new ArgumentException("Provided text string is not valid UTF8.", e);
+            }
+
             WriteUnsignedInteger(CborMajorType.TextString, (ulong)length);
             EnsureWriteCapacity(length);
             s_utf8Encoding.GetBytes(value, _buffer.AsSpan(_offset));