BigInteger.Parse returns bogus value for exponents above 1000

[mormegil-cz]

When parsing a BigInteger in exponential form, the FX code artificially limits the exponent to 1000. If a larger exponent than that is found, an exponent of 9999 (!!) is substituted instead. See FormatProvider.Number.cs, from line 495 or test it yourself:

Console.WriteLine(BigInteger.Parse("1e+1000", NumberStyles.AllowExponent).ToString("E", CultureInfo.InvariantCulture));
Console.WriteLine(BigInteger.Parse("1e+1001", NumberStyles.AllowExponent).ToString("E", CultureInfo.InvariantCulture));

1.000000E+1000

1.000000E+9999

The explicit coding seems like this was intentional, but it is hard to accept. The behavior is surprising and inconsistent; also, this exponent is not in any way a limit of BigInteger itself, only in the parsing:

Console.WriteLine((BigInteger.Parse("1e+1000", NumberStyles.AllowExponent) * 10).ToString("E", CultureInfo.InvariantCulture));

1.000000E+1001

If BigInteger.Parse needs to have a limitation (e.g. currently it uses an int to store the exponent, so it might need to have a limit around Int32.MaxValue until this gets reimplemented), it should probably rather die with a FormatException or NotSupportedException or somesuch when the limit is reached instead of delivering a completely bogus value.

(This was discovered in a question on StackOverflow.)

[stephentoub]

cc: @mellinoe, @axelheer

[mellinoe]

@mormegil-cz Thanks for the bug report. This behavior seems to be inherited from the .NET Framework. Usually for bugs like this we will be able to:

1. Fix the bug here in corefx such that projects using the NuGet package for System.Runtime.Numerics (which contains BigInteger) will have the fix immediately. This means ".NET Core" projects could get the fix immediately.

2. Triage the issue for the .NET Framework

3. Based on number 2., we will either:

   a. Fix the bug normally in the .NET Framework
   b. Fix the bug behind a "quirk" in the .NET Framework to maintain compatibility. This means older code will not get the fix unless they recompile or retarget to the newer version of the framework.

[mellinoe]

Given that this is a documented behavior quirk and is in line with what .NET Framework does, I'm going to close this. We would still most likely consider a fix for the bug, just taking into account the caveats I described above.

[mormegil-cz]

@mellinoe Where is that “behavior quirk” documented, please?

[mellinoe]

@mormegil-cz Very sorry about that: it actually isn't documented anywhere that I can find. I had mistaken this for another issue.

[mellinoe]

I don't believe this is a crucial enough bug to block our 2.0 release, especially given that it is a pre-existing and inherited issue from .NET Framework. I'm still open to a contribution of someone wants to fix it in that timeframe, though.

[ptoonen]

I'll take on this challenge.

[ptoonen]

After looking at this issue for a bit, I don't see any reason why it would be limited to 9999. Can anyone at Microsoft see any reason why this limit is imposed? I suggest making the max Int32.MaxValue as @mormegil-cz already suggested.

[kellypleahy]

FYI - relevant code is now here: https://github.com/dotnet/corefx/blob/master/src/Common/src/System/Globalization/FormatProvider.Number.cs#L478

[dfederm]

I have a quick and dirty fix for those looking for one. It allows parsing strings like "7.295232207972646e1336" and "7e1336". This does does some manual parsing of the number (gross) and then does multiplication for the exponent part. It's in no way a proper fix though, just works around this bug :)

var pieces = str.Split("e");
var exponent = int.Parse(pieces[1]);
if (exponent > 1000)
{
    var decimalPieces = pieces[0].Split('.');
    var baseNumber = decimalPieces[0];
    if (decimalPieces.Length == 2)
    {
        baseNumber += decimalPieces[1];
        exponent -= decimalPieces[1].Length;
    }

    var bigInt = BigInteger.Parse(baseNumber, NumberStyles.Float);
    bigInt *= BigInteger.Pow(10, exponent);
    return bigInt;
}
else
{
    return BigInteger.Parse(str, NumberStyles.Float);
}

[dfederm]

Also, I think the reason for 999 being the max exponent is mainly die to string length. When you do something like BigInteger.Zero.ToString("e18") then the result is 0.000000000000000000e+000. It looks like when using exponential formatting, it just always uses exactly 3 characters, no matter what the actual number of the exponent is.

I'm not sure that's a great reason for the restriction, but it may explain why it's there in the first place; for consistent string lengths.

[glen3b]

I can still reproduce this issue today. As far as I can tell, the limit is not documented, either in code as a comment or elsewhere; the restriction appears arbitrary to me. My thoughts on what we should do:

• Either remove the limit entirely (in which case we might need to watch out for overflow cases and handle them appropriately)
• Or set the limit closer to int.MaxValue (perhaps under it, e.g. a close power of 10 for convenience in code and/or to help prevent overflowing the NumberBuffer's scale)
• Comment the relevant code (whether it be to leave the current limit or an adjusted limit)
• Document the intended behavior
• Consider throwing an exception if the exponent is out-of-bounds, instead of silently defaulting to an arbitrary value (potentially breaking, see below)

I would certainly like to hear the rationale for this limit, because it appears its purpose is not obvious to anyone here.

Regarding any potential change, my understanding of the breaking change guidelines is that it would be a subtle corner case correction, which means a change isn't rendered impossible on backwards compatibility grounds per se.

I'm willing to work on this issue but I'd need guidance from Microsoft on which course to take.

re @dfederm:
Possibly, but formatting doesn't break with larger exponents (which can still readily be obtained in spite of the parsing issue) or anything:
BigInteger.Pow(10, 5123).ToString("e18", CultureInfo.InvariantCulture) => 1.000000000000000000e+5123

[stephentoub]

@tannergooding, has this been addressed by any of your changes?
cc: @bartonjs

[tannergooding]

No, this has not been addressed. We should likely remove this limitation and allow BigInteger.Parse/BigInteger.ToString to support anything that can be currently represented by BigInteger.

[WhiteBlackGoose]

As far as I can see, the problem is as simple as increasing this from 1000 to int.MaxValue / 10. Is that right?

[Maximys]

@WhiteBlackGoose, I think, your suggestion can fix exactly this issue, but for correctly fixing all cases, we should use another way for parsing. Currently I try to make it.