Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix TimeSpan parsing #21968

Merged
merged 2 commits into from
Jan 14, 2019
Merged

Fix TimeSpan parsing #21968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8555ca
Fix TimeSpan parsing
tarekgh Jan 11, 2019
99cf244
Temporary disabling the failed CI tests
tarekgh Jan 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 46 additions & 39 deletions src/System.Private.CoreLib/shared/System/Globalization/TimeSpanParse.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
//
// Standard Format:
// -=-=-=-=-=-=-=-
// "c": Constant format. [-][d'.']hh':'mm':'ss['.'fffffff]
// "c": Constant format. [-][d'.']hh':'mm':'ss['.'fffffff]
// Not culture sensitive. Default format (and null/empty format string) map to this format.
//
// "g": General format, short: [-][d':']h':'mm':'ss'.'FFFFFFF
// "g": General format, short: [-][d':']h':'mm':'ss'.'FFFFFFF
// Only print what's needed. Localized (if you want Invariant, pass in Invariant).
// The fractional seconds separator is localized, equal to the culture's DecimalSeparator.
//
Expand Down Expand Up @@ -43,8 +43,8 @@
// - For multi-letter formats "TryParseByFormat" is called
// - TryParseByFormat uses helper methods (ParseExactLiteral, ParseExactDigits, etc)
// which drive the underlying TimeSpanTokenizer. However, unlike standard formatting which
// operates on whole-tokens, ParseExact operates at the character-level. As such,
// TimeSpanTokenizer.NextChar and TimeSpanTokenizer.BackOne() are called directly.
// operates on whole-tokens, ParseExact operates at the character-level. As such,
// TimeSpanTokenizer.NextChar and TimeSpanTokenizer.BackOne() are called directly.
//
////////////////////////////////////////////////////////////////////////////

Expand Down Expand Up @@ -104,19 +104,50 @@ public TimeSpanToken(TTT type, int number, int leadingZeroes, ReadOnlySpan<char>
_sep = separator;
}

public bool IsInvalidFraction()
public bool NormalizeAndValidateFraction()
{
Debug.Assert(_ttt == TTT.Num);
Debug.Assert(_num > -1);

if (_num > MaxFraction || _zeroes > MaxFractionDigits)
if (_num == 0)
return true;

if (_num == 0 || _zeroes == 0)
if (_zeroes == 0 && _num > MaxFraction)
return false;

// num > 0 && zeroes > 0 && num <= maxValue && zeroes <= maxPrecision
return _num >= MaxFraction / Pow10(_zeroes - 1);
int totalDigitsCount = ((int) Math.Floor(Math.Log10(_num))) + 1 + _zeroes;

if (totalDigitsCount == MaxFractionDigits)
{
// Already normalized. no more action needed
// .9999999 normalize to 9,999,999 ticks
// .0000001 normalize to 1 ticks
return true;
}

if (totalDigitsCount < MaxFractionDigits)
{
// normalize the fraction to the 7-digits
// .999999 normalize to 9,999,990 ticks
// .99999 normalize to 9,999,900 ticks
// .000001 normalize to 10 ticks
// .1 normalize to 1,000,000 ticks

_num *= (int) Pow10(MaxFractionDigits - totalDigitsCount);
return true;
}

// totalDigitsCount is greater then MaxFractionDigits, we'll need to do the rounding to 7-digits length
// .00000001 normalized to 0 ticks
// .00000005 normalized to 1 ticks
// .09999999 normalize to 1,000,000 ticks
// .099999999 normalize to 1,000,000 ticks

Debug.Assert(_zeroes > 0); // Already validated that in the condition _zeroes == 0 && _num > MaxFraction
_num = (int) Math.Round((double)_num / Pow10(totalDigitsCount - MaxFractionDigits), MidpointRounding.AwayFromZero);
Debug.Assert(_num < MaxFraction);

return true;
}
}

Expand Down Expand Up @@ -184,7 +215,7 @@ internal TimeSpanToken GetNextToken()
}

num = num * 10 + digit;
if ((num & 0xF0000000) != 0)
if ((num & 0xF0000000) != 0) // Max limit we can support 268435455 which is FFFFFFF
{
return new TimeSpanToken(TTT.NumOverflow);
}
Expand Down Expand Up @@ -557,7 +588,7 @@ private static bool TryTimeToTicks(bool positive, TimeSpanToken days, TimeSpanTo
hours._num > MaxHours ||
minutes._num > MaxMinutes ||
seconds._num > MaxSeconds ||
fraction.IsInvalidFraction())
!fraction.NormalizeAndValidateFraction())
{
result = 0;
return false;
Expand All @@ -570,31 +601,7 @@ private static bool TryTimeToTicks(bool positive, TimeSpanToken days, TimeSpanTo
return false;
}

// Normalize the fraction component
//
// string representation => (zeroes,num) => resultant fraction ticks
// --------------------- ------------ ------------------------
// ".9999999" => (0,9999999) => 9,999,999 ticks (same as constant maxFraction)
// ".1" => (0,1) => 1,000,000 ticks
// ".01" => (1,1) => 100,000 ticks
// ".001" => (2,1) => 10,000 ticks
long f = fraction._num;
if (f != 0)
{
long lowerLimit = InternalGlobalizationHelper.TicksPerTenthSecond;
if (fraction._zeroes > 0)
{
long divisor = Pow10(fraction._zeroes);
lowerLimit = lowerLimit / divisor;
}

while (f < lowerLimit)
{
f *= 10;
}
}

result = ticks * TimeSpan.TicksPerMillisecond + f;
result = ticks * TimeSpan.TicksPerMillisecond + fraction._num;
if (positive && result < 0)
{
result = 0;
Expand Down Expand Up @@ -1338,7 +1345,7 @@ private static bool TryParseByFormat(ReadOnlySpan<char> input, ReadOnlySpan<char

case '%':
// Optional format character.
// For example, format string "%d" will print day
// For example, format string "%d" will print day
// Most of the cases, "%" can be ignored.
nextFormatChar = DateTimeFormat.ParseNextChar(format, i);

Expand Down Expand Up @@ -1455,7 +1462,7 @@ private static bool ParseExactLiteral(ref TimeSpanTokenizer tokenizer, StringBui

/// <summary>
/// Parses the "c" (constant) format. This code is 100% identical to the non-globalized v1.0-v3.5 TimeSpan.Parse() routine
/// and exists for performance/appcompat with legacy callers who cannot move onto the globalized Parse overloads.
/// and exists for performance/appcompat with legacy callers who cannot move onto the globalized Parse overloads.
/// </summary>
private static bool TryParseTimeSpanConstant(ReadOnlySpan<char> input, ref TimeSpanResult result) =>
new StringParser().TryParse(input, ref result);
Expand Down Expand Up @@ -1628,7 +1635,7 @@ internal bool ParseTime(out long time, ref TimeSpanResult result)
if (_ch == ':')
{
NextChar();

// allow seconds with the leading zero
if (_ch != '.')
{
Expand Down
16 changes: 16 additions & 0 deletions tests/CoreFX/CoreFX.issues.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,22 @@
{
"name": "System.Tests.ArrayTests.Copy",
"reason": "Needs updates for XUnit 2.4"
},
{
"name": "System.Tests.TimeSpanTests.Parse_Invalid",
"reason": "Temporary disabling till merging the PR https://github.com/dotnet/corefx/pull/34561"
},
{
"name": "System.Tests.TimeSpanTests.Parse_Span",
"reason": "Temporary disabling till merging the PR https://github.com/dotnet/corefx/pull/34561"
},
{
"name": "System.Tests.TimeSpanTests.Parse_Span_Invalid",
"reason": "Temporary disabling till merging the PR https://github.com/dotnet/corefx/pull/34561"
},
{
"name": "System.Tests.TimeSpanTests.Parse",
"reason": "Temporary disabling till merging the PR https://github.com/dotnet/corefx/pull/34561"
}
]
}
Expand Down