Skip to content

Commit 44b194b

Browse files
Throw OverflowException from TryFormat when no string can hold the text
Co-authored-by: matt-edmondson <19528727+matt-edmondson@users.noreply.github.com>
1 parent ffc0fc7 commit 44b194b

2 files changed

Lines changed: 92 additions & 24 deletions

File tree

‎PreciseNumber.Test/PreciseNumberTests.cs‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2555,22 +2555,63 @@ public void TestCountDecimalDigitsAtExtremeExponents()
25552555
}
25562556

25572557
[TestMethod]
2558-
public void TestTryFormatAtExtremeExponentsReturnsFalse()
2558+
public void TestTryFormatAtExtremeExponentsThrowsOverflow()
25592559
{
2560-
// The text these need is longer than any int, so the only correct answer for a
2561-
// destination this size is false, not an exception.
2562-
Span<char> buffer = stackalloc char[64];
2560+
// The text these need is longer than any string can hold. That is a property of the
2561+
// value, not of the destination, so false would only invite the caller to grow its
2562+
// buffer and retry forever.
25632563
foreach (int exponent in new[] { int.MinValue, int.MaxValue })
25642564
{
25652565
PreciseNumber number = PreciseNumber.CreateFromComponents(exponent, BigInteger.One);
25662566

2567+
Assert.ThrowsExactly<OverflowException>(
2568+
() => number.TryFormat(new char[64], out _, "G".AsSpan(), CultureInfo.InvariantCulture),
2569+
$"TryFormat should overflow for an exponent of {exponent}");
2570+
}
2571+
}
2572+
2573+
[TestMethod]
2574+
public void TestTryFormatReturnsFalseWhenOnlyTheDestinationIsTooSmall()
2575+
{
2576+
Span<char> buffer = stackalloc char[64];
2577+
foreach (int exponent in new[] { -1000, 1000 })
2578+
{
2579+
PreciseNumber number = PreciseNumber.CreateFromComponents(exponent, BigInteger.One);
2580+
25672581
bool result = number.TryFormat(buffer, out int charsWritten, "G".AsSpan(), CultureInfo.InvariantCulture);
25682582

25692583
Assert.IsFalse(result, $"TryFormat should fail for an exponent of {exponent}");
25702584
Assert.AreEqual(0, charsWritten);
25712585
}
25722586
}
25732587

2588+
[TestMethod]
2589+
public async Task TestInterpolationAndAppendAtExtremeExponentsFailPromptly()
2590+
{
2591+
// These callers grow their buffer and retry whenever TryFormat returns false, so they
2592+
// only stop if it throws. Each runs on its own task so a regression fails the test
2593+
// rather than hanging it.
2594+
foreach (string text in new[] { "1e-2147483648", "1e2147483647" })
2595+
{
2596+
PreciseNumber number = PreciseNumber.Parse(text, CultureInfo.InvariantCulture);
2597+
Func<string>[] formatters =
2598+
[
2599+
() => $"{number}",
2600+
() => new System.Text.StringBuilder().Append(number).ToString(),
2601+
() => string.Format(CultureInfo.InvariantCulture, "{0}", number),
2602+
];
2603+
2604+
foreach (Func<string> format in formatters)
2605+
{
2606+
Task<string> attempt = Task.Run(format);
2607+
Task finished = await Task.WhenAny(attempt, Task.Delay(TimeSpan.FromSeconds(10))).ConfigureAwait(false);
2608+
2609+
Assert.AreSame(attempt, finished, $"Formatting {text} should not hang");
2610+
await Assert.ThrowsExactlyAsync<OverflowException>(() => attempt).ConfigureAwait(false);
2611+
}
2612+
}
2613+
}
2614+
25742615
[TestMethod]
25752616
public void TestToStringAtExtremeExponentsThrowsOverflow()
25762617
{

‎PreciseNumber/PreciseNumber.cs‎

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -495,21 +495,7 @@ public override int GetHashCode()
495495
public static string ToString(PreciseNumber number, string? format, IFormatProvider? formatProvider)
496496
{
497497
NumberFormatInfo numberFormat = NumberFormatInfo.GetInstance(formatProvider ?? InvariantCulture);
498-
499-
// Digits, plus the padding zeros implied by the exponent, plus the sign, the decimal
500-
// separator and a possible leading "0". Widened, because an exponent of int.MinValue has
501-
// no negation that fits an int and one near int.MaxValue would wrap the sum.
502-
long desiredAlloc = number.SignificantDigits
503-
+ long.Abs(number.Exponent)
504-
+ numberFormat.NegativeSign.Length
505-
+ numberFormat.NumberDecimalSeparator.Length
506-
+ 1;
507-
508-
if (desiredAlloc > Array.MaxLength)
509-
{
510-
throw new OverflowException(
511-
$"An exponent of {number.Exponent.ToString(InvariantCulture)} needs more characters in fixed point notation than a string can hold.");
512-
}
498+
long desiredAlloc = number.GetFormattedLengthBound(numberFormat);
513499

514500
char[]? rentedBuffer = desiredAlloc > MaxStackAllocChars ? ArrayPool<char>.Shared.Rent((int)desiredAlloc) : null;
515501
Span<char> stackBuffer = stackalloc char[MaxStackAllocChars];
@@ -533,6 +519,43 @@ public static string ToString(PreciseNumber number, string? format, IFormatProvi
533519
/// <inheritdoc/>
534520
public string ToString(string? format, IFormatProvider? formatProvider) => ToString(this, format, formatProvider);
535521

522+
/// <summary>
523+
/// Returns an upper bound on the number of characters this number needs in fixed point notation.
524+
/// </summary>
525+
/// <param name="numberFormat">The format supplying the sign and decimal separator.</param>
526+
/// <returns>A length no shorter than the rendered text, and no longer than <see cref="Array.MaxLength"/>.</returns>
527+
/// <exception cref="OverflowException">Thrown when the text would be longer than any string can hold.</exception>
528+
/// <remarks>
529+
/// This is a property of the value, not of any destination, so <see cref="TryFormat"/> throws
530+
/// rather than returning <see langword="false"/>. Callers such as string interpolation and
531+
/// <see cref="System.Text.StringBuilder"/> read <see langword="false"/> as a request to grow the
532+
/// buffer and retry, which no buffer could ever satisfy.
533+
/// </remarks>
534+
private long GetFormattedLengthBound(NumberFormatInfo numberFormat)
535+
{
536+
if (Significand.IsZero)
537+
{
538+
return 1;
539+
}
540+
541+
// Digits, plus the padding zeros implied by the exponent, plus the sign, the decimal
542+
// separator and a possible leading "0". Widened, because an exponent of int.MinValue has
543+
// no negation that fits an int and one near int.MaxValue would wrap the sum.
544+
long bound = SignificantDigits
545+
+ long.Abs(Exponent)
546+
+ numberFormat.NegativeSign.Length
547+
+ numberFormat.NumberDecimalSeparator.Length
548+
+ 1;
549+
550+
if (bound > Array.MaxLength)
551+
{
552+
throw new OverflowException(
553+
$"An exponent of {Exponent.ToString(InvariantCulture)} needs more characters in fixed point notation than a string can hold.");
554+
}
555+
556+
return bound;
557+
}
558+
536559
/// <summary>
537560
/// Returns the absolute value of the current instance.
538561
/// </summary>
@@ -1351,13 +1374,20 @@ public static bool TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out
13511374
TryParse(s, NumberStyles.Any, provider, out result);
13521375

13531376
/// <inheritdoc/>
1377+
/// <exception cref="OverflowException">Thrown when the text would be longer than any string can hold, whatever the size of <paramref name="destination"/>.</exception>
13541378
public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider? provider)
13551379
{
13561380
if (!format.IsEmpty && !format.Equals("G", StringComparison.OrdinalIgnoreCase))
13571381
{
13581382
throw new FormatException();
13591383
}
13601384

1385+
NumberFormatInfo numberFormat = NumberFormatInfo.GetInstance(provider ?? InvariantCulture);
1386+
1387+
// Throws for text no string could hold, so that callers which grow their buffer on false
1388+
// fail promptly instead of retrying without end.
1389+
_ = GetFormattedLengthBound(numberFormat);
1390+
13611391
if (Significand.IsZero)
13621392
{
13631393
charsWritten = 0;
@@ -1371,8 +1401,6 @@ public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan
13711401
return true;
13721402
}
13731403

1374-
NumberFormatInfo numberFormat = NumberFormatInfo.GetInstance(provider ?? InvariantCulture);
1375-
13761404
int digitCount = SignificantDigits;
13771405
char[]? rentedDigits = digitCount > MaxStackAllocChars ? ArrayPool<char>.Shared.Rent(digitCount) : null;
13781406
Span<char> stackDigits = stackalloc char[MaxStackAllocChars];
@@ -1411,9 +1439,8 @@ private bool TryWriteDigits(Span<char> destination, ReadOnlySpan<char> digits, N
14111439
sign = numberFormat.NegativeSign;
14121440
}
14131441

1414-
// Every length below is widened, because the padding zeros an extreme exponent implies can
1415-
// outnumber anything an int holds. Past that point no destination is large enough, so the
1416-
// comparison against its length is what answers, rather than an overflow.
1442+
// TryFormat has already bounded the whole rendering by Array.MaxLength, so the casts back to
1443+
// int below cannot wrap. The lengths stay widened so that this holds without relying on it.
14171444
if (Exponent >= 0)
14181445
{
14191446
long wholeLength = (long)sign.Length + digits.Length + Exponent;

0 commit comments

Comments
 (0)