From 66020065425982c10031ebbbb91d1f9d4b4f3ccc Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 14 Jun 2024 18:06:52 +0200 Subject: [PATCH 1/5] add failing tests --- .../tests/InvalidInputTests.cs | 39 +++++++++++++++ .../System.Formats.Nrbf/tests/ReadTests.cs | 47 +++++++++++++++++++ .../tests/System.Formats.Nrbf.Tests.csproj | 1 + 3 files changed, 87 insertions(+) diff --git a/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs b/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs index bc134350eb7c92..98c12999092cc5 100644 --- a/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs +++ b/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs @@ -478,4 +478,43 @@ public void ThrowsOnInvalidArrayType() stream.Position = 0; Assert.Throws(() => NrbfDecoder.Decode(stream)); } + + public static IEnumerable ThrowsForOrphanedRecord_Args() + { + SerializationRecordType[] supported = + { + SerializationRecordType.BinaryObjectString, + SerializationRecordType.ArraySingleString, + SerializationRecordType.MemberPrimitiveTyped, + SerializationRecordType.ArraySinglePrimitive, + SerializationRecordType.SystemClassWithMembersAndTypes, + SerializationRecordType.ClassWithMembersAndTypes + }; + + for (int i = 0; i < supported.Length; i++) + { + for (int j = 0; j < supported.Length; j++) + { + yield return new object[] { supported[i], supported[j] }; + } + } + } + + [Theory] + [MemberData(nameof(ThrowsForOrphanedRecord_Args))] + public void ThrowsForOrphanedRecord(SerializationRecordType root, SerializationRecordType orphaned) + { + int objectId = 1; + using MemoryStream stream = new(); + BinaryWriter writer = new(stream, Encoding.UTF8); + + WriteSerializedStreamHeader(writer); + WriteValidRecord(writer, root, ref objectId); + WriteValidRecord(writer, orphaned, ref objectId); + + writer.Write((byte)SerializationRecordType.MessageEnd); + + stream.Position = 0; + Assert.Throws(() => NrbfDecoder.Decode(stream)); + } } diff --git a/src/libraries/System.Formats.Nrbf/tests/ReadTests.cs b/src/libraries/System.Formats.Nrbf/tests/ReadTests.cs index b9bee7d881a695..38c56439c7caaf 100644 --- a/src/libraries/System.Formats.Nrbf/tests/ReadTests.cs +++ b/src/libraries/System.Formats.Nrbf/tests/ReadTests.cs @@ -60,4 +60,51 @@ protected static void WriteBinaryLibrary(BinaryWriter writer, int objectId, stri writer.Write(objectId); writer.Write(libraryName); } + + protected static void WriteValidRecord(BinaryWriter writer, SerializationRecordType recordType, ref int objectId) + { + const int LibraryId = 12345; + if (recordType == SerializationRecordType.ClassWithMembersAndTypes) + { + WriteBinaryLibrary(writer, LibraryId, "libName"); + } + + writer.Write((byte)recordType); + writer.Write(objectId++); + + if (recordType == SerializationRecordType.BinaryObjectString) + { + writer.Write("aString"); + } + else if (recordType == SerializationRecordType.ArraySingleString) + { + writer.Write(2); // array length + WriteValidRecord(writer, SerializationRecordType.BinaryObjectString, ref objectId); + WriteValidRecord(writer, SerializationRecordType.BinaryObjectString, ref objectId); + } + else if (recordType == SerializationRecordType.MemberPrimitiveTyped) + { + writer.Write((byte)PrimitiveType.Boolean); + writer.Write(true); + } + else if (recordType == SerializationRecordType.ArraySinglePrimitive) + { + writer.Write(3); // array length + writer.Write((byte)PrimitiveType.Int32); + writer.Write(1); + writer.Write(2); + writer.Write(3); + } + else if (recordType == SerializationRecordType.SystemClassWithMembersAndTypes) + { + writer.Write("TypeName"); + writer.Write(0); // member count + } + else if (recordType == SerializationRecordType.ClassWithMembersAndTypes) + { + writer.Write("TypeName"); + writer.Write(0); // member count + writer.Write(LibraryId); + } + } } diff --git a/src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj b/src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj index 2acfeb3e162ba0..e3efe1b7f4c1cb 100644 --- a/src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj +++ b/src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj @@ -7,6 +7,7 @@ + From 5f66851dc094f9234339bbfb348d4dc773760d33 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 14 Jun 2024 18:14:49 +0200 Subject: [PATCH 2/5] first attempt that does not work because we can not make any assumptions about the order of referenced records --- .../src/System/Formats/Nrbf/MemberTypeInfo.cs | 6 +++-- .../src/System/Formats/Nrbf/NextInfo.cs | 6 ++--- .../src/System/Formats/Nrbf/NrbfDecoder.cs | 27 ++++++++++++++++--- .../Formats/Nrbf/SerializationRecordId.cs | 2 ++ 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs index 9e6c2445877e0b..7e8d7efe5da2bd 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs @@ -74,6 +74,8 @@ internal static MemberTypeInfo Decode(BinaryReader reader, int count, PayloadOpt | AllowedRecordTypes.ObjectNull | AllowedRecordTypes.MemberReference; const AllowedRecordTypes ObjectArray = AllowedRecordTypes.ArraySingleObject | AllowedRecordTypes.ObjectNull | AllowedRecordTypes.MemberReference; + const AllowedRecordTypes NonPrimitiveArray = AllowedRecordTypes.BinaryArray + | AllowedRecordTypes.ObjectNull | AllowedRecordTypes.MemberReference; // Every string can be a string, a null or a reference (to a string) const AllowedRecordTypes Strings = AllowedRecordTypes.BinaryObjectString @@ -95,8 +97,8 @@ internal static MemberTypeInfo Decode(BinaryReader reader, int count, PayloadOpt BinaryType.Object => (AllowedRecordTypes.AnyObject, default), BinaryType.StringArray => (StringArray, default), BinaryType.PrimitiveArray => (PrimitiveArray, default), - BinaryType.Class => (NonSystemClass, default), - BinaryType.SystemClass => (SystemClass, default), + BinaryType.Class => (((ClassTypeInfo)additionalInfo!).TypeName.IsArray ? NonPrimitiveArray : NonSystemClass, default), + BinaryType.SystemClass => (((TypeName)additionalInfo!).IsArray ? NonPrimitiveArray : SystemClass, default), _ => (ObjectArray, default) }; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NextInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NextInfo.cs index a01a25e60047a3..cb96aa591183d6 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NextInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NextInfo.cs @@ -6,10 +6,10 @@ namespace System.Formats.Nrbf; -[DebuggerDisplay("{Parent.RecordType}, {Allowed}, {PrimitiveType}")] +[DebuggerDisplay("{Parent?.RecordType}, {Allowed}, {PrimitiveType}")] internal readonly struct NextInfo { - internal NextInfo(AllowedRecordTypes allowed, SerializationRecord parent, + internal NextInfo(AllowedRecordTypes allowed, SerializationRecord? parent, Stack stack, PrimitiveType primitiveType = default) { Allowed = allowed; @@ -20,7 +20,7 @@ internal NextInfo(AllowedRecordTypes allowed, SerializationRecord parent, internal AllowedRecordTypes Allowed { get; } - internal SerializationRecord Parent { get; } + internal SerializationRecord? Parent { get; } internal Stack Stack { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs index 426755ee7fe87e..3fa02c67086b6e 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs @@ -8,6 +8,7 @@ using System.Formats.Nrbf.Utils; using System.Text; using System.Runtime.Serialization; +using System.Diagnostics; namespace System.Formats.Nrbf; @@ -162,7 +163,7 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op // Everything has to start with a header var header = (SerializedStreamHeaderRecord)DecodeNext(reader, recordMap, AllowedRecordTypes.SerializedStreamHeader, options, out _); // and can be followed by any Object, BinaryLibrary and a MessageEnd. - const AllowedRecordTypes Allowed = AllowedRecordTypes.AnyObject + AllowedRecordTypes allowed = AllowedRecordTypes.AnyObject | AllowedRecordTypes.BinaryLibrary | AllowedRecordTypes.MessageEnd; SerializationRecordType recordType; @@ -186,24 +187,44 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op } while (nextRecord is BinaryLibraryRecord); + if (nextRecord is MemberReferenceRecord) + { + // We were supposed to decode a record of specific type or a reference to it. + // Since a reference was decoded, it means that we still need to read the record. + // Reference can not point to a null. + AllowedRecordTypes allowedRecord = nextInfo.Allowed + & ~AllowedRecordTypes.MemberReference + & ~AllowedRecordTypes.Nulls; + // The parent is set to null, because we don't want to add it to member values. + readStack.Push(new NextInfo(allowedRecord, parent: null, readStack)); + } + // Handle it: // - add to the parent records list, // - push next info if there are remaining nested records to read. - nextInfo.Parent.HandleNextRecord(nextRecord, nextInfo); + nextInfo.Parent?.HandleNextRecord(nextRecord, nextInfo); // Push on the top of the stack the first nested record of last read record, // so it gets read as next record. PushFirstNestedRecordInfo(nextRecord, readStack); } else { + Debug.Assert(nextInfo.Parent is not null); + object value = reader.ReadPrimitiveValue(nextInfo.PrimitiveType); nextInfo.Parent.HandleNextValue(value, nextInfo); } } - nextRecord = DecodeNext(reader, recordMap, Allowed, options, out recordType); + nextRecord = DecodeNext(reader, recordMap, allowed, options, out recordType); PushFirstNestedRecordInfo(nextRecord, readStack); + + // Only a binary library can be followed by something that is not a MessageEnd. + if (readStack.Count == 0 && recordType != SerializationRecordType.BinaryLibrary) + { + allowed = AllowedRecordTypes.MessageEnd; + } } while (recordType != SerializationRecordType.MessageEnd); diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecordId.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecordId.cs index 5df884c2d6c564..d24e39b757d3ce 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecordId.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecordId.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Formats.Nrbf.Utils; using System.IO; using System.IO.Hashing; @@ -16,6 +17,7 @@ namespace System.Formats.Nrbf; /// /// The ID of . /// +[DebuggerDisplay("{_id}")] public readonly struct SerializationRecordId : IEquatable { #pragma warning disable CS0649 // the default value is used on purpose From a93760f03a8a6187bfef58a6b6751e2942472d13 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 17 Jun 2024 15:51:48 +0200 Subject: [PATCH 3/5] delay the type check for references until we read the actual record --- .../System/Formats/Nrbf/AllowedRecordType.cs | 3 + .../Formats/Nrbf/MemberReferenceRecord.cs | 2 + .../src/System/Formats/Nrbf/MemberTypeInfo.cs | 27 ++++- .../src/System/Formats/Nrbf/NextInfo.cs | 6 +- .../src/System/Formats/Nrbf/NrbfDecoder.cs | 112 +++++++++--------- .../src/System/Formats/Nrbf/RecordMap.cs | 71 ++++++++--- .../Formats/Nrbf/SerializationRecord.cs | 2 +- 7 files changed, 146 insertions(+), 77 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/AllowedRecordType.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/AllowedRecordType.cs index 8a3b3046105551..126a08f1d929e5 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/AllowedRecordType.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/AllowedRecordType.cs @@ -35,4 +35,7 @@ internal enum AllowedRecordTypes : uint | BinaryObjectString | MemberReference | ObjectNull, + + // The referenced record can point to any object except of a null or another reference. + ReferencedRecord = AnyObject & ~(ObjectNull | MemberReference) } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs index 162cf0b1d5c577..22145c9e17530c 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs @@ -25,6 +25,8 @@ private MemberReferenceRecord(SerializationRecordId reference, RecordMap recordM internal SerializationRecordId Reference { get; } + internal AllowedRecordTypes ReferencedRecordType { get; set; } + private RecordMap RecordMap { get; } // MemberReferenceRecord has no Id, which makes it impossible to create a cycle diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs index 7e8d7efe5da2bd..3e1d34eb37421f 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs @@ -98,9 +98,32 @@ internal static MemberTypeInfo Decode(BinaryReader reader, int count, PayloadOpt BinaryType.StringArray => (StringArray, default), BinaryType.PrimitiveArray => (PrimitiveArray, default), BinaryType.Class => (((ClassTypeInfo)additionalInfo!).TypeName.IsArray ? NonPrimitiveArray : NonSystemClass, default), - BinaryType.SystemClass => (((TypeName)additionalInfo!).IsArray ? NonPrimitiveArray : SystemClass, default), - _ => (ObjectArray, default) + BinaryType.SystemClass => (MapSystemClassTypeName((TypeName)additionalInfo!), default), + _ => (ObjectArray, default), }; + + static AllowedRecordTypes MapSystemClassTypeName(TypeName typeName) + { + if (!typeName.IsArray) + { + return SystemClass; + } + + if (typeName.IsSZArray) + { + TypeName elementTypeName = typeName.GetElementType(); + if (SerializationRecord.Matches(typeof(decimal), elementTypeName) + || SerializationRecord.Matches(typeof(TimeSpan), elementTypeName) + || SerializationRecord.Matches(typeof(DateTime), elementTypeName)) + { + // BinaryFormatter should use BinaryType.PrimitiveArray for these 3 types, + // but it uses BinaryType.SystemClass and we need this workaround. + return PrimitiveArray; + } + } + + return NonPrimitiveArray; + } } internal bool ShouldBeRepresentedAsArrayOfClassRecords() diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NextInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NextInfo.cs index cb96aa591183d6..a01a25e60047a3 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NextInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NextInfo.cs @@ -6,10 +6,10 @@ namespace System.Formats.Nrbf; -[DebuggerDisplay("{Parent?.RecordType}, {Allowed}, {PrimitiveType}")] +[DebuggerDisplay("{Parent.RecordType}, {Allowed}, {PrimitiveType}")] internal readonly struct NextInfo { - internal NextInfo(AllowedRecordTypes allowed, SerializationRecord? parent, + internal NextInfo(AllowedRecordTypes allowed, SerializationRecord parent, Stack stack, PrimitiveType primitiveType = default) { Allowed = allowed; @@ -20,7 +20,7 @@ internal NextInfo(AllowedRecordTypes allowed, SerializationRecord? parent, internal AllowedRecordTypes Allowed { get; } - internal SerializationRecord? Parent { get; } + internal SerializationRecord Parent { get; } internal Stack Stack { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs index 3fa02c67086b6e..becf5d67769db0 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs @@ -160,11 +160,13 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op Stack readStack = new(); RecordMap recordMap = new(); - // Everything has to start with a header + // Every NRBF payload has to start with a header var header = (SerializedStreamHeaderRecord)DecodeNext(reader, recordMap, AllowedRecordTypes.SerializedStreamHeader, options, out _); - // and can be followed by any Object, BinaryLibrary and a MessageEnd. - AllowedRecordTypes allowed = AllowedRecordTypes.AnyObject - | AllowedRecordTypes.BinaryLibrary | AllowedRecordTypes.MessageEnd; + + // All we know about root is its Id. It can be any Object or BinaryLibrary, but not a reference. + AllowedRecordTypes allowed = (AllowedRecordTypes.AnyObject & ~AllowedRecordTypes.MemberReference) | AllowedRecordTypes.BinaryLibrary; + SerializationRecord rootRecord = DecodeNext(reader, recordMap, allowed, options, out _); + PushFirstNestedRecordInfo(rootRecord, readStack); SerializationRecordType recordType; SerializationRecord nextRecord; @@ -177,54 +179,46 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op if (nextInfo.Allowed != AllowedRecordTypes.None) { // Decode the next Record - do - { - nextRecord = DecodeNext(reader, recordMap, nextInfo.Allowed, options, out _); - // BinaryLibrary often precedes class records. - // It has been already added to the RecordMap and it must not be added - // to the array record, so simply read next record. - // It's possible to read multiple BinaryLibraryRecord in a row, hence the loop. - } - while (nextRecord is BinaryLibraryRecord); + nextRecord = DecodeNext(reader, recordMap, nextInfo.Allowed, options, out SerializationRecordType nextRecordType); - if (nextRecord is MemberReferenceRecord) + if (nextRecordType == SerializationRecordType.MemberReference) { // We were supposed to decode a record of specific type or a reference to it. - // Since a reference was decoded, it means that we still need to read the record. - // Reference can not point to a null. - AllowedRecordTypes allowedRecord = nextInfo.Allowed - & ~AllowedRecordTypes.MemberReference - & ~AllowedRecordTypes.Nulls; - // The parent is set to null, because we don't want to add it to member values. - readStack.Push(new NextInfo(allowedRecord, parent: null, readStack)); + // Since a reference was decoded and we don't know when the referenced record will be provided. + // We just store the allowed record type and are going to check it later. + ((MemberReferenceRecord)nextRecord).ReferencedRecordType = nextInfo.Allowed + & ~(AllowedRecordTypes.MemberReference | AllowedRecordTypes.Nulls); } // Handle it: // - add to the parent records list, // - push next info if there are remaining nested records to read. - nextInfo.Parent?.HandleNextRecord(nextRecord, nextInfo); + nextInfo.Parent.HandleNextRecord(nextRecord, nextInfo); // Push on the top of the stack the first nested record of last read record, // so it gets read as next record. PushFirstNestedRecordInfo(nextRecord, readStack); } else { - Debug.Assert(nextInfo.Parent is not null); - object value = reader.ReadPrimitiveValue(nextInfo.PrimitiveType); nextInfo.Parent.HandleNextValue(value, nextInfo); } } - nextRecord = DecodeNext(reader, recordMap, allowed, options, out recordType); - PushFirstNestedRecordInfo(nextRecord, readStack); - - // Only a binary library can be followed by something that is not a MessageEnd. - if (readStack.Count == 0 && recordType != SerializationRecordType.BinaryLibrary) + if (recordMap.UnresolvedReferences == 0) { + // There are no unresolved references, so the End is the only allowed record. allowed = AllowedRecordTypes.MessageEnd; } + else + { + // There are unresolved references and we don't know in what order they are going to appear. + allowed = AllowedRecordTypes.ReferencedRecord; + } + + nextRecord = DecodeNext(reader, recordMap, allowed, options, out recordType); + PushFirstNestedRecordInfo(nextRecord, readStack); } while (recordType != SerializationRecordType.MessageEnd); @@ -235,34 +229,44 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op private static SerializationRecord DecodeNext(BinaryReader reader, RecordMap recordMap, AllowedRecordTypes allowed, PayloadOptions options, out SerializationRecordType recordType) { - byte nextByte = reader.ReadByte(); - if (((uint)allowed & (1u << nextByte)) == 0) - { - ThrowHelper.ThrowForUnexpectedRecordType(nextByte); - } - recordType = (SerializationRecordType)nextByte; + SerializationRecord? record; - SerializationRecord record = recordType switch + do { - SerializationRecordType.ArraySingleObject => ArraySingleObjectRecord.Decode(reader), - SerializationRecordType.ArraySinglePrimitive => DecodeArraySinglePrimitiveRecord(reader), - SerializationRecordType.ArraySingleString => ArraySingleStringRecord.Decode(reader), - SerializationRecordType.BinaryArray => BinaryArrayRecord.Decode(reader, recordMap, options), - SerializationRecordType.BinaryLibrary => BinaryLibraryRecord.Decode(reader), - SerializationRecordType.BinaryObjectString => BinaryObjectStringRecord.Decode(reader), - SerializationRecordType.ClassWithId => ClassWithIdRecord.Decode(reader, recordMap), - SerializationRecordType.ClassWithMembersAndTypes => ClassWithMembersAndTypesRecord.Decode(reader, recordMap, options), - SerializationRecordType.MemberPrimitiveTyped => DecodeMemberPrimitiveTypedRecord(reader), - SerializationRecordType.MemberReference => MemberReferenceRecord.Decode(reader, recordMap), - SerializationRecordType.MessageEnd => MessageEndRecord.Singleton, - SerializationRecordType.ObjectNull => ObjectNullRecord.Instance, - SerializationRecordType.ObjectNullMultiple => ObjectNullMultipleRecord.Decode(reader), - SerializationRecordType.ObjectNullMultiple256 => ObjectNullMultiple256Record.Decode(reader), - SerializationRecordType.SerializedStreamHeader => SerializedStreamHeaderRecord.Decode(reader), - _ => SystemClassWithMembersAndTypesRecord.Decode(reader, recordMap, options), - }; + byte nextByte = reader.ReadByte(); + if (((uint)allowed & (1u << nextByte)) == 0) + { + ThrowHelper.ThrowForUnexpectedRecordType(nextByte); + } + recordType = (SerializationRecordType)nextByte; - recordMap.Add(record); + record = recordType switch + { + SerializationRecordType.ArraySingleObject => ArraySingleObjectRecord.Decode(reader), + SerializationRecordType.ArraySinglePrimitive => DecodeArraySinglePrimitiveRecord(reader), + SerializationRecordType.ArraySingleString => ArraySingleStringRecord.Decode(reader), + SerializationRecordType.BinaryArray => BinaryArrayRecord.Decode(reader, recordMap, options), + SerializationRecordType.BinaryLibrary => BinaryLibraryRecord.Decode(reader), + SerializationRecordType.BinaryObjectString => BinaryObjectStringRecord.Decode(reader), + SerializationRecordType.ClassWithId => ClassWithIdRecord.Decode(reader, recordMap), + SerializationRecordType.ClassWithMembersAndTypes => ClassWithMembersAndTypesRecord.Decode(reader, recordMap, options), + SerializationRecordType.MemberPrimitiveTyped => DecodeMemberPrimitiveTypedRecord(reader), + SerializationRecordType.MemberReference => MemberReferenceRecord.Decode(reader, recordMap), + SerializationRecordType.MessageEnd => MessageEndRecord.Singleton, + SerializationRecordType.ObjectNull => ObjectNullRecord.Instance, + SerializationRecordType.ObjectNullMultiple => ObjectNullMultipleRecord.Decode(reader), + SerializationRecordType.ObjectNullMultiple256 => ObjectNullMultiple256Record.Decode(reader), + SerializationRecordType.SerializedStreamHeader => SerializedStreamHeaderRecord.Decode(reader), + _ => SystemClassWithMembersAndTypesRecord.Decode(reader, recordMap, options), + }; + + recordMap.Add(record, allowed == AllowedRecordTypes.ReferencedRecord); + + // BinaryLibrary often precedes class records. + // It has been already added to the RecordMap and it must not be added + // to the array record or class member values, so simply read next record. + // It's possible to read multiple BinaryLibraryRecord in a row, hence the loop. + } while (recordType == SerializationRecordType.BinaryLibrary); return record; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs index a25ab508f5db3e..71f6a2b19a2942 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs @@ -4,6 +4,7 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Formats.Nrbf.Utils; using System.Runtime.InteropServices; using System.Runtime.Serialization; @@ -21,6 +22,8 @@ internal sealed class RecordMap : IReadOnlyDictionary _map[objectId]; + internal int UnresolvedReferences { get; private set; } + public bool ContainsKey(SerializationRecordId key) => _map.ContainsKey(key); public bool TryGetValue(SerializationRecordId key, [MaybeNullWhen(false)] out SerializationRecord value) => _map.TryGetValue(key, out value); @@ -29,8 +32,32 @@ internal sealed class RecordMap : IReadOnlyDictionary _map.GetEnumerator(); - internal void Add(SerializationRecord record) + internal void Add(SerializationRecord record, bool isReferencedRecord) { + if (isReferencedRecord) + { + if (record.Id._id <= 0) + { + // Negative record Ids should never be referenced. + // 0 is simply illegal Id for such records. + ThrowHelper.ThrowInvalidReference(); + } + else if (!_map.TryGetValue(record.Id, out SerializationRecord? stored) || stored is not MemberReferenceRecord memberReferenceRecord) + { + // The id was either unexpected or there was no reference stored for it. + ThrowHelper.ThrowForUnexpectedRecordType((byte)record.RecordType); + } + else if (((uint)memberReferenceRecord.ReferencedRecordType & (1u << (byte)record.RecordType)) == 0) + { + // We expected a reference to a record of a different type. + ThrowHelper.ThrowInvalidReference(); + } + + _map[record.Id] = record; + UnresolvedReferences--; + return; + } + // From https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-nrbf/0a192be0-58a1-41d0-8a54-9c91db0ab7bf: // "If the ObjectId is not referenced by any MemberReference in the serialization stream, // then the ObjectId SHOULD be positive, but MAY be negative." @@ -38,25 +65,21 @@ internal void Add(SerializationRecord record) { if (record.Id._id < 0) { - // Negative record Ids should never be referenced. Duplicate negative ids can be - // exported by the writer. The root object Id can be negative. + // Negative ids can be exported by the writer. The root object Id can be negative. _map[record.Id] = record; } - else + else if (!TryAdd(record.Id, record)) { -#if NET - if (_map.TryAdd(record.Id, record)) - { - return; - } -#else - if (!_map.ContainsKey(record.Id)) - { - _map.Add(record.Id, record); - return; - } -#endif - throw new SerializationException(SR.Format(SR.Serialization_DuplicateSerializationRecordId, record.Id)); + throw new SerializationException(SR.Format(SR.Serialization_DuplicateSerializationRecordId, record.Id._id)); + } + } + else if (record.RecordType == SerializationRecordType.MemberReference) + { + MemberReferenceRecord memberReferenceRecord = (MemberReferenceRecord)record; + + if (TryAdd(memberReferenceRecord.Reference, memberReferenceRecord)) + { + UnresolvedReferences++; } } } @@ -72,4 +95,18 @@ internal SerializationRecord GetRootRecord(SerializedStreamHeaderRecord header) return rootRecord; } + + private bool TryAdd(SerializationRecordId id, SerializationRecord record) + { +#if NET + return _map.TryAdd(id, record); +#else + if (!_map.ContainsKey(id)) + { + _map.Add(id, record); + return true; + } + return false; +#endif + } } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecord.cs index ccfce69124b706..d22c458514861c 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecord.cs @@ -52,7 +52,7 @@ internal SerializationRecord() // others can't derive from this type /// if the serialized type name match provided type; otherwise, . public bool TypeNameMatches(Type type) => Matches(type, TypeName); - private static bool Matches(Type type, TypeName typeName) + internal static bool Matches(Type type, TypeName typeName) { // We don't need to check for pointers and references to arrays, // as it's impossible to serialize them with BF. From 4eeabdeabb9c11bdea3d0274fa33e12b39246caf Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 17 Jun 2024 19:06:02 +0200 Subject: [PATCH 4/5] refactor --- .../Formats/Nrbf/MemberReferenceRecord.cs | 18 ++++++++++++++---- .../src/System/Formats/Nrbf/MemberTypeInfo.cs | 3 +-- .../src/System/Formats/Nrbf/NrbfDecoder.cs | 15 ++------------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs index 22145c9e17530c..2fd5233c3516f5 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs @@ -15,17 +15,18 @@ namespace System.Formats.Nrbf; /// internal sealed class MemberReferenceRecord : SerializationRecord { - private MemberReferenceRecord(SerializationRecordId reference, RecordMap recordMap) + private MemberReferenceRecord(SerializationRecordId reference, RecordMap recordMap, AllowedRecordTypes referencedRecordType) { Reference = reference; RecordMap = recordMap; + ReferencedRecordType = referencedRecordType; } public override SerializationRecordType RecordType => SerializationRecordType.MemberReference; internal SerializationRecordId Reference { get; } - internal AllowedRecordTypes ReferencedRecordType { get; set; } + internal AllowedRecordTypes ReferencedRecordType { get; } private RecordMap RecordMap { get; } @@ -37,8 +38,17 @@ private MemberReferenceRecord(SerializationRecordId reference, RecordMap recordM internal override object? GetValue() => GetReferencedRecord().GetValue(); - internal static MemberReferenceRecord Decode(BinaryReader reader, RecordMap recordMap) - => new(SerializationRecordId.Decode(reader), recordMap); + internal static MemberReferenceRecord Decode(BinaryReader reader, RecordMap recordMap, AllowedRecordTypes allowed) + { + SerializationRecordId reference = SerializationRecordId.Decode(reader); + + // We were supposed to decode a record of specific type or a reference to it. + // Since a reference was decoded and we don't know when the referenced record will be provided. + // We just store the allowed record type and are going to check it later. + AllowedRecordTypes referencedRecordType = allowed & ~(AllowedRecordTypes.MemberReference | AllowedRecordTypes.Nulls); + + return new MemberReferenceRecord(reference, recordMap, referencedRecordType); + } internal SerializationRecord GetReferencedRecord() => RecordMap[Reference]; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs index 3e1d34eb37421f..dcc7af894dbdf7 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs @@ -108,8 +108,7 @@ static AllowedRecordTypes MapSystemClassTypeName(TypeName typeName) { return SystemClass; } - - if (typeName.IsSZArray) + else if (typeName.IsSZArray) { TypeName elementTypeName = typeName.GetElementType(); if (SerializationRecord.Matches(typeof(decimal), elementTypeName) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs index becf5d67769db0..5089e34ca85ade 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs @@ -8,7 +8,6 @@ using System.Formats.Nrbf.Utils; using System.Text; using System.Runtime.Serialization; -using System.Diagnostics; namespace System.Formats.Nrbf; @@ -179,17 +178,7 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op if (nextInfo.Allowed != AllowedRecordTypes.None) { // Decode the next Record - nextRecord = DecodeNext(reader, recordMap, nextInfo.Allowed, options, out SerializationRecordType nextRecordType); - - if (nextRecordType == SerializationRecordType.MemberReference) - { - // We were supposed to decode a record of specific type or a reference to it. - // Since a reference was decoded and we don't know when the referenced record will be provided. - // We just store the allowed record type and are going to check it later. - ((MemberReferenceRecord)nextRecord).ReferencedRecordType = nextInfo.Allowed - & ~(AllowedRecordTypes.MemberReference | AllowedRecordTypes.Nulls); - } - + nextRecord = DecodeNext(reader, recordMap, nextInfo.Allowed, options, out _); // Handle it: // - add to the parent records list, // - push next info if there are remaining nested records to read. @@ -251,7 +240,7 @@ private static SerializationRecord DecodeNext(BinaryReader reader, RecordMap rec SerializationRecordType.ClassWithId => ClassWithIdRecord.Decode(reader, recordMap), SerializationRecordType.ClassWithMembersAndTypes => ClassWithMembersAndTypesRecord.Decode(reader, recordMap, options), SerializationRecordType.MemberPrimitiveTyped => DecodeMemberPrimitiveTypedRecord(reader), - SerializationRecordType.MemberReference => MemberReferenceRecord.Decode(reader, recordMap), + SerializationRecordType.MemberReference => MemberReferenceRecord.Decode(reader, recordMap, allowed), SerializationRecordType.MessageEnd => MessageEndRecord.Singleton, SerializationRecordType.ObjectNull => ObjectNullRecord.Instance, SerializationRecordType.ObjectNullMultiple => ObjectNullMultipleRecord.Decode(reader), From bc33259133c9ebc6c4438ee976d1666fdc1250bd Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 18 Jun 2024 11:56:08 +0200 Subject: [PATCH 5/5] finish the implementation --- .../System/Formats/Nrbf/AllowedRecordType.cs | 11 +-- .../Formats/Nrbf/ArraySingleObjectRecord.cs | 2 +- .../Formats/Nrbf/ArraySingleStringRecord.cs | 5 -- .../Formats/Nrbf/MemberReferenceRecord.cs | 12 +++- .../src/System/Formats/Nrbf/MemberTypeInfo.cs | 32 +++++++-- .../src/System/Formats/Nrbf/NrbfDecoder.cs | 17 +++-- .../src/System/Formats/Nrbf/RecordMap.cs | 72 +++++++++++++------ .../Formats/Nrbf/SerializationRecord.cs | 2 +- .../Formats/Nrbf/SerializationRecordId.cs | 2 + .../System/Formats/Nrbf/Utils/ThrowHelper.cs | 3 + 10 files changed, 110 insertions(+), 48 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/AllowedRecordType.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/AllowedRecordType.cs index 126a08f1d929e5..3a3be2c1f2dab7 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/AllowedRecordType.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/AllowedRecordType.cs @@ -27,15 +27,16 @@ internal enum AllowedRecordTypes : uint Nulls = ObjectNull | ObjectNullMultiple256 | ObjectNullMultiple, /// - /// Any .NET object (a primitive, a reference type, a reference or single null). + /// Any .NET object (a class, primitive type or an array). /// AnyObject = MemberPrimitiveTyped | ArraySingleObject | ArraySinglePrimitive | ArraySingleString | BinaryArray | ClassWithId | ClassWithMembersAndTypes | SystemClassWithMembersAndTypes | BinaryObjectString - | MemberReference - | ObjectNull, + | MemberReference, - // The referenced record can point to any object except of a null or another reference. - ReferencedRecord = AnyObject & ~(ObjectNull | MemberReference) + /// + /// Any .NET object or a reference or a single null. + /// + AnyObjectOrNullOrReference = AnyObject | ObjectNull | MemberReference, } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs index de77d3e0f84fac..f63e8a7e081245 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs @@ -70,7 +70,7 @@ internal static ArraySingleObjectRecord Decode(BinaryReader reader) internal override (AllowedRecordTypes allowed, PrimitiveType primitiveType) GetAllowedRecordType() { // An array of objects can contain any Object or multiple nulls. - const AllowedRecordTypes Allowed = AllowedRecordTypes.AnyObject | AllowedRecordTypes.Nulls; + const AllowedRecordTypes Allowed = AllowedRecordTypes.AnyObjectOrNullOrReference | AllowedRecordTypes.Nulls; return (Allowed, default); } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs index 4d7f5ace476791..689c30dcf5e704 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs @@ -57,11 +57,6 @@ internal override (AllowedRecordTypes allowed, PrimitiveType primitiveType) GetA if (record is MemberReferenceRecord memberReference) { record = memberReference.GetReferencedRecord(); - - if (record is not BinaryObjectStringRecord) - { - ThrowHelper.ThrowInvalidReference(); - } } if (record is BinaryObjectStringRecord stringRecord) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs index 2fd5233c3516f5..e5ed60e2d68a4b 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Formats.Nrbf.Utils; using System.IO; using System.Reflection.Metadata; @@ -26,7 +27,7 @@ private MemberReferenceRecord(SerializationRecordId reference, RecordMap recordM internal SerializationRecordId Reference { get; } - internal AllowedRecordTypes ReferencedRecordType { get; } + private AllowedRecordTypes ReferencedRecordType { get; } private RecordMap RecordMap { get; } @@ -51,4 +52,13 @@ internal static MemberReferenceRecord Decode(BinaryReader reader, RecordMap reco } internal SerializationRecord GetReferencedRecord() => RecordMap[Reference]; + + internal void VerifyReferencedRecordType(SerializationRecord serializationRecord) + { + if (((uint)ReferencedRecordType & (1u << (byte)serializationRecord.RecordType)) == 0) + { + // We expected a reference to a record of a different type. + ThrowHelper.ThrowInvalidReference(); + } + } } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs index dcc7af894dbdf7..eca93e4f8827bf 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs @@ -94,7 +94,7 @@ internal static MemberTypeInfo Decode(BinaryReader reader, int count, PayloadOpt { BinaryType.Primitive => (default, (PrimitiveType)additionalInfo!), BinaryType.String => (Strings, default), - BinaryType.Object => (AllowedRecordTypes.AnyObject, default), + BinaryType.Object => (AllowedRecordTypes.AnyObjectOrNullOrReference, default), BinaryType.StringArray => (StringArray, default), BinaryType.PrimitiveArray => (PrimitiveArray, default), BinaryType.Class => (((ClassTypeInfo)additionalInfo!).TypeName.IsArray ? NonPrimitiveArray : NonSystemClass, default), @@ -111,13 +111,31 @@ static AllowedRecordTypes MapSystemClassTypeName(TypeName typeName) else if (typeName.IsSZArray) { TypeName elementTypeName = typeName.GetElementType(); - if (SerializationRecord.Matches(typeof(decimal), elementTypeName) - || SerializationRecord.Matches(typeof(TimeSpan), elementTypeName) - || SerializationRecord.Matches(typeof(DateTime), elementTypeName)) + if (elementTypeName.IsSimple && elementTypeName.FullName.StartsWith("System.", StringComparison.Ordinal)) { - // BinaryFormatter should use BinaryType.PrimitiveArray for these 3 types, - // but it uses BinaryType.SystemClass and we need this workaround. - return PrimitiveArray; + switch (elementTypeName.FullName) + { + case "System.Boolean": + case "System.Byte": + case "System.SByte": + case "System.Char": + case "System.Int16": + case "System.UInt16": + case "System.Int32": + case "System.UInt32": + case "System.Int64": + case "System.UInt64": + case "System.Single": + case "System.Double": + case "System.Decimal": + case "System.DateTime": + case "System.TimeSpan": + // BinaryFormatter should use BinaryType.PrimitiveArray for these primitive types, + // but it uses BinaryType.SystemClass and we need this workaround. + return PrimitiveArray; + default: + break; + } } } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs index 5089e34ca85ade..cfa281cba2bb6c 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs @@ -160,10 +160,11 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op RecordMap recordMap = new(); // Every NRBF payload has to start with a header - var header = (SerializedStreamHeaderRecord)DecodeNext(reader, recordMap, AllowedRecordTypes.SerializedStreamHeader, options, out _); + AllowedRecordTypes allowed = AllowedRecordTypes.SerializedStreamHeader; + var header = (SerializedStreamHeaderRecord)DecodeNext(reader, recordMap, allowed, options, out _); - // All we know about root is its Id. It can be any Object or BinaryLibrary, but not a reference. - AllowedRecordTypes allowed = (AllowedRecordTypes.AnyObject & ~AllowedRecordTypes.MemberReference) | AllowedRecordTypes.BinaryLibrary; + // The root can be any Object or BinaryLibrary, but not a reference. + allowed = AllowedRecordTypes.AnyObject | AllowedRecordTypes.BinaryLibrary; SerializationRecord rootRecord = DecodeNext(reader, recordMap, allowed, options, out _); PushFirstNestedRecordInfo(rootRecord, readStack); @@ -203,10 +204,12 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op else { // There are unresolved references and we don't know in what order they are going to appear. - allowed = AllowedRecordTypes.ReferencedRecord; + // We allow for any Object (which does not include references or nulls). + // The actual type validation is going to be performed by RecordMap.Add. + allowed = AllowedRecordTypes.AnyObject | AllowedRecordTypes.BinaryLibrary; } - nextRecord = DecodeNext(reader, recordMap, allowed, options, out recordType); + nextRecord = DecodeNext(reader, recordMap, allowed, options, out recordType, isReferencedRecord: true); PushFirstNestedRecordInfo(nextRecord, readStack); } while (recordType != SerializationRecordType.MessageEnd); @@ -216,7 +219,7 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op } private static SerializationRecord DecodeNext(BinaryReader reader, RecordMap recordMap, - AllowedRecordTypes allowed, PayloadOptions options, out SerializationRecordType recordType) + AllowedRecordTypes allowed, PayloadOptions options, out SerializationRecordType recordType, bool isReferencedRecord = false) { SerializationRecord? record; @@ -249,7 +252,7 @@ private static SerializationRecord DecodeNext(BinaryReader reader, RecordMap rec _ => SystemClassWithMembersAndTypesRecord.Decode(reader, recordMap, options), }; - recordMap.Add(record, allowed == AllowedRecordTypes.ReferencedRecord); + recordMap.Add(record, isReferencedRecord); // BinaryLibrary often precedes class records. // It has been already added to the RecordMap and it must not be added diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs index 71f6a2b19a2942..2403ae30f6dc65 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs @@ -3,6 +3,7 @@ using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Formats.Nrbf.Utils; using System.Runtime.InteropServices; @@ -34,12 +35,56 @@ internal sealed class RecordMap : IReadOnlyDictionary if the serialized type name match provided type; otherwise, . public bool TypeNameMatches(Type type) => Matches(type, TypeName); - internal static bool Matches(Type type, TypeName typeName) + private static bool Matches(Type type, TypeName typeName) { // We don't need to check for pointers and references to arrays, // as it's impossible to serialize them with BF. diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecordId.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecordId.cs index d24e39b757d3ce..c278505d6b4138 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecordId.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecordId.cs @@ -57,4 +57,6 @@ public override int GetHashCode() #endif return (int)XxHash32.HashToUInt32(MemoryMarshal.AsBytes(integers)); } + + internal bool IsDefault => _id == default; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs index de543f2e098cf4..cc88f181c819b9 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs @@ -14,6 +14,9 @@ internal static void ThrowInvalidValue(object value) internal static void ThrowInvalidReference() => throw new SerializationException(SR.Serialization_InvalidReference); + internal static void ThrowDuplicateSerializationRecordId(SerializationRecordId id) + => throw new SerializationException(SR.Format(SR.Serialization_DuplicateSerializationRecordId, id._id)); + internal static void ThrowUnexpectedNullRecordCount() => throw new SerializationException(SR.Serialization_UnexpectedNullRecordCount);