Skip to content

Commit

Permalink
[NRBF] Don't use Unsafe.As when decoding DateTime(s) (dotnet#105749)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamsitnik committed Sep 13, 2024
1 parent 9bff9c5 commit 2d93a7d
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ private static List<T> DecodeFromNonSeekableStream(BinaryReader reader, int coun
}
else if (typeof(T) == typeof(DateTime))
{
values.Add((T)(object)Utils.BinaryReaderExtensions.CreateDateTimeFromData(reader.ReadInt64()));
values.Add((T)(object)Utils.BinaryReaderExtensions.CreateDateTimeFromData(reader.ReadUInt64()));
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ private static SerializationRecord DecodeMemberPrimitiveTypedRecord(BinaryReader
PrimitiveType.Single => new MemberPrimitiveTypedRecord<float>(reader.ReadSingle()),
PrimitiveType.Double => new MemberPrimitiveTypedRecord<double>(reader.ReadDouble()),
PrimitiveType.Decimal => new MemberPrimitiveTypedRecord<decimal>(decimal.Parse(reader.ReadString(), CultureInfo.InvariantCulture)),
PrimitiveType.DateTime => new MemberPrimitiveTypedRecord<DateTime>(Utils.BinaryReaderExtensions.CreateDateTimeFromData(reader.ReadInt64())),
PrimitiveType.DateTime => new MemberPrimitiveTypedRecord<DateTime>(Utils.BinaryReaderExtensions.CreateDateTimeFromData(reader.ReadUInt64())),
// String is handled with a record, never on it's own
_ => new MemberPrimitiveTypedRecord<TimeSpan>(new TimeSpan(reader.ReadInt64())),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,29 +75,30 @@ ulong value when TypeNameMatches(typeof(UIntPtr)) => Create(new UIntPtr(value)),
_ => this
};
}
else if (HasMember("_ticks") && MemberValues[0] is long ticks && TypeNameMatches(typeof(TimeSpan)))
else if (HasMember("_ticks") && GetRawValue("_ticks") is long ticks && TypeNameMatches(typeof(TimeSpan)))
{
return Create(new TimeSpan(ticks));
}
}
else if (MemberValues.Count == 2
&& HasMember("ticks") && HasMember("dateData")
&& MemberValues[0] is long value && MemberValues[1] is ulong
&& GetRawValue("ticks") is long && GetRawValue("dateData") is ulong dateData
&& TypeNameMatches(typeof(DateTime)))
{
return Create(Utils.BinaryReaderExtensions.CreateDateTimeFromData(value));
return Create(Utils.BinaryReaderExtensions.CreateDateTimeFromData(dateData));
}
else if(MemberValues.Count == 4
else if (MemberValues.Count == 4
&& HasMember("lo") && HasMember("mid") && HasMember("hi") && HasMember("flags")
&& MemberValues[0] is int && MemberValues[1] is int && MemberValues[2] is int && MemberValues[3] is int
&& GetRawValue("lo") is int lo && GetRawValue("mid") is int mid
&& GetRawValue("hi") is int hi && GetRawValue("flags") is int flags
&& TypeNameMatches(typeof(decimal)))
{
int[] bits =
[
GetInt32("lo"),
GetInt32("mid"),
GetInt32("hi"),
GetInt32("flags")
lo,
mid,
hi,
flags
];

return Create(new decimal(bits));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,18 @@

using System.Globalization;
using System.IO;
using System.Reflection;
using System.Reflection.Metadata;
using System.Runtime.CompilerServices;
using System.Runtime.Serialization;
using System.Threading;

namespace System.Formats.Nrbf.Utils;

internal static class BinaryReaderExtensions
{
private static object? s_baseAmbiguousDstDateTime;

internal static BinaryArrayType ReadArrayType(this BinaryReader reader)
{
byte arrayType = reader.ReadByte();
Expand Down Expand Up @@ -70,36 +74,70 @@ internal static object ReadPrimitiveValue(this BinaryReader reader, PrimitiveTyp
PrimitiveType.Single => reader.ReadSingle(),
PrimitiveType.Double => reader.ReadDouble(),
PrimitiveType.Decimal => decimal.Parse(reader.ReadString(), CultureInfo.InvariantCulture),
PrimitiveType.DateTime => CreateDateTimeFromData(reader.ReadInt64()),
PrimitiveType.DateTime => CreateDateTimeFromData(reader.ReadUInt64()),
_ => new TimeSpan(reader.ReadInt64()),
};

// TODO: fix https://github.com/dotnet/runtime/issues/102826
/// <summary>
/// Creates a <see cref="DateTime"/> object from raw data with validation.
/// </summary>
/// <exception cref="SerializationException"><paramref name="data"/> was invalid.</exception>
internal static DateTime CreateDateTimeFromData(long data)
/// <exception cref="SerializationException"><paramref name="dateData"/> was invalid.</exception>
internal static DateTime CreateDateTimeFromData(ulong dateData)
{
// Copied from System.Runtime.Serialization.Formatters.Binary.BinaryParser

// Use DateTime's public constructor to validate the input, but we
// can't return that result as it strips off the kind. To address
// that, store the value directly into a DateTime via an unsafe cast.
// See BinaryFormatterWriter.WriteDateTime for details.
ulong ticks = dateData & 0x3FFFFFFF_FFFFFFFFUL;
DateTimeKind kind = (DateTimeKind)(dateData >> 62);

try
{
const long TicksMask = 0x3FFFFFFFFFFFFFFF;
_ = new DateTime(data & TicksMask);
return ((uint)kind <= (uint)DateTimeKind.Local) ? new DateTime((long)ticks, kind) : CreateFromAmbiguousDst(ticks);
}
catch (ArgumentException ex)
{
// Bad data
throw new SerializationException(ex.Message, ex);
}

return Unsafe.As<long, DateTime>(ref data);
[MethodImpl(MethodImplOptions.NoInlining)]
static DateTime CreateFromAmbiguousDst(ulong ticks)
{
// There's no public API to create a DateTime from an ambiguous DST, and we
// can't use private reflection to access undocumented .NET Framework APIs.
// However, the ISerializable pattern *is* a documented protocol, so we can
// use DateTime's serialization ctor to create a zero-tick "ambiguous" instance,
// then keep reusing it as the base to which we can add our tick offsets.

if (s_baseAmbiguousDstDateTime is not DateTime baseDateTime)
{
#pragma warning disable SYSLIB0050 // Type or member is obsolete
SerializationInfo si = new(typeof(DateTime), new FormatterConverter());
// We don't know the value of "ticks", so we don't specify it.
// If the code somehow runs on a very old runtime that does not know the concept of "dateData"
// (it should not be possible as the library targets .NET Standard 2.0)
// the ctor is going to throw rather than silently return an invalid value.
si.AddValue("dateData", 0xC0000000_00000000UL); // new value (serialized as ulong)

#if NET
baseDateTime = CallPrivateSerializationConstructor(si, new StreamingContext(StreamingContextStates.All));
#else
ConstructorInfo ci = typeof(DateTime).GetConstructor(
BindingFlags.Instance | BindingFlags.NonPublic,
binder: null,
new Type[] { typeof(SerializationInfo), typeof(StreamingContext) },
modifiers: null);

baseDateTime = (DateTime)ci.Invoke(new object[] { si, new StreamingContext(StreamingContextStates.All) });
#endif

#pragma warning restore SYSLIB0050 // Type or member is obsolete
Volatile.Write(ref s_baseAmbiguousDstDateTime, baseDateTime); // it's ok if two threads race here
}

return baseDateTime.AddTicks((long)ticks);
}

#if NET
[UnsafeAccessor(UnsafeAccessorKind.Constructor)]
extern static DateTime CallPrivateSerializationConstructor(SerializationInfo si, StreamingContext ct);
#endif
}

internal static bool? IsDataAvailable(this BinaryReader reader, long requiredBytes)
Expand Down
43 changes: 42 additions & 1 deletion src/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.IO;
using System.Collections.Generic;
using System.IO;
using System.Runtime.Serialization.Formatters;
using System.Runtime.Serialization.Formatters.Binary;
using Microsoft.DotNet.XUnitExtensions;
Expand Down Expand Up @@ -103,4 +104,44 @@ public void FormatterTypeStyleOtherThanTypesAlwaysAreNotSupportedByDesign(Format

Assert.Throws<NotSupportedException>(() => NrbfDecoder.Decode(ms));
}

public static IEnumerable<object[]> CanReadAllKindsOfDateTimes_Arguments
{
get
{
yield return new object[] { new DateTime(1990, 11, 24, 0, 0, 0, DateTimeKind.Local) };
yield return new object[] { new DateTime(1990, 11, 25, 0, 0, 0, DateTimeKind.Utc) };
yield return new object[] { new DateTime(1990, 11, 26, 0, 0, 0, DateTimeKind.Unspecified) };
}
}

[Theory]
[MemberData(nameof(CanReadAllKindsOfDateTimes_Arguments))]
public void CanReadAllKindsOfDateTimes_DateTimeIsTheRootRecord(DateTime input)
{
using MemoryStream stream = Serialize(input);

PrimitiveTypeRecord<DateTime> dateTimeRecord = (PrimitiveTypeRecord<DateTime>)NrbfDecoder.Decode(stream);

Assert.Equal(input.Ticks, dateTimeRecord.Value.Ticks);
Assert.Equal(input.Kind, dateTimeRecord.Value.Kind);
}

[Serializable]
public class ClassWithDateTime
{
public DateTime Value;
}

[Theory]
[MemberData(nameof(CanReadAllKindsOfDateTimes_Arguments))]
public void CanReadAllKindsOfDateTimes_DateTimeIsMemberOfTheRootRecord(DateTime input)
{
using MemoryStream stream = Serialize(new ClassWithDateTime() { Value = input });

ClassRecord classRecord = NrbfDecoder.DecodeClassRecord(stream);

Assert.Equal(input.Ticks, classRecord.GetDateTime(nameof(ClassWithDateTime.Value)).Ticks);
Assert.Equal(input.Kind, classRecord.GetDateTime(nameof(ClassWithDateTime.Value)).Kind);
}
}

0 comments on commit 2d93a7d

Please # to comment.