From b8520b2efcbccab9a6a5a6c1a2f24d59ba62c0ce Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 18 Jun 2024 14:46:46 +0200 Subject: [PATCH 1/3] change StartsWithPayloadHeader to accept a Span rather than array --- .../ref/System.Formats.Nrbf.cs | 2 +- .../src/System/Formats/Nrbf/NrbfDecoder.cs | 38 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/ref/System.Formats.Nrbf.cs b/src/libraries/System.Formats.Nrbf/ref/System.Formats.Nrbf.cs index 22afcd06071199..8e12cf7c3712f5 100644 --- a/src/libraries/System.Formats.Nrbf/ref/System.Formats.Nrbf.cs +++ b/src/libraries/System.Formats.Nrbf/ref/System.Formats.Nrbf.cs @@ -48,7 +48,7 @@ public static partial class NrbfDecoder public static System.Formats.Nrbf.SerializationRecord Decode(System.IO.Stream payload, out System.Collections.Generic.IReadOnlyDictionary recordMap, System.Formats.Nrbf.PayloadOptions options=null, bool leaveOpen=false) { throw null; } public static System.Formats.Nrbf.SerializationRecord Decode(System.IO.Stream payload, System.Formats.Nrbf.PayloadOptions? options=null, bool leaveOpen=false) { throw null; } public static System.Formats.Nrbf.ClassRecord DecodeClassRecord(System.IO.Stream payload, System.Formats.Nrbf.PayloadOptions? options=null, bool leaveOpen=false) { throw null; } - public static bool StartsWithPayloadHeader(byte[] bytes) { throw null; } + public static bool StartsWithPayloadHeader(System.ReadOnlySpan bytes) { throw null; } public static bool StartsWithPayloadHeader(System.IO.Stream stream) { throw null; } } public sealed partial class PayloadOptions 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..84c3005ec7faec 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.Runtime.InteropServices; namespace System.Formats.Nrbf; @@ -23,26 +24,25 @@ public static class NrbfDecoder /// /// The buffer to inspect. /// if it starts with NRBF payload header; otherwise, . - public static bool StartsWithPayloadHeader(byte[] bytes) + public static bool StartsWithPayloadHeader(ReadOnlySpan bytes) { -#if NET - ArgumentNullException.ThrowIfNull(bytes); -#else - if (bytes is null) + if (bytes.Length < SerializedStreamHeaderRecord.Size || bytes[0] != (byte)SerializationRecordType.SerializedStreamHeader) { - throw new ArgumentNullException(nameof(bytes)); + return false; } -#endif - return bytes.Length >= SerializedStreamHeaderRecord.Size - && bytes[0] == (byte)SerializationRecordType.SerializedStreamHeader -#if NET - && BinaryPrimitives.ReadInt32LittleEndian(bytes.AsSpan(9)) == SerializedStreamHeaderRecord.MajorVersion - && BinaryPrimitives.ReadInt32LittleEndian(bytes.AsSpan(13)) == SerializedStreamHeaderRecord.MinorVersion; -#else - && BitConverter.ToInt32(bytes, 9) == SerializedStreamHeaderRecord.MajorVersion - && BitConverter.ToInt32(bytes, 13) == SerializedStreamHeaderRecord.MinorVersion; -#endif + ReadOnlySpan integers = MemoryMarshal.Cast(bytes.Slice(sizeof(byte), sizeof(int) * 4)); + int majorVersion = integers[2]; + int minorVersion = integers[3]; + + if (!BitConverter.IsLittleEndian) + { + majorVersion = BinaryPrimitives.ReverseEndianness(majorVersion); + minorVersion = BinaryPrimitives.ReverseEndianness(minorVersion); + } + + return majorVersion == SerializedStreamHeaderRecord.MajorVersion + && minorVersion == SerializedStreamHeaderRecord.MinorVersion; } @@ -76,13 +76,13 @@ public static bool StartsWithPayloadHeader(Stream stream) return false; } - byte[] buffer = new byte[SerializedStreamHeaderRecord.Size]; - try { #if NET - stream.ReadExactly(buffer, 0, buffer.Length); + Span buffer = stackalloc byte[SerializedStreamHeaderRecord.Size]; + stream.ReadExactly(buffer); #else + byte[] buffer = new byte[SerializedStreamHeaderRecord.Size]; int offset = 0; while (offset < buffer.Length) { From 5a3385d997b44e22f98ea87812cd906187821027 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 27 Jun 2024 17:28:59 +0200 Subject: [PATCH 2/3] address code review feedback: don't use MemoryMarshal.Cast because it's not guaranteed to work everywhere --- .../src/System/Formats/Nrbf/NrbfDecoder.cs | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) 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 84c3005ec7faec..1e42e6a64aebf3 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 @@ -31,21 +31,34 @@ public static bool StartsWithPayloadHeader(ReadOnlySpan bytes) return false; } - ReadOnlySpan integers = MemoryMarshal.Cast(bytes.Slice(sizeof(byte), sizeof(int) * 4)); - int majorVersion = integers[2]; - int minorVersion = integers[3]; - + // The header consists of: + // - a byte that describes the record type (SerializationRecordType.SerializedStreamHeader) + // - four 32 bit integers: + // - root Id (every value is valid) + // - header Id (value is ignored) + // - major version, it has to be equal 1. + // - minor version, it has to be equal 0. + // For little endian: 0 * * * * * * * * 1 0 0 0 0 0 0 0 + // For big endian: 0 * * * * * * * * 0 0 0 1 0 0 0 0 + + const int PrefixLength = sizeof(byte) + sizeof(int) * 2; + int majorVersionByteIndex = PrefixLength; if (!BitConverter.IsLittleEndian) { - majorVersion = BinaryPrimitives.ReverseEndianness(majorVersion); - minorVersion = BinaryPrimitives.ReverseEndianness(minorVersion); + majorVersionByteIndex += 3; + } + for (int i = PrefixLength; i < SerializedStreamHeaderRecord.Size; i++) + { + int expected = i == majorVersionByteIndex ? 1 : 0; + if (bytes[i] != expected) + { + return false; + } } - return majorVersion == SerializedStreamHeaderRecord.MajorVersion - && minorVersion == SerializedStreamHeaderRecord.MinorVersion; + return true; } - /// /// Checks if given stream starts with NRBF payload header. /// From 22f8b41d0eb30128cd92e67f83b581effaa3a51a Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 28 Jun 2024 12:14:37 +0200 Subject: [PATCH 3/3] address code review feedback --- .../src/System/Formats/Nrbf/NrbfDecoder.cs | 45 +++++-------------- 1 file changed, 12 insertions(+), 33 deletions(-) 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 1e42e6a64aebf3..d75a78073e4826 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 @@ -19,45 +19,24 @@ public static class NrbfDecoder { private static UTF8Encoding ThrowOnInvalidUtf8Encoding { get; } = new(false, throwOnInvalidBytes: true); + // The header consists of: + // - a byte that describes the record type (SerializationRecordType.SerializedStreamHeader) + // - four 32 bit integers: + // - root Id (every value is valid) + // - header Id (value is ignored) + // - major version, it has to be equal 1. + // - minor version, it has to be equal 0. + private static ReadOnlySpan HeaderSuffix => [1, 0, 0, 0, 0, 0, 0, 0]; + /// /// Checks if given buffer starts with NRBF payload header. /// /// The buffer to inspect. /// if it starts with NRBF payload header; otherwise, . public static bool StartsWithPayloadHeader(ReadOnlySpan bytes) - { - if (bytes.Length < SerializedStreamHeaderRecord.Size || bytes[0] != (byte)SerializationRecordType.SerializedStreamHeader) - { - return false; - } - - // The header consists of: - // - a byte that describes the record type (SerializationRecordType.SerializedStreamHeader) - // - four 32 bit integers: - // - root Id (every value is valid) - // - header Id (value is ignored) - // - major version, it has to be equal 1. - // - minor version, it has to be equal 0. - // For little endian: 0 * * * * * * * * 1 0 0 0 0 0 0 0 - // For big endian: 0 * * * * * * * * 0 0 0 1 0 0 0 0 - - const int PrefixLength = sizeof(byte) + sizeof(int) * 2; - int majorVersionByteIndex = PrefixLength; - if (!BitConverter.IsLittleEndian) - { - majorVersionByteIndex += 3; - } - for (int i = PrefixLength; i < SerializedStreamHeaderRecord.Size; i++) - { - int expected = i == majorVersionByteIndex ? 1 : 0; - if (bytes[i] != expected) - { - return false; - } - } - - return true; - } + => bytes.Length >= SerializedStreamHeaderRecord.Size + && bytes[0] == (byte)SerializationRecordType.SerializedStreamHeader + && bytes.Slice(SerializedStreamHeaderRecord.Size - HeaderSuffix.Length, HeaderSuffix.Length).SequenceEqual(HeaderSuffix); /// /// Checks if given stream starts with NRBF payload header.