Edge-Case failures to deserialize DateTime value

[thowoi]

On the following line, it is assumed that the date part of an dateoffset string is always 19 characters long:

https://github.com/rpgmaker/NetJSON/blob/161c22d09c34fe4c1ec61753465c9751b577f0f4/NetJSON/NetJSON.Internals.cs#L437

That is not always true, for example if the year is below 1000 or something like "1-01-01T00:00:00.0-0457". In some cases this results in an exception, like on the following settings case the substring function will get -1 as length parameter:

https://github.com/rpgmaker/NetJSON/blob/161c22d09c34fe4c1ec61753465c9751b577f0f4/NetJSON/NetJSON.Internals.cs#L452

[rpgmaker]

Thanks for calling this out. I will look into it. Would not mind taking a pull request for the fix if you have one already.

Thanks,

[wmjordan]

Due to ISO 8601, the datetime format has to be that long.

The year must be of four digits. You can see this by running

Console.WriteLine(new DateTimeOffset().ToString("s"));

However, it may be appropriated to check the length before Substring--or from performance prospect, avoid Substring as much as possible.

[rpgmaker]

Since it is an edge case could it not be considered as an error and throw invalid date ?

[thowoi]

That sounds reasonable, but then the DateToISOFormat function could be changed. On this line the year is added without leading zeros, if below 1000:

https://github.com/rpgmaker/NetJSON/blob/161c22d09c34fe4c1ec61753465c9751b577f0f4/NetJSON/NetJSON.Internals.cs#L1063

Maybe something like this:

var value = (_cachedDateStringBuilder ?? (_cachedDateStringBuilder = new StringBuilder(25)))
                .Clear()
//-----Added part-----              
                .Append(year < 10 ? "0" : string.Empty)
                .Append(year < 100 ? "0" : string.Empty)
                .Append(year < 1000 ? "0" : string.Empty)
//-----End-----
                .Append(IntToStr(year))
                .Append('-')
                .Append(month < 10 ? "0" : string.Empty)
                .Append(IntToStr(month))
            .Append('-').Append(day < 10 ? "0" : string.Empty).Append(IntToStr(day)).Append('T').Append(hour < 10 ? "0" : string.Empty).Append(IntToStr(hour)).Append(':')
            .Append(minute < 10 ? "0" : string.Empty).Append(IntToStr(minute)).Append(':')
            .Append(second < 10 ? "0" : string.Empty).Append(IntToStr(second)).Append('.')
            .Append(IntToStr(totalSeconds));

By the way, thanks for this library and all the work!

[wmjordan]

The above method could be optimized.

Very seldom the year part will have less than 4 digits, thus there should not be so many code branches there.

Since ISO date-time strings are fixed-length, we have an even faster way than using StringBuilder.

We can pre-allocate a string with that fixed length and use a pointer to patch numeric values onto the string--Yes, strings are actually mutable in unsafe environment.

Since month, day, hour, minute and second parts are all two-digits' long, we can pre-calculate the literal form from 00 to 99, cache them to an array and patch the appropriated value onto the string. For the year part, it is also possible to divide it by 100 to get two 2 two-digit numbers, then adopt the above method like months. Or we can utilize the existing four-digit values in the IntUtility.

[rpgmaker]

I agree with @wmjordan. The frequency of having less than 4 is rather almost nonexistent due to the standard of iso. @thowoi , Could you consider submitting a PR with your changes including @wmjordan recommendations.

What is the behavior when running the dates against other json serializer? Don't remember if you mentioned it.

[rpgmaker]

@thowoi , should I close this issue?

[rpgmaker]

Closing. Please reopen if necessary. thanks

[rpgmaker]

@thowoi , I will close this issue for now unless you deem it necessary to reopen