Add support for ISpanFormattable in StringBuilder.Append(Object)

src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs

@@ -1052,12 +1052,19 @@ private void AppendWithExpansion(char value)
         [CLSCompliant(false)]
         public StringBuilder Append(ulong value) => AppendSpanFormattable(value);
 
-        private StringBuilder AppendSpanFormattable<T>(T value) where T : ISpanFormattable
+        private StringBuilder AppendSpanFormattable<T>(T value, bool untrusted = false) where T : ISpanFormattable
         {
-            Debug.Assert(typeof(T).Assembly.Equals(typeof(object).Assembly), "Implementation trusts the results of TryFormat because T is expected to be something known");
+            Debug.Assert(untrusted || typeof(T).Assembly.Equals(typeof(object).Assembly), "Implementation trusts the results of TryFormat because T is expected to be something known");
 
             if (value.TryFormat(RemainingCurrentChunk, out int charsWritten, format: default, provider: null))
             {
+                if (untrusted && ((uint)charsWritten > (uint)RemainingCurrentChunkLength))
+                {
+                    // Protect against faulty ISpanFormattable implementations returning invalid charsWritten values.
+                    // Other code in _stringBuilder uses Unsafe manipulation, and we want to ensure m_ChunkLength remains safe.
+                    ThrowHelper.ThrowFormatInvalidString();
+                }
+
                 m_ChunkLength += charsWritten;
                 return this;
             }
@@ -1078,7 +1085,12 @@ internal StringBuilder AppendSpanFormattable<T>(T value, string? format, IFormat
             return Append(value.ToString(format, provider));
         }
 
-        public StringBuilder Append(object? value) => (value == null) ? this : Append(value.ToString());
+        public StringBuilder Append(object? value) => value switch
+        {
+            null => this,
+            ISpanFormattable sf => AppendSpanFormattable(sf, untrusted: true),
+            object o => Append(o.ToString())
+        };
 
         public StringBuilder Append(char[]? value)
         {
@@ -1620,7 +1632,7 @@ internal StringBuilder AppendFormatHelper(IFormatProvider? provider, string form
                         arg is ISpanFormattable spanFormattableArg &&
                         spanFormattableArg.TryFormat(RemainingCurrentChunk, out int charsWritten, itemFormatSpan, provider))
                     {
-                        if ((uint)charsWritten > (uint)RemainingCurrentChunk.Length)
+                        if ((uint)charsWritten > (uint)RemainingCurrentChunkLength)
                         {
                             // Untrusted ISpanFormattable implementations might return an erroneous charsWritten value,
                             // and m_ChunkLength might end up being used in Unsafe code, so fail if we get back an
@@ -2423,6 +2435,8 @@ private Span<char> RemainingCurrentChunk
             get => new Span<char>(m_ChunkChars, m_ChunkLength, m_ChunkChars.Length - m_ChunkLength);
         }
 
+        private int RemainingCurrentChunkLength => m_ChunkChars.Length - m_ChunkLength;
+
         /// <summary>
         /// Finds the chunk that logically succeeds the specified chunk.
         /// </summary>

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Text/StringBuilderTests.cs

@@ -378,17 +378,84 @@ public static void Append_Long_NoSpareCapacity_ThrowsArgumentOutOfRangeException
             AssertExtensions.Throws<ArgumentOutOfRangeException>(s_noCapacityParamName, () => builder.Append((long)1));
         }
 
+        public static IEnumerable<object[]> Append_Object_TestData()
+        {
+            yield return new object[] { "Hello", "abc", "Helloabc" };
+            yield return new object[] { "", "g", "g" };
+            yield return new object[] { "Hello", "", "Hello" };
+            yield return new object[] { "Hello", null, "Hello" };
+            yield return new object[] { "Hello", $"abc", "Helloabc" };
+            // ISpanFormattable inputs: simple validation of known types that implement the interface
+            yield return new object[] { "", (byte)42, "42" };
+            yield return new object[] { "", 'A', "A" };
+            yield return new object[] { "", DateTime.ParseExact("2021-03-15T14:52:51.5058563Z", "o", null, DateTimeStyles.AdjustToUniversal | DateTimeStyles.AssumeUniversal), "03/15/2021 14:52:51" };
+            yield return new object[] { "", DateTimeOffset.ParseExact("2021-03-15T14:52:51.5058563Z", "o", null, DateTimeStyles.AdjustToUniversal | DateTimeStyles.AssumeUniversal), "03/15/2021 14:52:51 +00:00" };
+            yield return new object[] { "", (decimal)42, "42" };
+            yield return new object[] { "", (double)42, "42" };
+            yield return new object[] { "", Guid.Parse("68d9cfaf-feab-4d5b-96d8-a3fd889ae89f"), "68d9cfaf-feab-4d5b-96d8-a3fd889ae89f" };
+            yield return new object[] { "", (Half)42, "42" };
+            yield return new object[] { "", (short)42, "42" };
+            yield return new object[] { "", (int)42, "42" };
+            yield return new object[] { "", (long)42, "42" };
+            yield return new object[] { "", (IntPtr)42, "42" };
+            yield return new object[] { "", new Rune('A'), "A" };
+            yield return new object[] { "", (sbyte)42, "42" };
+            yield return new object[] { "", (float)42, "42" };
+            yield return new object[] { "", TimeSpan.FromSeconds(42), "00:00:42" };
+            yield return new object[] { "", (ushort)42, "42" };
+            yield return new object[] { "", (uint)42, "42" };
+            yield return new object[] { "", (ulong)42, "42" };
+            yield return new object[] { "", (UIntPtr)42, "42" };
+            yield return new object[] { "", new Version(1, 2, 3, 4), "1.2.3.4" };
+            // ISpanFormattable inputs: advanced validation to ensure that ToString() is not called by default.
+            yield return new object[] { "Hello", new FormattableStringWithSpanOnly("abc"), "Helloabc" };
+            yield return new object[] { "Hello", $"{new FormattableStringWithSpanOnly("abc")}", "Helloabc" };
+        }
+
+        private struct FormattableStringWithSpanOnly(string value) : ISpanFormattable
+        {
+            public override readonly string ToString() => throw new NotImplementedException();
+            public readonly string ToString(string? format, IFormatProvider? formatProvider) => ToString();
+
+            public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider? provider) =>
+                destination.TryWrite($"{value}", out charsWritten);
+        }
+
         [Theory]
-        [InlineData("Hello", "abc", "Helloabc")]
-        [InlineData("Hello", "def", "Hellodef")]
-        [InlineData("", "g", "g")]
-        [InlineData("Hello", "", "Hello")]
-        [InlineData("Hello", null, "Hello")]
+        [MemberData(nameof(Append_Object_TestData))]
         public static void Append_Object(string original, object value, string expected)
         {
-            var builder = new StringBuilder(original);
-            builder.Append(value);
-            Assert.Equal(expected, builder.ToString());
+            using (new ThreadCultureChange(CultureInfo.InvariantCulture))
+            {
+                var builder = new StringBuilder(original);
+                builder.Append(value);
+                Assert.Equal(expected, builder.ToString());
+            }
+        }
+
+		[Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void Append_Object_InvalidTryFormatCharsWritten_Throws(bool tooBig) // vs tooSmall
+        {
+            var builder = new StringBuilder();
+            Assert.Throws<FormatException>(() => builder.Append(new InvalidCharsWritten(tooBig)));
+        }
+
+        private sealed class InvalidCharsWritten : ISpanFormattable
+        {
+            private bool _tooBig;
+
+            public InvalidCharsWritten(bool tooBig) => _tooBig = tooBig;
+
+            public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider provider)
+            {
+                charsWritten = _tooBig ? destination.Length + 1 : -1;
+                return true;
+            }
+
+            public string ToString(string format, IFormatProvider formatProvider) =>
+                throw new NotImplementedException();
         }
 
         [Fact]