diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs index c1f86a1ca2d1ee..98994b6b40e4c7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs +++ b/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 value) where T : ISpanFormattable + private StringBuilder AppendSpanFormattable(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 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 RemainingCurrentChunk get => new Span(m_ChunkChars, m_ChunkLength, m_ChunkChars.Length - m_ChunkLength); } + private int RemainingCurrentChunkLength => m_ChunkChars.Length - m_ChunkLength; + /// /// Finds the chunk that logically succeeds the specified chunk. /// diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Text/StringBuilderTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Text/StringBuilderTests.cs index 2d1f569dd103be..ea97e79b1fdcf3 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Text/StringBuilderTests.cs +++ b/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(s_noCapacityParamName, () => builder.Append((long)1)); } + public static IEnumerable 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 destination, out int charsWritten, ReadOnlySpan 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(() => builder.Append(new InvalidCharsWritten(tooBig))); + } + + private sealed class InvalidCharsWritten : ISpanFormattable + { + private bool _tooBig; + + public InvalidCharsWritten(bool tooBig) => _tooBig = tooBig; + + public bool TryFormat(Span destination, out int charsWritten, ReadOnlySpan format, IFormatProvider provider) + { + charsWritten = _tooBig ? destination.Length + 1 : -1; + return true; + } + + public string ToString(string format, IFormatProvider formatProvider) => + throw new NotImplementedException(); } [Fact]