Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBOR Writer: Use canonical NaN representation for NaN values #92934

Merged
merged 5 commits into from
Oct 6, 2023
Merged

CBOR Writer: Use canonical NaN representation for NaN values #92934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b6f257
Use canonical NaN representation for NaN values
tomeksowi Oct 2, 2023
8959a2a
Use canonical CBOR (positive) NaN in WriteHalf
tomeksowi Oct 4, 2023
e555cb9
Put canonical CBOR NaNs into named constants
tomeksowi Oct 5, 2023
a77502c
Add CborReader tests to verify the previously emitted negative NaN bi…
tomeksowi Oct 5, 2023
69b97e9
Use only half-float canonical CBOR representation for NaNs
tomeksowi Oct 6, 2023
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
5 changes: 5 additions & 0 deletions src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/HalfHelpers.netstandard.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ static float CreateSingle(bool sign, byte exp, uint sig)
=> CborHelpers.Int32BitsToSingle((int)(((sign ? 1U : 0U) << FloatSignShift) + ((uint)exp << FloatExponentShift) + sig));
}

public static bool HalfIsNaN(ushort value)
{
return (value & ~((ushort)1 << HalfSignShift)) > HalfPositiveInfinityBits;
}

private static (int Exp, uint Sig) NormSubnormalF16Sig(uint sig)
{
int shiftDist = LeadingZeroCount(sig) - 16 - 5;
Expand Down
6 changes: 5 additions & 1 deletion src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/Writer/CborWriter.Simple.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ namespace System.Formats.Cbor
{
public partial class CborWriter
{
// CBOR RFC 8949 says: if NaN is an allowed value, and there is no intent to support NaN payloads or signaling NaNs, the protocol needs to pick a single representation, typically 0xf97e00. If that simple choice is not possible, specific attention will be needed for NaN handling.
// In this implementation "that simple choice is not possible" for CTAP2 mode (RequiresPreservingFloatPrecision), in which "representations of any floating-point values are not changed".
private const ushort PositiveQNaNBitsHalf = 0x7e00;

// Implements major type 7 encoding per https://tools.ietf.org/html/rfc7049#section-2.1

/// <summary>Writes a single-precision floating point number (major type 7).</summary>
Expand Down Expand Up @@ -130,7 +134,7 @@ public void WriteSimpleValue(CborSimpleValue value)
private static bool TryConvertDoubleToSingle(double value, out float result)
{
result = (float)value;
return BitConverter.DoubleToInt64Bits(result) == BitConverter.DoubleToInt64Bits(value);
return double.IsNaN(value) || BitConverter.DoubleToInt64Bits(result) == BitConverter.DoubleToInt64Bits(value);
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
11 changes: 9 additions & 2 deletions ...raries/System.Formats.Cbor/src/System/Formats/Cbor/Writer/CborWriter.Simple.netcoreapp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,14 @@ public void WriteHalf(Half value)
{
EnsureWriteCapacity(1 + sizeof(short));
WriteInitialByte(new CborInitialByte(CborMajorType.Simple, CborAdditionalInfo.Additional16BitData));
BinaryPrimitives.WriteHalfBigEndian(_buffer.AsSpan(_offset), value);
if (Half.IsNaN(value) && !CborConformanceModeHelpers.RequiresPreservingFloatPrecision(ConformanceMode))
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{
BinaryPrimitives.WriteUInt16BigEndian(_buffer.AsSpan(_offset), PositiveQNaNBitsHalf);
}
else
{
BinaryPrimitives.WriteHalfBigEndian(_buffer.AsSpan(_offset), value);
}
_offset += sizeof(short);
AdvanceDataItemCounters();
}
Expand All @@ -30,7 +37,7 @@ public void WriteHalf(Half value)
internal static bool TryConvertSingleToHalf(float value, out Half result)
{
result = (Half)value;
return BitConverter.SingleToInt32Bits((float)result) == BitConverter.SingleToInt32Bits(value);
return float.IsNaN(value) || BitConverter.SingleToInt32Bits((float)result) == BitConverter.SingleToInt32Bits(value);
}
}
}
11 changes: 9 additions & 2 deletions ...aries/System.Formats.Cbor/src/System/Formats/Cbor/Writer/CborWriter.Simple.netstandard.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,14 @@ private void WriteHalf(ushort value)
{
EnsureWriteCapacity(1 + sizeof(ushort));
WriteInitialByte(new CborInitialByte(CborMajorType.Simple, CborAdditionalInfo.Additional16BitData));
CborHelpers.WriteHalfBigEndian(_buffer.AsSpan(_offset), value);
if (HalfHelpers.HalfIsNaN(value) && !CborConformanceModeHelpers.RequiresPreservingFloatPrecision(ConformanceMode))
{
BinaryPrimitives.WriteUInt16BigEndian(_buffer.AsSpan(_offset), PositiveQNaNBitsHalf);
}
else
{
CborHelpers.WriteHalfBigEndian(_buffer.AsSpan(_offset), value);
}
_offset += sizeof(ushort);
AdvanceDataItemCounters();
}
Expand All @@ -30,7 +37,7 @@ private void WriteHalf(ushort value)
internal static bool TryConvertSingleToHalf(float value, out ushort result)
{
result = HalfHelpers.FloatToHalf(value);
return CborHelpers.SingleToInt32Bits(HalfHelpers.HalfToFloat(result)) == CborHelpers.SingleToInt32Bits(value);
return float.IsNaN(value) || CborHelpers.SingleToInt32Bits(HalfHelpers.HalfToFloat(result)) == CborHelpers.SingleToInt32Bits(value);
}
}
}
3 changes: 3 additions & 0 deletions src/libraries/System.Formats.Cbor/tests/CborTestHelpers.netcoreapp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,8 @@ namespace System.Formats.Cbor.Tests
internal static class CborTestHelpers
{
public static readonly DateTimeOffset UnixEpoch = DateTimeOffset.UnixEpoch;

public static int SingleToInt32Bits(float value)
=> BitConverter.SingleToInt32Bits(value);
}
}
3 changes: 3 additions & 0 deletions src/libraries/System.Formats.Cbor/tests/CborTestHelpers.netstandard.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,8 @@ internal static class CborTestHelpers
{
private const long UnixEpochTicks = 719162L /*Number of days from 1/1/0001 to 12/31/1969*/ * 10000 * 1000 * 60 * 60 * 24; /* Ticks per day.*/
public static readonly DateTimeOffset UnixEpoch = new DateTimeOffset(UnixEpochTicks, TimeSpan.Zero);

public static unsafe int SingleToInt32Bits(float value)
=> *((int*)&value);
}
}
5 changes: 5 additions & 0 deletions src/libraries/System.Formats.Cbor/tests/Reader/CborReaderTests.Simple.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public partial class CborReaderTests
[InlineData(float.PositiveInfinity, "fa7f800000")]
[InlineData(float.NegativeInfinity, "faff800000")]
[InlineData(float.NaN, "fa7fc00000")]
[InlineData(float.NaN, "faffc00000")]
public static void ReadSingle_SingleValue_HappyPath(float expectedResult, string hexEncoding)
{
byte[] encoding = hexEncoding.HexToByteArray();
Expand All @@ -36,6 +37,7 @@ public static void ReadSingle_SingleValue_HappyPath(float expectedResult, string
[InlineData(double.PositiveInfinity, "fb7ff0000000000000")]
[InlineData(double.NegativeInfinity, "fbfff0000000000000")]
[InlineData(double.NaN, "fb7ff8000000000000")]
[InlineData(double.NaN, "fbfff8000000000000")]
public static void ReadDouble_SingleValue_HappyPath(double expectedResult, string hexEncoding)
{
byte[] encoding = hexEncoding.HexToByteArray();
Expand All @@ -52,6 +54,7 @@ public static void ReadDouble_SingleValue_HappyPath(double expectedResult, strin
[InlineData(double.PositiveInfinity, "fa7f800000")]
[InlineData(double.NegativeInfinity, "faff800000")]
[InlineData(double.NaN, "fa7fc00000")]
[InlineData(double.NaN, "faffc00000")]
public static void ReadDouble_SinglePrecisionValue_ShouldCoerceToDouble(double expectedResult, string hexEncoding)
{
byte[] encoding = hexEncoding.HexToByteArray();
Expand All @@ -73,6 +76,7 @@ public static void ReadDouble_SinglePrecisionValue_ShouldCoerceToDouble(double e
[InlineData(-4.0, "f9c400")]
[InlineData(double.PositiveInfinity, "f97c00")]
[InlineData(double.NaN, "f97e00")]
[InlineData(double.NaN, "f9fe00")]
[InlineData(double.NegativeInfinity, "f9fc00")]
public static void ReadDouble_HalfPrecisionValue_ShouldCoerceToDouble(double expectedResult, string hexEncoding)
{
Expand All @@ -95,6 +99,7 @@ public static void ReadDouble_HalfPrecisionValue_ShouldCoerceToDouble(double exp
[InlineData(-4.0, "f9c400")]
[InlineData(float.PositiveInfinity, "f97c00")]
[InlineData(float.NaN, "f97e00")]
[InlineData(float.NaN, "f9fe00")]
[InlineData(float.NegativeInfinity, "f9fc00")]
public static void ReadSingle_HalfPrecisionValue_ShouldCoerceToSingle(float expectedResult, string hexEncoding)
{
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Formats.Cbor/tests/System.Formats.Cbor.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<NoWarn>$(NoWarn);CS8002</NoWarn>
<!-- FSharp.Core: Could not find embedded resource 'FSharpOptimizationCompressedData.FSharp.Core' to remove in assembly 'FSharp.Core'. See https://github.com/dotnet/fsharp/pull/14395 -->
<NoWarn>$(NoWarn);IL2040</NoWarn>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(CommonTestPath)System\Security\Cryptography\ByteUtils.cs">
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.Formats.Cbor/tests/Writer/CborWriterTests.Array.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public partial class CborWriterTests
[InlineData(new object[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25 }, "98190102030405060708090a0b0c0d0e0f101112131415161718181819")]
[InlineData(new object[] { 1, -1, "", new byte[] { 7 } }, "840120604107")]
[InlineData(new object[] { "lorem", "ipsum", "dolor" }, "83656c6f72656d65697073756d65646f6c6f72")]
[InlineData(new object?[] { false, null, float.NaN, double.PositiveInfinity }, "84f4f6f9fe00f97c00")]
[InlineData(new object?[] { false, null, float.NaN, double.PositiveInfinity }, "84f4f6f97e00f97c00")]
public static void WriteArray_SimpleValues_HappyPath(object[] values, string expectedHexEncoding)
{
byte[] expectedEncoding = expectedHexEncoding.HexToByteArray();
Expand Down Expand Up @@ -48,7 +48,7 @@ public static void WriteArray_NestedValues_HappyPath(object[] values, string exp
[InlineData(new object[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25 }, "9f0102030405060708090a0b0c0d0e0f101112131415161718181819ff")]
[InlineData(new object[] { 1, -1, "", new byte[] { 7 } }, "9f0120604107ff")]
[InlineData(new object[] { "lorem", "ipsum", "dolor" }, "9f656c6f72656d65697073756d65646f6c6f72ff")]
[InlineData(new object?[] { false, null, float.NaN, double.PositiveInfinity }, "9ff4f6f9fe00f97c00ff")]
[InlineData(new object?[] { false, null, float.NaN, double.PositiveInfinity }, "9ff4f6f97e00f97c00ff")]
public static void WriteArray_IndefiniteLength_NoPatching_HappyPath(object[] values, string expectedHexEncoding)
{
byte[] expectedEncoding = expectedHexEncoding.HexToByteArray();
Expand Down Expand Up @@ -82,7 +82,7 @@ public static void WriteArray_IndefiniteLength_NoPatching_NestedValues_HappyPath
[InlineData(new object[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25 }, "98190102030405060708090a0b0c0d0e0f101112131415161718181819")]
[InlineData(new object[] { 1, -1, "", new byte[] { 7 } }, "840120604107")]
[InlineData(new object[] { "lorem", "ipsum", "dolor" }, "83656c6f72656d65697073756d65646f6c6f72")]
[InlineData(new object?[] { false, null, float.NaN, double.PositiveInfinity }, "84f4f6f9fe00f97c00")]
[InlineData(new object?[] { false, null, float.NaN, double.PositiveInfinity }, "84f4f6f97e00f97c00")]
public static void WriteArray_IndefiniteLength_WithPatching_HappyPath(object[] values, string expectedHexEncoding)
{
byte[] expectedEncoding = expectedHexEncoding.HexToByteArray();
Expand Down
38 changes: 28 additions & 10 deletions src/libraries/System.Formats.Cbor/tests/Writer/CborWriterTests.Simple.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public partial class CborWriterTests
[InlineData(3.4028234663852886e+38, "fa7f7fffff")]
[InlineData(float.PositiveInfinity, "f97c00")]
[InlineData(float.NegativeInfinity, "f9fc00")]
[InlineData(float.NaN, "f9fe00")]
[InlineData(float.NaN, "f97e00")]
public static void WriteSingle_SingleValue_HappyPath(float input, string hexExpectedEncoding)
{
byte[] expectedEncoding = hexExpectedEncoding.HexToByteArray();
Expand All @@ -26,9 +26,9 @@ public static void WriteSingle_SingleValue_HappyPath(float input, string hexExpe
}

[Theory]
[InlineData(float.NaN, "f9fe00", CborConformanceMode.Lax)]
[InlineData(float.NaN, "f9fe00", CborConformanceMode.Strict)]
[InlineData(float.NaN, "f9fe00", CborConformanceMode.Canonical)]
[InlineData(float.NaN, "f97e00", CborConformanceMode.Lax)]
[InlineData(float.NaN, "f97e00", CborConformanceMode.Strict)]
[InlineData(float.NaN, "f97e00", CborConformanceMode.Canonical)]
public static void WriteSingle_NonCtapConformance_ShouldMinimizePrecision(float input, string hexExpectedEncoding, CborConformanceMode mode)
{
byte[] expectedEncoding = hexExpectedEncoding.HexToByteArray();
Expand All @@ -42,7 +42,6 @@ public static void WriteSingle_NonCtapConformance_ShouldMinimizePrecision(float
[InlineData(3.4028234663852886e+38, "fa7f7fffff")]
[InlineData(float.PositiveInfinity, "fa7f800000")]
[InlineData(float.NegativeInfinity, "faff800000")]
[InlineData(float.NaN, "faffc00000")]
public static void WriteSingle_Ctap2Conformance_ShouldPreservePrecision(float input, string hexExpectedEncoding)
{
byte[] expectedEncoding = hexExpectedEncoding.HexToByteArray();
Expand All @@ -51,14 +50,24 @@ public static void WriteSingle_Ctap2Conformance_ShouldPreservePrecision(float in
AssertHelper.HexEqual(expectedEncoding, writer.Encode());
}

[Fact]
public static void WriteSingle_Ctap2Conformance_ShouldPreservePrecision_NaN()
{
// float.NaN may differ across architectures, in particular it's negative on x86 and positive elsewhere
byte[] expectedEncoding = ("fa" + CborTestHelpers.SingleToInt32Bits(float.NaN).ToString("x4")).HexToByteArray();
var writer = new CborWriter(CborConformanceMode.Ctap2Canonical);
writer.WriteSingle(float.NaN);
AssertHelper.HexEqual(expectedEncoding, writer.Encode());
}

[Theory]
[InlineData(1.1, "fb3ff199999999999a")]
[InlineData(1.0e+300, "fb7e37e43c8800759c")]
[InlineData(-4.1, "fbc010666666666666")]
[InlineData(3.1415926, "fb400921fb4d12d84a")]
[InlineData(double.PositiveInfinity, "f97c00")]
[InlineData(double.NegativeInfinity, "f9fc00")]
[InlineData(double.NaN, "f9fe00")]
[InlineData(double.NaN, "f97e00")]
public static void WriteDouble_SingleValue_HappyPath(double input, string hexExpectedEncoding)
{
byte[] expectedEncoding = hexExpectedEncoding.HexToByteArray();
Expand All @@ -68,9 +77,9 @@ public static void WriteDouble_SingleValue_HappyPath(double input, string hexExp
}

[Theory]
[InlineData(double.NaN, "f9fe00", CborConformanceMode.Lax)]
[InlineData(double.NaN, "f9fe00", CborConformanceMode.Strict)]
[InlineData(double.NaN, "f9fe00", CborConformanceMode.Canonical)]
[InlineData(double.NaN, "f97e00", CborConformanceMode.Lax)]
[InlineData(double.NaN, "f97e00", CborConformanceMode.Strict)]
[InlineData(double.NaN, "f97e00", CborConformanceMode.Canonical)]
[InlineData(65505, "fa477fe100", CborConformanceMode.Lax)]
[InlineData(65505, "fa477fe100", CborConformanceMode.Strict)]
[InlineData(65505, "fa477fe100", CborConformanceMode.Canonical)]
Expand All @@ -89,7 +98,6 @@ public static void WriteDouble_NonCtapConformance_ShouldMinimizePrecision(double
[InlineData(3.1415926, "fb400921fb4d12d84a")]
[InlineData(double.PositiveInfinity, "fb7ff0000000000000")]
[InlineData(double.NegativeInfinity, "fbfff0000000000000")]
[InlineData(double.NaN, "fbfff8000000000000")]
public static void WriteDouble_Ctap2Conformance_ShouldPreservePrecision(double input, string hexExpectedEncoding)
{
byte[] expectedEncoding = hexExpectedEncoding.HexToByteArray();
Expand All @@ -98,6 +106,16 @@ public static void WriteDouble_Ctap2Conformance_ShouldPreservePrecision(double i
AssertHelper.HexEqual(expectedEncoding, writer.Encode());
}

[Fact]
public static void WriteDouble_Ctap2Conformance_ShouldPreservePrecision_NaN()
{
// double.NaN may differ across architectures, in particular it's negative on x86 and positive elsewhere
byte[] expectedEncoding = ("fb" + BitConverter.DoubleToInt64Bits(double.NaN).ToString("x8")).HexToByteArray();
var writer = new CborWriter(CborConformanceMode.Ctap2Canonical);
writer.WriteDouble(double.NaN);
AssertHelper.HexEqual(expectedEncoding, writer.Encode());
}

[Fact]
public static void WriteNull_SingleValue_HappyPath()
{
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Formats.Cbor/tests/Writer/CborWriterTests.Simple.netcoreapp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public partial class CborWriterTests
[InlineData(0.00006103515625, "f90400")]
[InlineData(-4.0, "f9c400")]
[InlineData(float.PositiveInfinity, "f97c00")]
[InlineData(float.NaN, "f9fe00")]
tomeksowi marked this conversation as resolved.
Show resolved Hide resolved
[InlineData(float.NaN, "f97e00")]
[InlineData(float.NegativeInfinity, "f9fc00")]
public static void WriteHalf_SingleValue_HappyPath(float input, string hexExpectedEncoding)
{
Expand Down
Loading